Page MenuHomePhabricator

[mlir] Replace usages of Identifier with StringAttr

Authored by rriddle on Nov 9 2021, 6:43 PM.



Identifier and StringAttr essentially serve the same purpose,
i.e. to hold a string value. Keeping these seemingly identical
pieces of functionality separate has caused problems in certain situations:

  • Identifier has nice accessors that StringAttr doesn't
  • Identifier can't be used as an Attribute, meaning strings are often duplicated between Identifier/StringAttr (e.g. in PDL)

The only thing that Identifier has that StringAttr doesn't is support
for caching a dialect that is referenced by the string (e.g.
This functionality is added to StringAttr, as this is useful for StringAttr
in generally the same ways it was useful for Identifier.

Diff Detail

Event Timeline

rriddle created this revision.Nov 9 2021, 6:43 PM
rriddle requested review of this revision.Nov 9 2021, 6:43 PM
lattner accepted this revision.Nov 9 2021, 9:15 PM

Awesome, this is a great direction that will simplify things. Thank you!


Randomly, this alone will be really handy. I have built a lot of densemaps with "attribute" as the key instead of StringAttr just to work around this :-)


Should this be size_t instead of unsigned?

This revision is now accepted and ready to land.Nov 9 2021, 9:15 PM

LG, thanks River!

bondhugula added inline comments.

is a -> is


Add a trailing "null otherwise"?

This revision was automatically updated to reflect the committed changes.
rriddle marked 4 inline comments as done.

Thanks for the review!


Thanks for catching this, copy over from the old Identifier class.