search

tensorflow / mlir

"Multi-Level Intermediate Representation" Compiler Infrastructure
1.73k stars 257 forks source link

Rename MemRefType to RankedMemRefType - NFC #268

Closed nmostafa closed 4 years ago

nmostafa commented 4 years ago

This change renames MemRefType to RankedMemRefType to enable #261 to introduce base MemRefType class and Unranked/RankedMemRefType sub-classes.

nicolasvasilache commented 4 years ago

Thanks for doing this! This one is not going to be fun as conflicts will arise often if we don't push it quick.. Can you rebase please as it's already conflicting?

nicolasvasilache commented 4 years ago

I'll try to iterate quickly on it, please also mark it "- NFC" and rebase so I can start our internal tests.

bondhugula commented 4 years ago

To keep names and lines short and because the ranked MemRefType is far more widespread in use and utility across the codebase, would it make sense to just continue to use MemRefType for a ranked memref and use RankErasedMemRefType or UnrankedMemRefType for an unranked one? Note that the use case for an unranked tensor type (eg. shape inference, etc. in more abstract levels of the IR) are quite different from that of an unranked memref type(?) and so it isn't strictly needed to maintain naming consistency across the two. (IIUC, unranked memref types are more for interfacing.)

nicolasvasilache commented 4 years ago

@bondhugula see @River707's comments in #261 which prompted this change.

bondhugula commented 4 years ago

@bondhugula see @River707's comments in #261 which prompted this change.

Thanks for the context - I'll post there.

nmostafa commented 4 years ago

@nicolasvasilache, seems master was broken yesterday. Updated branch again.

nmostafa commented 4 years ago

Agreed that a rename is not necessary. Closing.