This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add derived attribute op interface
ClosedPublic

Authored by jpienaar on Mar 10 2020, 11:46 AM.

Details

Reviewers
rriddle
Summary

Interface provides uniform access to the the derived attribute query method.

Diff Detail

Event Timeline

jpienaar created this revision.Mar 10 2020, 11:46 AM
rriddle added inline comments.Mar 10 2020, 1:06 PM
mlir/include/mlir/Analysis/DerivedAttributeOpInterface.h
2

Can we move this to the newly added Interfaces/ directory?

17

I believe you should only need OpDefinition.h here.

mlir/include/mlir/Analysis/DerivedAttributeOpInterface.td
20

Can you provide more detail here on what derived attributes are?

rriddle edited the summary of this revision. (Show Details)Mar 10 2020, 1:07 PM
rriddle added a reviewer: rriddle.
jpienaar updated this revision to Diff 249774.Mar 11 2020, 3:08 PM
jpienaar marked 2 inline comments as done.

Moved to interfaces directory.

jpienaar updated this revision to Diff 249777.Mar 11 2020, 3:15 PM

forgot cmakefile ...

Updated, thanks

rriddle added inline comments.Mar 11 2020, 3:32 PM
mlir/include/mlir/Interfaces/CMakeLists.txt
13 ↗(On Diff #249777)

Looks like you are missing the accompanying .cpp file and the necessary CMake changes, i.e. nothing seems to include DerivedAttributeOpInterface.cpp.inc

mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.h
20 ↗(On Diff #249777)

This needs to be updated.

mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td
22 ↗(On Diff #249777)

typo: stotred

29 ↗(On Diff #249777)

nit: string -> name

jpienaar updated this revision to Diff 249794.Mar 11 2020, 4:21 PM
jpienaar marked 5 inline comments as done.

adding more missing files

mlir/include/mlir/Interfaces/CMakeLists.txt
13 ↗(On Diff #249777)

D'oh, seems I should probably avoid operating heavy machinery today ...

rriddle accepted this revision.Mar 12 2020, 10:29 AM
rriddle added inline comments.
mlir/lib/Interfaces/DerivedAttributeOpInterface.cpp
17 ↗(On Diff #249794)

Note: this isn't strictly necessary given 'using namespace mlir'.

This revision is now accepted and ready to land.Mar 12 2020, 10:29 AM
jpienaar closed this revision.Mar 12 2020, 10:32 AM