[mlir] Generate constructor in generic adaptors allowing construction from an op instance

[zero9178]

Summary

In essentially all occurrences of adaptor constructions in the codebase, an instance of the op is available and only a different value range is being used. Nevertheless, one had to perform the ritual of calling and pass getAttrDictionary(), getProperties and getRegions manually.

This patch changes that by teaching TableGen to generate a new constructor in the adaptor that is constructable using GenericAdaptor(valueRange, op). The (discardable) attr dictionary, properties and the regions are then taken directly from the passed op, with only the value range being taken from the first parameter.

This simplifies a lot of code and also guarantees that all the various getters of the adaptor work in all scenarios.

[mehdi_amini]

Nice cleanup!

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

3975

The condition seems reversed?

[zero9178]

Address review comments

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

3975

Nice catch, thanks! Fixed and added a test as well.

[mehdi_amini]

One thought: could it be a method on the Op class? getAdaptor(ValueRange values) { ... }.
It's templated with the ConcreteOp and should be able to handle everything. Seems slightly less intrusive than TableGen.

[zero9178]

In D157516#4575700, @mehdi_amini wrote:

One thought: could it be a method on the Op class? getAdaptor(ValueRange values) { ... }.
It's templated with the ConcreteOp and should be able to handle everything. Seems slightly less intrusive than TableGen.

I briefly tried it but I think its less ergonomic to use. What I had was essentially:

