[PATCH] D78302: [mlir][ods] Add materialize derived attribute method

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 20 10:48:31 PDT 2020


rriddle accepted this revision.
rriddle added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/include/mlir/IR/OpBase.td:1434
+  //
+  // * `$_ctx` will be replaced by the MLIRContext* instance.
+  // * `$_self` will be replaced with the derived attribute (value produces
----------------
Can we add a substitution for a builder instance? The builder makes constructing many of the standard attributes much easier.


================
Comment at: mlir/include/mlir/Interfaces/DerivedAttributeOpInterface.td:37
+        Materializes the derived attributes. Returns null attribute where
+	unable to materialize a derived attribute as attribute.
+      }],
----------------
Weird indent here.


================
Comment at: mlir/test/lib/Dialect/Test/CMakeLists.txt:33
   MLIRTransformUtils
+  MLIRDerivedAttributeOpInterface
   MLIRInferTypeOpInterface
----------------
Can we have these sorted?


================
Comment at: mlir/test/lib/Dialect/Test/TestPatterns.cpp:146
+    for (auto d : dAttr)
+      dOp.emitRemark() << d.first.strref() << " = " << d.second;
+  });
----------------
Is the strref necessary here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78302





More information about the llvm-commits mailing list