[mlir] m_Constant()

[chelini]

Summary

Introduce m_Constant() which allows matching a constant operation without forcing the user also to capture the attribute value.

[ftynse]

mlir/include/mlir/IR/Matchers.h

54

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.

[merge_guards_bot]

Unit tests: pass. 61312 tests passed, 0 failed and 736 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[lebedev.ri]

Passing-by remark:
Please would it be possible to consistently follow the general trend and prefix patch titles with appropriate [tags]?
This is a MLIR patch, so it would have been awesome to tag it as such.
It makes reading throught llvm-commits, and eventually git log, that much easier.

[chelini]

Introduce [mlir] tag in the commit message. Modify the constant_op_binder class as suggested by Alex. Specifically, introduce a class constructor with zero arguments which sets bind_value to nullptr. In the match method, insert an early exit if we match a constantOp, but we don't need to bind to any attribute value.

[mehdi_amini]

In D72397#1810061, @lebedev.ri wrote:

Passing-by remark:
Please would it be possible to consistently follow the general trend and prefix patch titles with appropriate [tags]?
This is a MLIR patch, so it would have been awesome to tag it as such.
It makes reading throught llvm-commits, and eventually git log, that much easier.

This looks to me like something better done by a tool wrapping git log and generating the tag before sending an email.

[rriddle]

mlir/include/mlir/IR/Matchers.h

71

Seems like this will be prone to subtle failure if we allow matching attribute kinds other than Attribute. I would check the cast(and optionally set 'bind_value') before returning true.

Another alternative would be to just have another matcher class(that inherits from this one) that contains a field for the attribute to bind.

[chelini]

Address River's comment by checking the cast before returning true and optionally binding to bind_value.

[rriddle]

mlir/include/mlir/IR/Matchers.h

73

Avoid the double return here:

if (bind_value)

*bind_value = attrT;

[chelini]

Address River's comment by avoiding the double return.

[chelini]

Could you please commit this patch? I don't have commit permission.

[mehdi_amini]

Thanks for the contribution.

Can you:

• join this group to enable CI testing of your revisions.
• As described in https://mlir.llvm.org/getting_started/DeveloperGuide/ Follow the git conventions for writing a commit message, in particular the first line is the “title”, it should be followed by an empty line and an optional description. (this changes landed titled "[mlir] m_Constant()" which isn't descriptive enough).

Thanks!

mlir/include/mlir/IR/Matchers.h

bool match(const Attribute &attr) {
     }
     return false;
   }
 };
 
 /// The matcher that matches a constant foldable operation that has no side
 /// effect, no operands and produces a single result.
 template <typename AttrT> struct constant_op_binder {
   AttrT *bind_value;
 
   /// Creates a matcher instance that binds the constant attribute value to
   /// bind_value if match succeeds.
   constant_op_binder(AttrT *bind_value) : bind_value(bind_value) {}
+  /// Creates a matcher instance that doesn't bind if match succeeds.
+  constant_op_binder() : bind_value(nullptr) {}
 
   bool match(Operation *op) {
     if (op->getNumOperands() > 0 || op->getNumResults() != 1)
       return false;
     if (!op->hasNoSideEffect())
       return false;
 
     SmallVector<OpFoldResult, 1> foldedOp;
     if (succeeded(op->fold(/*operands=*/llvm::None, foldedOp))) {
       if (auto attr = foldedOp.front().dyn_cast<Attribute>()) {
-        if ((*bind_value = attr.dyn_cast<AttrT>()))
+        if (auto attrT = attr.dyn_cast<AttrT>()) {
+          if (bind_value)
+            *bind_value = attrT;
           return true;
         }
       }
+    }
     return false;
   }
 };
 
 /// The matcher that matches a constant scalar / vector splat / tensor splat
 /// integer operation and binds the constant integer value.
 struct constant_int_op_binder {
   IntegerAttr::ValueType *bind_value;
bool match(Operation *op) {
     });
     return res;
   }
   std::tuple<OperandMatchers...> operandMatchers;
 };
 
 } // end namespace detail
 
+/// Matches a constant foldable operation.
+inline detail::constant_op_binder<Attribute> m_Constant() {
+  return detail::constant_op_binder<Attribute>();
+}
+
 /// Matches a value from a constant foldable operation and writes the value to
 /// bind_value.
 template <typename AttrT>
 inline detail::constant_op_binder<AttrT> m_Constant(AttrT *bind_value) {
   return detail::constant_op_binder<AttrT>(bind_value);
 }
 
 /// Matches a constant scalar / vector splat / tensor splat integer one.

mlir/lib/IR/Builders.cpp

LogicalResult OpBuilder::tryFold(Operation *op,
                                  SmallVectorImpl<Value> &results) {
   results.reserve(op->getNumResults());
   auto cleanupFailure = [&] {
     results.assign(op->result_begin(), op->result_end());
     return failure();
   };
 
   // If this operation is already a constant, there is nothing to do.
-  Attribute unused;
-  if (matchPattern(op, m_Constant(&unused)))
+  if (matchPattern(op, m_Constant()))
     return cleanupFailure();
 
   // Check to see if any operands to the operation is constant and whether
   // the operation knows how to constant fold itself.
   SmallVector<Attribute, 4> constOperands(op->getNumOperands());
   for (unsigned i = 0, e = op->getNumOperands(); i != e; ++i)
     matchPattern(op->getOperand(i), m_Constant(&constOperands[i]));
 

mlir/test/IR/test-matchers.mlir

func @test2(%a: f32) -> f32 {
   %0 = constant 1.0: f32
   %1 = addf %a, %0: f32
   %2 = mulf %a, %1: f32
   return %2: f32
 }
 
 // CHECK-LABEL: test2
 //       CHECK:   Pattern add(add(a, constant), a) matched and bound constant to: 1.000000e+00
+//       CHECK:   Pattern add(add(a, constant), a) matched

mlir/test/lib/IR/TestMatchers.cpp

static void test1(FuncOp f) {
   llvm::outs() << "Pattern mul(mul(a, *), add(c, b)) matched "
                << countMatches(f, p17) << " times\n";
 }
 
 void test2(FuncOp f) {
   auto a = m_Val(f.getArgument(0));
   FloatAttr floatAttr;
   auto p = m_Op<MulFOp>(a, m_Op<AddFOp>(a, m_Constant(&floatAttr)));
+  auto p1 = m_Op<MulFOp>(a, m_Op<AddFOp>(a, m_Constant()));
   // Last operation that is not the terminator.
   Operation *lastOp = f.getBody().front().back().getPrevNode();
   if (p.match(lastOp))
     llvm::outs()
         << "Pattern add(add(a, constant), a) matched and bound constant to: "
         << floatAttr.getValueAsDouble() << "\n";
+  if (p1.match(lastOp))
+    llvm::outs() << "Pattern add(add(a, constant), a) matched\n";
 }
 
 void TestMatchers::runOnFunction() {
   auto f = getFunction();
   llvm::outs() << f.getName() << "\n";
   if (f.getName() == "test1")
     test1(f);
   if (f.getName() == "test2")
     test2(f);
 }
 
 static PassRegistration<TestMatchers> pass("test-matchers",
                                            "Test C++ pattern matchers.");