[MLIR] Support for compatible Dialect Builders

[AlexEichenberger]

Summary

We have currently one builder for a few common StandardOps operation in ArithBuilder. In order to facilitate the creation of one dialect builder from another one, this patch create a superclass, DialectBuilder that stores the builder and location. Specialized builders inherit from the superclass their data and can focus to primarily providing methods to generate code.

Say in the future we may have a LinAlg or SCF builder, we would then be able to initialize these other specialized builders from a current one, e.g.

SCFBuilder createSCF(createArith);

In addition, I added a subtract method to the current ArithBuilder.

[rriddle]

What's the difference between this and just inheriting from ImplicitLocOpBuilder?

[AlexEichenberger]

In D105212#2850761, @rriddle wrote:

What's the difference between this and just inheriting from ImplicitLocOpBuilder?

My intent is to make the least amount of change to the ArithBuilder introduced by @nicolasvasilache. That struct is very lightweight and basically offers auto-completion and transparent argument typing.

@rriddle: you are suggesting that I make ArithBuilder a subclass of ImplicitLocOpBuilder? If that is your intend, I can prototype that solution as well.

[rriddle]

In D105212#2851019, @AlexEichenberger wrote:

In D105212#2850761, @rriddle wrote:

What's the difference between this and just inheriting from ImplicitLocOpBuilder?

My intent is to make the least amount of change to the ArithBuilder introduced by @nicolasvasilache. That struct is very lightweight and basically offers auto-completion and transparent argument typing.

@rriddle: you are suggesting that I make ArithBuilder a subclass of ImplicitLocOpBuilder? If that is your intend, I can prototype that solution as well.

Yeah, I would suggest prototyping other approaches to see feasibility there. We should only be adding new utility classes to IR/ when an existing solution doesn't work (can't be reasonably adapted to work). Right now, the benefits of an explicit DialectBuilder class aren't clear to me.

[AlexEichenberger]

Second prototype. There is one subtle difference: the old ArithBuilder had only a reference to a builder, so if the original builder was changed, it would change the one used by the ArithBuilder too. In this new version, ArithBuilder is a builder specialized for arithmetic ops, and thus changes to an original builder will not impact it's version.

[AlexEichenberger]

<format>

[stephenneuendorffer]

This seems reasonable to me.

[rriddle]

Can you update the patch description?

mlir/include/mlir/Dialect/StandardOps/Utils/Utils.h

 // utilities.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_DIALECT_STANDARDOPS_UTILS_UTILS_H
 #define MLIR_DIALECT_STANDARDOPS_UTILS_UTILS_H
 
 #include "mlir/Dialect/StandardOps/IR/Ops.h"
+#include "mlir/IR/ImplicitLocOpBuilder.h"
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/IR/Value.h"
 
 namespace mlir {
 
 /// Matches a ConstantIndexOp.
 detail::op_matcher<ConstantIndexOp> matchConstantIndex();
LogicalResult matchAndRewrite(OpType op,
     func(rewriter, op, newOp);
 
     return success();
   }
 };
 
 /// Helper struct to build simple arithmetic quantities with minimal type
 /// inference support.
-struct ArithBuilder {
-  ArithBuilder(OpBuilder &b, Location loc) : b(b), loc(loc) {}
+class ArithBuilder : public ImplicitLocOpBuilder {
+  public:
+  ArithBuilder(OpBuilder &b, Location loc) : ImplicitLocOpBuilder(loc, b) {}
+  ArithBuilder(ImplicitLocOpBuilder &lb) : ImplicitLocOpBuilder(lb) {}
 
   Value _and(Value lhs, Value rhs);
   Value add(Value lhs, Value rhs);
+  Value sub(Value lhs, Value rhs);
   Value mul(Value lhs, Value rhs);
   Value select(Value cmp, Value lhs, Value rhs);
   Value sgt(Value lhs, Value rhs);
   Value slt(Value lhs, Value rhs);
-
-private:
-  OpBuilder &b;
-  Location loc;
 };
 } // end namespace mlir
 
 #endif // MLIR_DIALECT_STANDARDOPS_UTILS_UTILS_H

mlir/lib/Dialect/StandardOps/Utils/Utils.cpp

for (unsigned pos = 0, e = shape.size(); pos < e && rank > 0; ++pos) {
     if (shape[pos] == 1) {
       dimsToProject.insert(pos);
       --rank;
     }
   }
 }
 
 Value ArithBuilder::_and(Value lhs, Value rhs) {
-  return b.create<AndOp>(loc, lhs, rhs);
+  return create<AndOp>(lhs, rhs);
 }
 Value ArithBuilder::add(Value lhs, Value rhs) {
   if (lhs.getType().isa<IntegerType>())
-    return b.create<AddIOp>(loc, lhs, rhs);
-  return b.create<AddFOp>(loc, lhs, rhs);
+    return create<AddIOp>(lhs, rhs);
+  return create<AddFOp>(lhs, rhs);
+}
+Value ArithBuilder::sub(Value lhs, Value rhs) {
+  if (lhs.getType().isa<IntegerType>())
+    return create<SubIOp>(lhs, rhs);
+  return create<SubFOp>(lhs, rhs);
 }
 Value ArithBuilder::mul(Value lhs, Value rhs) {
   if (lhs.getType().isa<IntegerType>())
-    return b.create<MulIOp>(loc, lhs, rhs);
-  return b.create<MulFOp>(loc, lhs, rhs);
+    return create<MulIOp>(lhs, rhs);
+  return create<MulFOp>(lhs, rhs);
 }
 Value ArithBuilder::sgt(Value lhs, Value rhs) {
   if (lhs.getType().isa<IndexType, IntegerType>())
-    return b.create<CmpIOp>(loc, CmpIPredicate::sgt, lhs, rhs);
-  return b.create<CmpFOp>(loc, CmpFPredicate::OGT, lhs, rhs);
+    return create<CmpIOp>(CmpIPredicate::sgt, lhs, rhs);
+  return create<CmpFOp>(CmpFPredicate::OGT, lhs, rhs);
 }
 Value ArithBuilder::slt(Value lhs, Value rhs) {
   if (lhs.getType().isa<IndexType, IntegerType>())
-    return b.create<CmpIOp>(loc, CmpIPredicate::slt, lhs, rhs);
-  return b.create<CmpFOp>(loc, CmpFPredicate::OLT, lhs, rhs);
+    return create<CmpIOp>(CmpIPredicate::slt, lhs, rhs);
+  return create<CmpFOp>(CmpFPredicate::OLT, lhs, rhs);
 }
 Value ArithBuilder::select(Value cmp, Value lhs, Value rhs) {
-  return b.create<SelectOp>(loc, cmp, lhs, rhs);
+  return create<SelectOp>(cmp, lhs, rhs);
 }