[PATCH] D72397: m_Constant()

[Alex Zinenko via Phabricator via llvm-commits]

ftynse requested changes to this revision.
ftynse added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/include/mlir/IR/Matchers.h:54
+struct constant_op {
+  bool match(Operation *op) {
+    if (op->getNumOperands() > 0 || op->getNumResults() != 1)
----------------
This looks a bit weird to me: it matches _any_ side-effect free operation without operands and with one result. There's nothing specific about it being a _constant_. For example, gpu::ThreadIdOp fits the bill. I think the original `constant_op_binder` was working correctly because it attempted to constant-fold the operation and check if an attribute (e.g. a compile-time constant) value was produced as a result. This is only possible for actual constants.

If you want to avoid capturing the value, you may just extend the `constant_op_binder` to work around the null pointer in `bind_value`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72397