template <typename RangeT, typename ConcreteOpT = ConcreteType>
typename ConcreteOpT::GenericAdaptor<std::decay_t<RangeT>> getAdaptor(RangeT &&range) {

(ignore the second template parameter, its required for compilation but does not change semantics).

The first issue I encountered is that you could not construct a OpAdaptor as it is a subclass of GenericAdaptor<ValueRange>.
Second, I think the type deduction performed by C++ in this case leads to more surprising code. If you have an interface expecting GenericAdaptor<ValueRange> but pass in a ArrayRef<Value> into the above, you'll get a different adaptor type. One could fix this by having TableGen generate implicit constructors if the ranges are convertible but that would once again require TableGen changes. (To be fair, we maybe want this at some point anyways).

To counteract these two issues I also attempted:

template <typename Adaptor, class RangeT>
auto getAdaptor(RangeT &&range) {

This fixes both of my concerns but is still annoying to use in generic contexts: sourceOp.template getAdaptor<OpAdaptor>(operands) as opposed to just OpAdaptor(operands, sourceOp).
It is also a lot easier and faster to write a using for an adaptor class than it is to write a wrapper for getAdaptor.
You'd also have to write a static_assert within getAdaptor to make sure that Adaptor is an instance or subclass of ConcreteOp::GenericAdaptor to get the same strong typing as the current solution. I can definitely do that but it adds to the complexity.

I personally prefer the current solution for that reason. While I agree that writing C++ code is better than making TableGen generate code, I like the ergonomics of the TableGen approach more and think they are worth it.

[Mogball]

mlir/include/mlir/TableGen/Class.h

181

Does the API rely on deduplication of the template arguments? Otherwise, the SetVector doesn't make much sense.

192

Please default this to zero given that most methods won't have template params.

[zero9178]

Address review comments

mlir/include/mlir/TableGen/Class.h

181

Not really, this was just lazy copy paste programming from my side. This is required in other places, but definitely not here.

mlir/include/mlir/Conversion/LLVMCommon/Pattern.h

explicit ConvertOpToLLVMPattern(LLVMTypeConverter &typeConverter,
                                   PatternBenefit benefit = 1)
       : ConvertToLLVMPattern(SourceOp::getOperationName(),
                              &typeConverter.getContext(), typeConverter,
                              benefit) {}
 
   /// Wrappers around the RewritePattern methods that pass the derived op type.
   void rewrite(Operation *op, ArrayRef<Value> operands,
                ConversionPatternRewriter &rewriter) const final {
-    if constexpr (SourceOp::hasProperties())
-      return rewrite(cast<SourceOp>(op),
-                     OpAdaptor(operands, op->getDiscardableAttrDictionary(),
-                               cast<SourceOp>(op).getProperties()),
-                     rewriter);
-    rewrite(cast<SourceOp>(op),
-            OpAdaptor(operands, op->getDiscardableAttrDictionary()), rewriter);
+    rewrite(cast<SourceOp>(op), OpAdaptor(operands, cast<SourceOp>(op)),
   }
   LogicalResult match(Operation *op) const final {
     return match(cast<SourceOp>(op));
   }
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
-    if constexpr (SourceOp::hasProperties())
-      return matchAndRewrite(cast<SourceOp>(op),
-                             OpAdaptor(operands,
-                                       op->getDiscardableAttrDictionary(),
-                                       cast<SourceOp>(op).getProperties()),
-                             rewriter);
-    return matchAndRewrite(
-        cast<SourceOp>(op),
-        OpAdaptor(operands, op->getDiscardableAttrDictionary()), rewriter);
+                           OpAdaptor(operands, cast<SourceOp>(op)), rewriter);
   }
 
   /// Rewrite and Match methods that operate on the SourceOp type. These must be
   /// overridden by the derived pattern class.
   virtual LogicalResult match(SourceOp op) const {
     llvm_unreachable("must override match or matchAndRewrite");
   }
   virtual void rewrite(SourceOp op, OpAdaptor adaptor,

mlir/include/mlir/IR/OpDefinition.h

private:
   /// Return the result of folding a single result operation that defines a
   /// `fold` method.
   template <typename ConcreteOpT>
   static LogicalResult
   foldSingleResultHook(Operation *op, ArrayRef<Attribute> operands,
                        SmallVectorImpl<OpFoldResult> &results) {
     OpFoldResult result;
     if constexpr (has_fold_adaptor_single_result_v<ConcreteOpT>) {
-      if constexpr (hasProperties()) {
-        result = cast<ConcreteOpT>(op).fold(typename ConcreteOpT::FoldAdaptor(
-            operands, op->getDiscardableAttrDictionary(),
-            cast<ConcreteOpT>(op).getProperties(), op->getRegions()));
-      } else {
-        result = cast<ConcreteOpT>(op).fold(typename ConcreteOpT::FoldAdaptor(
-            operands, op->getDiscardableAttrDictionary(), {},
-            op->getRegions()));
-      }
+      result = cast<ConcreteOpT>(op).fold(
+          typename ConcreteOpT::FoldAdaptor(operands, cast<ConcreteOpT>(op)));
     } else {
       result = cast<ConcreteOpT>(op).fold(operands);
     }
 
     // If the fold failed or was in-place, try to fold the traits of the
     // operation.
     if (!result ||
         llvm::dyn_cast_if_present<Value>(result) == op->getResult(0)) {
       if (succeeded(op_definition_impl::foldTraits<Traits<ConcreteType>...>(
               op, operands, results)))
         return success();
       return success(static_cast<bool>(result));
     }
     results.push_back(result);
     return success();
   }
   /// Return the result of folding an operation that defines a `fold` method.
   template <typename ConcreteOpT>
   static LogicalResult foldHook(Operation *op, ArrayRef<Attribute> operands,
                                 SmallVectorImpl<OpFoldResult> &results) {
     auto result = LogicalResult::failure();
     if constexpr (has_fold_adaptor_v<ConcreteOpT>) {
-      if constexpr (hasProperties()) {
-        result = cast<ConcreteOpT>(op).fold(
-            typename ConcreteOpT::FoldAdaptor(
-                operands, op->getDiscardableAttrDictionary(),
-                cast<ConcreteOpT>(op).getProperties(), op->getRegions()),
-            results);
-      } else {
-        result = cast<ConcreteOpT>(op).fold(
-            typename ConcreteOpT::FoldAdaptor(
-                operands, op->getDiscardableAttrDictionary(), {},
-                op->getRegions()),
-            results);
-      }
-    } else {
+          typename ConcreteOpT::FoldAdaptor(operands, cast<ConcreteOpT>(op)),
       result = cast<ConcreteOpT>(op).fold(operands, results);
     }
 
     // If the fold failed or was in-place, try to fold the traits of the
     // operation.
     if (failed(result) || results.empty()) {
       if (succeeded(op_definition_impl::foldTraits<Traits<ConcreteType>...>(
               op, operands, results)))

mlir/include/mlir/TableGen/Class.h

public:
   /// Write the signature as part of a method declaration.
   void writeDeclTo(raw_indented_ostream &os) const;
 
   /// Write the signature as part of a method definition. `namePrefix` is to be
   /// prepended to the method name (typically namespaces for qualifying the
   /// method definition).
   void writeDefTo(raw_indented_ostream &os, StringRef namePrefix) const;
 
+  /// Write the template parameters of the signature.
+  void writeTemplateParamsTo(raw_indented_ostream &os) const;
+
+  /// Add a template parameter.
+  template <typename ParamT>
+  void addTemplateParam(ParamT param) {
+    templateParams.push_back(stringify(param));
+  }
+
+  /// Add a list of template parameters.
+  template <typename ContainerT>
+  void addTemplateParams(ContainerT &&container) {
+    templateParams.insert(std::begin(container), std::end(container));
+  }
+
 private:
   /// The method's C++ return type.
   std::string returnType;
   /// The method name.
   std::string methodName;
   /// The method's parameter list.
   MethodParameters parameters;
+  /// An optional list of template parameters.
+  SmallVector<std::string, 0> templateParams;
 };
 
 /// This class contains the body of a C++ method.
 class MethodBody {
 public:
   /// Create a method body, indicating whether it should be elided for methods
   /// that are declaration-only.
   MethodBody(bool declOnly);
public:
 
   /// Write the method declaration, including the definition if inline.
   void writeDeclTo(raw_indented_ostream &os) const override;
 
   /// Write the method definition. This is a no-op for inline methods.
   void writeDefTo(raw_indented_ostream &os,
                   StringRef namePrefix) const override;
 
+  /// Add a template parameter.
+  template <typename ParamT>
+  void addTemplateParam(ParamT param);
+
+  /// Add a list of template parameters.
+  template <typename ContainerT>
+  void addTemplateParams(ContainerT &&container);
+
 protected:
   /// A collection of method properties.
   Properties properties;
   /// The signature of the method.
   MethodSignature methodSignature;
   /// The body of the method, if it has one.
   MethodBody methodBody;
   /// Deprecation message if the method is deprecated.
operator|=(mlir::tblgen::Method::Properties &lhs,
            mlir::tblgen::Method::Properties rhs) {
   return lhs = mlir::tblgen::Method::Properties(static_cast<unsigned>(lhs) |
                                                 static_cast<unsigned>(rhs));
 }
 
 namespace mlir {
 namespace tblgen {
 
+template <typename ParamT>
+void Method::addTemplateParam(ParamT param) {
+  // Templates imply inline.
+  properties |= Method::Inline;
+  methodSignature.addTemplateParam(param);
+}
+
+template <typename ContainerT>
+void Method::addTemplateParams(ContainerT &&container) {
+  // Templates imply inline.
+  properties |= Method::Inline;
+  methodSignature.addTemplateParam(std::forward<ContainerT>(container));
+}
+
 /// This class describes a C++ parent class declaration.
 class ParentClass {
 public:
   /// Create a parent class with a class name and visibility.
   template <typename NameT>
   ParentClass(NameT &&name, Visibility visibility = Visibility::Public)
       : name(stringify(std::forward<NameT>(name))), visibility(visibility) {}
 

mlir/include/mlir/Transforms/DialectConversion.h

public:
   /// Wrappers around the ConversionPattern methods that pass the derived op
   /// type.
   LogicalResult match(Operation *op) const final {
     return match(cast<SourceOp>(op));
   }
   void rewrite(Operation *op, ArrayRef<Value> operands,
                ConversionPatternRewriter &rewriter) const final {
     auto sourceOp = cast<SourceOp>(op);
-    rewrite(sourceOp,
-            OpAdaptor(operands, op->getDiscardableAttrDictionary(),
-                      sourceOp.getProperties()),
-            rewriter);
+    rewrite(sourceOp, OpAdaptor(operands, sourceOp), rewriter);
   }
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
     auto sourceOp = cast<SourceOp>(op);
-    if constexpr (SourceOp::hasProperties())
-      return matchAndRewrite(sourceOp,
-                             OpAdaptor(operands,
-                                       op->getDiscardableAttrDictionary(),
-                                       sourceOp.getProperties()),
-                             rewriter);
-    return matchAndRewrite(
-        sourceOp, OpAdaptor(operands, op->getDiscardableAttrDictionary()),
-        rewriter);
+    return matchAndRewrite(sourceOp, OpAdaptor(operands, sourceOp), rewriter);
   }
 
   /// Rewrite and Match methods that operate on the SourceOp type. These must be
   /// overridden by the derived pattern class.
   virtual LogicalResult match(SourceOp op) const {
     llvm_unreachable("must override match or matchAndRewrite");
   }
   virtual void rewrite(SourceOp op, OpAdaptor adaptor,

mlir/include/mlir/Transforms/OneToNTypeConversion.h

class OpAdaptor
       : public SourceOp::template GenericAdaptor<ArrayRef<ValueRange>> {
   public:
     using RangeT = ArrayRef<ValueRange>;
     using BaseT = typename SourceOp::template GenericAdaptor<RangeT>;
     using Properties = typename SourceOp::template InferredProperties<SourceOp>;
 
     OpAdaptor(const OneToNTypeMapping *operandMapping,
               const OneToNTypeMapping *resultMapping,
-              const ValueRange *convertedOperands, RangeT values,
-              DictionaryAttr attrs = nullptr, Properties &properties = {},
-              RegionRange regions = {})
-        : BaseT(values, attrs, properties, regions),
-          operandMapping(operandMapping), resultMapping(resultMapping),
-          convertedOperands(convertedOperands) {}
+              const ValueRange *convertedOperands, RangeT values, SourceOp op)
+        : BaseT(values, op), operandMapping(operandMapping),
+          resultMapping(resultMapping), convertedOperands(convertedOperands) {}
 
     /// Get the type mapping of the original operands to the converted operands.
     const OneToNTypeMapping &getOperandMapping() const {
       return *operandMapping;
     }
 
     /// Get the type mapping of the original results to the converted results.
     const OneToNTypeMapping &getResultMapping() const { return *resultMapping; }
LogicalResult matchAndRewrite(Operation *op, OneToNPatternRewriter &rewriter,
                                 ValueRange convertedOperands) const final {
     // Wrap converted operands and type mappings into an adaptor.
     SmallVector<ValueRange> valueRanges;
     for (int64_t i = 0; i < op->getNumOperands(); i++) {
       auto values = operandMapping.getConvertedValues(convertedOperands, i);
       valueRanges.push_back(values);
     }
     OpAdaptor adaptor(&operandMapping, &resultMapping, &convertedOperands,
-                      valueRanges, op->getAttrDictionary(),
-                      cast<SourceOp>(op).getProperties(), op->getRegions());
+                      valueRanges, cast<SourceOp>(op));
 
     // Call overload implemented by the derived class.
     return matchAndRewrite(cast<SourceOp>(op), adaptor, rewriter);
   }
 };
 
 /// Applies the given set of patterns recursively on the given op and adds user
 /// materializations where necessary. The patterns are expected to be

mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp

struct ReallocOpLoweringBase : public AllocationOpLLVMLowering {
   virtual std::tuple<Value, Value>
   allocateBuffer(ConversionPatternRewriter &rewriter, Location loc,
                  Value sizeBytes, memref::ReallocOp op) const = 0;
 
   LogicalResult
   matchAndRewrite(Operation *op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const final {
     auto reallocOp = cast<memref::ReallocOp>(op);
-    return matchAndRewrite(reallocOp,
-                           OpAdaptor(operands,
-                                     op->getDiscardableAttrDictionary(),
-                                     reallocOp.getProperties()),
-                           rewriter);
+    return matchAndRewrite(reallocOp, OpAdaptor(operands, reallocOp), rewriter);
   }
 
   // A `realloc` is converted as follows:
   //   If new_size > old_size
   //     1. allocates a new buffer
   //     2. copies the content of the old buffer to the new buffer
   //     3. release the old buffer
   //     3. updates the buffer pointers in the memref descriptor

mlir/lib/Dialect/SCF/IR/SCF.cpp

assert((!index || index == getParentOp().getAfter().getRegionNumber()) &&
          "condition op can only exit the loop or branch to the after"
          "region");
   // Pass all operands except the condition to the successor region.
   return getArgsMutable();
 }
 
 void ConditionOp::getSuccessorRegions(
     ArrayRef<Attribute> operands, SmallVectorImpl<RegionSuccessor> &regions) {
-  FoldAdaptor adaptor(operands);
+  FoldAdaptor adaptor(operands, *this);
 
   WhileOp whileOp = getParentOp();
 
   // Condition can either lead to the after region or back to the parent op
   // depending on whether the condition is true or not.
   auto boolAttr = dyn_cast_or_null<BoolAttr>(adaptor.getCondition());
   if (!boolAttr || boolAttr.getValue())
     regions.emplace_back(&whileOp.getAfter(),
void IfOp::getSuccessorRegions(std::optional<unsigned> index,
   if (elseRegion->empty())
     regions.push_back(RegionSuccessor());
   else
     regions.push_back(RegionSuccessor(elseRegion));
 }
 
 void IfOp::getEntrySuccessorRegions(ArrayRef<Attribute> operands,
                                     SmallVectorImpl<RegionSuccessor> &regions) {
-  FoldAdaptor adaptor(operands);
+  FoldAdaptor adaptor(operands, *this);
   auto boolAttr = dyn_cast_or_null<BoolAttr>(adaptor.getCondition());
   if (!boolAttr || boolAttr.getValue())
     regions.emplace_back(&getThenRegion());
 
   // If the else region is empty, execution continues after the parent op.
   if (!boolAttr || !boolAttr.getValue()) {
     if (!getElseRegion().empty())
       regions.emplace_back(&getElseRegion());
void IndexSwitchOp::getSuccessorRegions(
   }
 
   llvm::copy(getRegions(), std::back_inserter(successors));
 }
 
 void IndexSwitchOp::getEntrySuccessorRegions(
     ArrayRef<Attribute> operands,
     SmallVectorImpl<RegionSuccessor> &successors) {
-  FoldAdaptor adaptor(operands);
+  FoldAdaptor adaptor(operands, *this);
 
   // If a constant was not provided, all regions are possible successors.
   auto arg = dyn_cast_or_null<IntegerAttr>(adaptor.getArg());
   if (!arg) {
     llvm::copy(getRegions(), std::back_inserter(successors));
     return;
   }
 

mlir/lib/TableGen/Class.cpp

 void MethodSignature::writeDefTo(raw_indented_ostream &os,
                                  StringRef namePrefix) const {
   os << returnType << getSpaceAfterType(returnType) << namePrefix
      << (namePrefix.empty() ? "" : "::") << methodName << "(";
   parameters.writeDefTo(os);
   os << ")";
 }
 
+void MethodSignature::writeTemplateParamsTo(
+    mlir::raw_indented_ostream &os) const {
+  if (templateParams.empty())
+    return;
+
+  os << "template <";
+  llvm::interleaveComma(templateParams, os,
+                        [&](StringRef param) { os << "typename " << param; });
+  os << ">\n";
+}
+
 //===----------------------------------------------------------------------===//
 // MethodBody definitions
 //===----------------------------------------------------------------------===//
 
 MethodBody::MethodBody(bool declOnly)
     : declOnly(declOnly), stringOs(body), os(stringOs) {}
 
 void MethodBody::writeTo(raw_indented_ostream &os) const {
   auto bodyRef = StringRef(body).drop_while([](char c) { return c == '\n'; });
   os << bodyRef;
   if (bodyRef.empty())
     return;
   if (bodyRef.back() != '\n')
     os << "\n";
 }
 
 //===----------------------------------------------------------------------===//
 // Method definitions
 //===----------------------------------------------------------------------===//
 
 void Method::writeDeclTo(raw_indented_ostream &os) const {
+  methodSignature.writeTemplateParamsTo(os);
   if (deprecationMessage) {
     os << "[[deprecated(\"";
     os.write_escaped(*deprecationMessage);
     os << "\")]]\n";
   }
   if (isStatic())
     os << "static ";
   if (properties & ConstexprValue)
void Method::writeDefTo(raw_indented_ostream &os, StringRef namePrefix) const {
   os << "}\n\n";
 }
 
 //===----------------------------------------------------------------------===//
 // Constructor definitions
 //===----------------------------------------------------------------------===//
 
 void Constructor::writeDeclTo(raw_indented_ostream &os) const {
+  methodSignature.writeTemplateParamsTo(os);
   if (properties & ConstexprValue)
     os << "constexpr ";
   methodSignature.writeDeclTo(os);
   if (!isInline()) {
     os << ";\n\n";
     return;
   }
   os << ' ';

mlir/test/mlir-tblgen/op-decl-and-defs.td

def NS_AOp : NS_Op<"a_op", [IsolatedFromAbove, IsolatedFromAbove]> {
   }];
 }
 
 // CHECK-LABEL: NS::AOp declarations
 
 // CHECK: namespace detail {
 // CHECK: class AOpGenericAdaptorBase {
 // CHECK: public:
+// CHECK:   AOpGenericAdaptorBase(AOp{{[[:space:]]}}
 // CHECK:   ::mlir::IntegerAttr getAttr1Attr();
 // CHECK:   uint32_t getAttr1();
 // CHECK:   ::mlir::FloatAttr getSomeAttr2Attr();
 // CHECK:   ::std::optional< ::llvm::APFloat > getSomeAttr2();
 // CHECK:   ::mlir::Region &getSomeRegion();
 // CHECK:   ::mlir::RegionRange getSomeRegions();
 // CHECK: };
 // CHECK: }

 // CHECK:   ::mlir::LogicalResult fold(FoldAdaptor adaptor, ::llvm::SmallVectorImpl<::mlir::OpFoldResult> &results);
 // CHECK:   // Display a graph for debugging purposes.
 // CHECK:   void displayGraph();
 // CHECK: };
 
 // DEFS-LABEL: NS::AOp definitions
 
 // DEFS: AOpGenericAdaptorBase::AOpGenericAdaptorBase(::mlir::DictionaryAttr attrs, const ::mlir::EmptyProperties &properties, ::mlir::RegionRange regions) : odsAttrs(attrs), odsRegions(regions)
+
+// Check that `getAttrDictionary()` is used when not using properties.
+
+// DEFS: AOpGenericAdaptorBase::AOpGenericAdaptorBase(AOp op)
+// DEFS-SAME: op->getAttrDictionary()
+// DEFS-SAME: p.getProperties()
+// DEFS-SAME: op->getRegions()
+
 // DEFS: ::mlir::RegionRange AOpGenericAdaptorBase::getSomeRegions()
 // DEFS-NEXT: return odsRegions.drop_front(1);
 // DEFS: ::mlir::RegionRange AOpGenericAdaptorBase::getRegions()
 
 // Check AttrSizedOperandSegments
 // ---
 
 def NS_AttrSizedOperandOp : NS_Op<"attr_sized_operands",
def NS_MOp : NS_Op<"op_with_single_result_and_fold_adaptor_fold", []> {
   let results = (outs AnyType:$res);
 
   let hasFolder = 1;
 }
 
 // CHECK-LABEL: class MOp :
 // CHECK: ::mlir::OpFoldResult fold(FoldAdaptor adaptor);
 
+def NS_NOp : NS_Op<"op_with_properties", []> {
+  let arguments = (ins Property<"unsigned">:$value);
+}
+
+// Check that `getDiscardableAttrDictionary()` is used with properties.
+
+// DEFS: NOpGenericAdaptorBase::NOpGenericAdaptorBase(NOp op) : NOpGenericAdaptorBase(
+// DEFS-SAME: op->getDiscardableAttrDictionary()
+// DEFS-SAME: op.getProperties()
+// DEFS-SAME: op->getRegions()
+
 // Test that type defs have the proper namespaces when used as a constraint.
 // ---
 
 def Test_Dialect2 : Dialect {
   let name = "test";
   let cppNamespace = "::mlir::dialect2";
 }
 def TestDialect2Type : TypeDef<Test_Dialect2, "Dialect2Type">;

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

if (useProperties) {
           "values, "
           "attrs, "
           "(properties ? *properties.as<::mlir::EmptyProperties *>() : "
           "::mlir::EmptyProperties{}), "
           "regions");
     }
   }
 
+  // Create constructors constructing the adaptor from an instance of the op.
+  // This takes the attributes, properties and regions from the op instance
+  // and the value range from the parameter.
+  {
+    // Base class is in the cpp file and can simply access the members of the op
+    // class to initialize the template independent fields.
+    auto *constructor = genericAdaptorBase.addConstructor(
+        MethodParameter(op.getCppClassName(), "op"));
+    constructor->addMemberInitializer(
+        genericAdaptorBase.getClassName(),
+        llvm::Twine(!useProperties ? "op->getAttrDictionary()"
+                                   : "op->getDiscardableAttrDictionary()") +
+            ", op.getProperties(), op->getRegions()");
+
+    // Generic adaptor is templated and therefore defined inline in the header.
+    // We cannot use the Op class here as it is an incomplete type (we have a
+    // circular reference between the two).
+    // Use a template trick to make the constructor be instantiated at call site
+    // when the op class is complete.
+    constructor = genericAdaptor.addConstructor(
+        MethodParameter("RangeT", "values"), MethodParameter("LateInst", "op"));
+    constructor->addTemplateParam("LateInst = " + op.getCppClassName());
+    constructor->addTemplateParam(
+        "= std::enable_if_t<std::is_same_v<LateInst, " + op.getCppClassName() +
+        ">>");
+    constructor->addMemberInitializer("Base", "op");
+    constructor->addMemberInitializer("odsOperands", "values");
+  }
+
   std::string sizeAttrInit;
   if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments")) {
     if (op.getDialect().usePropertiesForAttributes())
       sizeAttrInit =
           formatv(adapterSegmentSizeAttrInitCodeProperties,
                   llvm::formatv("getProperties().operandSegmentSizes"));
     else
       sizeAttrInit = formatv(adapterSegmentSizeAttrInitCode,
adaptor.addParent(ParentClass(genericAdaptorClassName))
       .addTemplateParam("::mlir::ValueRange");
   adaptor.declare<VisibilityDeclaration>(Visibility::Public);
   adaptor.declare<UsingDeclaration>(genericAdaptorClassName +
                                     "::" + genericAdaptorClassName);
   {
     // Constructor taking the Op as single parameter.
     auto *constructor =
         adaptor.addConstructor(MethodParameter(op.getCppClassName(), "op"));
-    constructor->addMemberInitializer(
-        adaptor.getClassName(), "op->getOperands(), op->getAttrDictionary(), "
-                                "op.getProperties(), op->getRegions()");
+    constructor->addMemberInitializer(genericAdaptorClassName,
+                                      "op->getOperands(), op");
   }
 
   // Add verification function.
   addVerification();
 
   genericAdaptorBase.finalize();
   genericAdaptor.finalize();
   adaptor.finalize();