This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Mark additional builtin attr methods const
ClosedPublic

Authored by fabianschuiki on Jun 7 2021, 8:37 AM.

Details

Summary
  • Mark the following methods const:
    • ArrayAttr::getAsRange
    • ArrayAttr::getAsValueRange
    • DictionaryAttr::getAs(StringRef)
  • Add a DictionarAttr::getAs(Identifier) overload that may be faster in some cases.

Diff Detail

Event Timeline

fabianschuiki created this revision.Jun 7 2021, 8:37 AM
fabianschuiki requested review of this revision.Jun 7 2021, 8:37 AM
rriddle accepted this revision.Jun 7 2021, 10:45 AM
rriddle added inline comments.
mlir/include/mlir/IR/BuiltinAttributes.h
13 ↗(On Diff #350311)

Please try to avoid the need for an extra dependency here.

This revision is now accepted and ready to land.Jun 7 2021, 10:45 AM
fabianschuiki marked an inline comment as done.Jun 8 2021, 5:31 AM
fabianschuiki added inline comments.
mlir/include/mlir/IR/BuiltinAttributes.h
13 ↗(On Diff #350311)

Done. Only way I found to avoid this is to make getAs generic over the type of its name argument. Otherwise the compiler would choke on Identifier being an incomplete type, requiring all places that include BuiltinAttributes.h to also include Identifier.h. With the template the only place where this is necessary are call sites that pass in an Identifier, which are already very likely to include the appropriate header.

This revision was landed with ongoing or failed builds.Jun 8 2021, 5:45 AM
This revision was automatically updated to reflect the committed changes.
fabianschuiki marked an inline comment as done.
rriddle added inline comments.Jun 8 2021, 12:38 PM
mlir/include/mlir/IR/BuiltinAttributes.td
349

nit; You'll likely want to do NameClass && here.

fabianschuiki marked an inline comment as done.Jun 9 2021, 1:05 AM
fabianschuiki added inline comments.
mlir/include/mlir/IR/BuiltinAttributes.td
349

Added in 41135a4367a7.