This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ods] Add materialize derived attribute method
ClosedPublic

Authored by jpienaar on Apr 16 2020, 8:05 AM.

Details

Summary

Generate method to generate a DictionaryAttr with attribute values of derived
attribute. If a conversion back from the derived attribute C++ type to
Attribute is not defined, then attempting to materialize such an op's derived
attributes would result in runtime failure.

This allows to treat derived attributes and attributes of an op in more uniform manner where needed. The derived attributes are not added to the operation but returned as new attribute instead.

Diff Detail

Event Timeline

jpienaar created this revision.Apr 16 2020, 8:05 AM
Herald added a project: Restricted Project. · View Herald Transcript
rriddle added inline comments.Apr 16 2020, 10:48 AM
mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td
35

Can you indent this like the others?

mlir/test/lib/Dialect/Test/TestPatterns.cpp
146

Seems easier to just do:

emitRemark() << d.first << " = " << d.second;

jpienaar updated this revision to Diff 258740.Apr 20 2020, 7:42 AM
jpienaar marked 2 inline comments as done.

Addressing review suggestions and add missing cmake dep

Updated, thanks

jpienaar updated this revision to Diff 258742.Apr 20 2020, 7:52 AM

rebase to head to avoid unrelated failure at previous head

rriddle accepted this revision.Apr 20 2020, 10:42 AM
rriddle added inline comments.
mlir/include/mlir/IR/OpBase.td
1434

Can we add a substitution for a builder instance? The builder makes constructing many of the standard attributes much easier.

mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td
37

Weird indent here.

mlir/test/lib/Dialect/Test/CMakeLists.txt
33 ↗(On Diff #258742)

Can we have these sorted?

mlir/test/lib/Dialect/Test/TestPatterns.cpp
146

Is the strref necessary here?

This revision is now accepted and ready to land.Apr 20 2020, 10:42 AM
jpienaar updated this revision to Diff 258822.Apr 20 2020, 1:09 PM
jpienaar marked 5 inline comments as done.

Add builder as replacement pattern

This revision was automatically updated to reflect the committed changes.