[PATCH] D75953: [mlir] Add derived attribute op interface

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 11 15:33:08 PDT 2020


rriddle added inline comments.


================
Comment at: mlir/include/mlir/Interfaces/CMakeLists.txt:13
+mlir_tablegen(DerivedAttributeOpInterface.h.inc -gen-op-interface-decls)
+mlir_tablegen(DerivedAttributeOpInterface.cpp.inc -gen-op-interface-defs)
+add_public_tablegen_target(MLIRDerivedAttributeOpInterfaceIncGen)
----------------
Looks like you are missing the accompanying .cpp file and the necessary CMake changes, i.e. nothing seems to include `DerivedAttributeOpInterface.cpp.inc`


================
Comment at: mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.h:20
+
+#include "mlir/Analysis/DerivedAttributeOpInterface.h.inc"
+
----------------
This needs to be updated.


================
Comment at: mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td:22
+
+    Derived attributes are not stotred in the operation but are instead derived
+    from information of the operation. ODS generates convenience accessors for
----------------
typo: stotred


================
Comment at: mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td:29
+    StaticInterfaceMethod<
+      /*desc=*/"Returns whether string corresponds to derived attribute.",
+      /*retTy=*/"bool",
----------------
nit: string -> `name`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75953/new/

https://reviews.llvm.org/D75953





More information about the llvm-commits mailing list