[PATCH] D76020: [mlir] Add a new `ConstantLike` trait to better identify operations that represent a "constant".

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 12 11:56:26 PDT 2020


rriddle added inline comments.


================
Comment at: mlir/include/mlir/IR/OpDefinition.h:916
+                  "expected operation to take zero operands");
+    // TODO: We should verify that the operation can always be folded, but this
+    // requires that the attributes of the op already be verified. We should add
----------------
jpienaar wrote:
> What would happen if this wasn't true for the op? What error/assert/failure would be triggered?
An assert in m_Constant(i.e. all of the current users) would fire when the folding fails.


================
Comment at: mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp:1551
+
+  // Add the native and interface traits.
+  for (const auto &trait : op.getTraits()) {
----------------
jpienaar wrote:
> I think we also are missing documentation on the ordering effect of traits and even discussion pending on if order should matter ...
Yes, I find the behavior surprising sometimes when my mental model conflicts with what the implementation is. I'll try to draft something in a followup for Traits.md.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76020





More information about the llvm-commits mailing list