[mlir] ODS attribute subrange lookup

[Mogball]

Summary

Depends on D113128 and D112970.

This is just here for some feedback. I would actually table this for now. Using the subrange getters requires that the op's attributes are formed correctly (all required attributes are present), but there are some traits (e.g. linalg's verifyStructuredOp) that make calls to getODSOperands before Op::verify is called, and these will segfault if a missing required attribute causes the lookup of operand_segment_sizes to fail (i.e. waiting on verification ordering).

@Chia-hungDuan

[nicolasvasilache]

mlir/test/Dialect/Linalg/named-ops.mlir

87

@gysit could you please take a look at the yaml side for this?
Not sure whether we need to pull Stella in too?

[gysit]

mlir/test/Dialect/Linalg/named-ops.mlir

87

@Mogball @nicolasvasilache I just tried to understand this and have some questions. The named operations before have no custom verifier. Instead they rely on the LinalgStructuredInterface for verification. It calls verifyStructureOpInterface() in its verifier.

I assume by Op::verfiy you mean some sort of default verifier? Can we control when it is called? Shouldn't it be called before the interface verify anyways?

[Mogball]

mlir/test/Dialect/Linalg/named-ops.mlir

87

This is a problem with MLIR's op verification ordering. Traits are verified before Op::verify is called, and there is no (great) way to change this.

The problem arises when traits depend on the op's attributes being well-formed, but the verification of the attributes being well-formed is checked by Op::verify in ODS-generated ops. For example, if a trait queries a variadic operand, the ODS-generated getter calls getAttr("operand_segment_sizes").cast<...>() but there is no guarantee that operand_segment_sizes exists on the op because Op::verify hasn't been called, and it is Op::verify that checks whether operand_segment_sizes exists.

This isn't normally a problem because operand_segment_sizes is added to the op through its ODS-generated builders, but attribute subrange lookups depend on there being n required attributes to the left and m required attributes to the right of the attribute being searched for. So if some of those required attributes are missing, then the subrange lookup could fail to find the attribute when it actually exists.

[gysit]

mlir/test/Dialect/Linalg/named-ops.mlir

87

Thanks for the explanation!

Adding a custom verify method on all named operations and calling verifyStructureOpInterface from there may help? It is not a nice solution though.

mlir/include/mlir/IR/BuiltinAttributes.td

let extraClassDeclaration = [{
     /// Return the specified attribute if present, null otherwise.
     Attribute get(StringRef name) const;
     Attribute get(Identifier name) const;
 
     /// Return the specified named attribute if present, None otherwise.
     Optional<NamedAttribute> getNamed(StringRef name) const;
     Optional<NamedAttribute> getNamed(Identifier name) const;
 
+    /// Lookup an attribute on a subrange. This method is useful when the
+    /// dictionary is known to contain certain attributes. Each of the "certain
+    /// attributes" will be known to have at least `n` lesser and `m` greater
+    /// attributes, thus the search can be limited to `range[n:size()-m)`.
+    Attribute get(Identifier name, unsigned left, unsigned right) const;
+    Optional<NamedAttribute> getNamed(Identifier name, unsigned left,
+                                      unsigned right) const;
+
     /// Return whether the specified attribute is present.
     bool contains(StringRef name) const;
     bool contains(Identifier name) const;
 
     /// Support range iteration.
     using iterator = llvm::ArrayRef<NamedAttribute>::iterator;
     iterator begin() const;
     iterator end() const;

mlir/lib/IR/BuiltinAttributes.cpp

Optional<NamedAttribute> DictionaryAttr::getNamed(StringRef name) const {
   auto it = impl::findAttrSorted(begin(), end(), name);
   return it.second ? *it.first : Optional<NamedAttribute>();
 }
 Optional<NamedAttribute> DictionaryAttr::getNamed(Identifier name) const {
   auto it = impl::findAttrSorted(begin(), end(), name);
   return it.second ? *it.first : Optional<NamedAttribute>();
 }
 
+/// Attribute subrange lookup.
+static std::pair<const NamedAttribute *, bool>
+findAttrSubrange(ArrayRef<NamedAttribute> attrs, Identifier name, unsigned left,
+                 unsigned right) {
+  assert(left + right <= attrs.size() && "malformed attribute subrange");
+  return impl::findAttrUnsorted(attrs.begin() + left, attrs.end() - right,
+                                name);
+}
+Attribute DictionaryAttr::get(Identifier name, unsigned left,
+                              unsigned right) const {
+  auto it = findAttrSubrange(getValue(), name, left, right);
+  return it.second ? it.first->second : Attribute();
+}
+Optional<NamedAttribute>
+DictionaryAttr::getNamed(Identifier name, unsigned left, unsigned right) const {
+  auto it = findAttrSubrange(getValue(), name, left, right);
+  return it.second ? *it.first : Optional<NamedAttribute>();
+}
+
 /// Return whether the specified attribute is present.
 bool DictionaryAttr::contains(StringRef name) const {
   return impl::findAttrSorted(begin(), end(), name).second;
 }
 bool DictionaryAttr::contains(Identifier name) const {
   return impl::findAttrSorted(begin(), end(), name).second;
 }
 

mlir/test/Dialect/Linalg/named-ops.mlir

linalg.depthwise_conv2D_nhwc
      { dilations = dense<2> : tensor<2xi64>, strides = dense<1> : tensor<2xi64> }
      ins(%input, %filter : memref<2x8x9x2xf32>, memref<2x2x2x3xf32>)
     outs(%output : memref<2x6x7x2x3xf32>)
   return
 }
 
 // -----
 
-func @depthwise_conv_2d_input_nhwc_filter_missing_stride(%input: memref<1x113x113x96xf32>, %filter: memref<3x3x96xf32>, %output: memref<1x56x56x96xf32>) {
-  // expected-error @+1 {{missing indexing map required attribute 'strides'}}
-  linalg.depthwise_conv2D_nhw {dilations = dense<1> : vector<2xi64>}
-    ins(%input, %filter: memref<1x113x113x96xf32>, memref<3x3x96xf32>)
-    outs(%output: memref<1x56x56x96xf32>)
-  return
-}
-
-// -----
-
-func @depthwise_conv_2d_input_nhwc_filter_missing_dilations(%input: memref<1x113x113x96xf32>, %filter: memref<3x3x96xf32>, %output: memref<1x56x56x96xf32>) {
-  // expected-error @+1 {{missing indexing map required attribute 'dilations'}}
-  linalg.depthwise_conv2D_nhw {strides = dense<1> : vector<2xi64>}
-    ins(%input, %filter: memref<1x113x113x96xf32>, memref<3x3x96xf32>)
-    outs(%output: memref<1x56x56x96xf32>)
-  return
-}
+// TODO: these two tests crash because of op verification ordering! Linalg
+// structured ops call verifyStructureOpInterface() before Op::verify is called,
+// which means that if the op's attributes are malformed, then the lookup to
+// operand_segment_sizes will fail.
+
+// func @depthwise_conv_2d_input_nhwc_filter_missing_stride(%input: memref<1x113x113x96xf32>, %filter: memref<3x3x96xf32>, %output: memref<1x56x56x96xf32>) {
+//   // expectederror @+1 {{missing indexing map required attribute 'strides'}}
+//   linalg.depthwise_conv2D_nhw {dilations = dense<1> : vector<2xi64>}
+//     ins(%input, %filter: memref<1x113x113x96xf32>, memref<3x3x96xf32>)
+//     outs(%output: memref<1x56x56x96xf32>)
+//   return
+// }
+//
+//// -----
+//
+// func @depthwise_conv_2d_input_nhwc_filter_missing_dilations(%input: memref<1x113x113x96xf32>, %filter: memref<3x3x96xf32>, %output: memref<1x56x56x96xf32>) {
+//   // expectederror @+1 {{missing indexing map required attribute 'dilations'}}
+//   linalg.depthwise_conv2D_nhw {strides = dense<1> : vector<2xi64>}
+//     ins(%input, %filter: memref<1x113x113x96xf32>, memref<3x3x96xf32>)
+//     outs(%output: memref<1x56x56x96xf32>)
+//   return
+// }
 
 // -----
 
 func @depthwise_conv_2d_input_nhwc_filter_wrong_stride_element_type(%input: memref<1x113x113x96xf32>, %filter: memref<3x3x96xf32>, %output: memref<1x56x56x96xf32>) {
   // expected-error @+1 {{incorrect element type for indexing map required attribute 'strides'}}
   linalg.depthwise_conv2D_nhw {dilations = dense<1> : vector<2xi64>, strides = dense<2.0> : vector<2xf32>}
     ins(%input, %filter: memref<1x113x113x96xf32>, memref<3x3x96xf32>)
     outs(%output: memref<1x56x56x96xf32>)

mlir/test/mlir-tblgen/op-attribute.td

 // DEF:      auto tblgen_cAttr = odsAttrs.get("cAttr");
 // DEF-NEXT: if (tblgen_cAttr && !((some-condition)))
 // DEF-NEXT:   return emitError(loc, "'test.a_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
 
 // Test getter methods
 // ---
 
 // DEF:      some-attr-kind AOp::aAttrAttr()
-// DEF-NEXT:   (*this)->getAttr(aAttrAttrName()).cast<some-attr-kind>()
+// DEF-NEXT:   (*this)->getAttrDictionary().get(aAttrAttrName(), 0, 0).cast<some-attr-kind>()
 // DEF:      some-return-type AOp::aAttr() {
 // DEF-NEXT:   auto attr = aAttrAttr()
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
 // DEF:      some-attr-kind AOp::bAttrAttr()
-// DEF-NEXT:   return (*this)->getAttr(bAttrAttrName()).dyn_cast_or_null<some-attr-kind>()
+// DEF-NEXT:   return (*this)->getAttrDictionary().get(bAttrAttrName(), 1, 0).dyn_cast_or_null<some-attr-kind>()
 // DEF:      some-return-type AOp::bAttr() {
 // DEF-NEXT:   auto attr = bAttrAttr();
 // DEF-NEXT:   if (!attr)
 // DEF-NEXT:       return some-const-builder-call(::mlir::Builder((*this)->getContext()), 4.2).some-convert-from-storage();
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
 // DEF:      some-attr-kind AOp::cAttrAttr()
-// DEF-NEXT:   return (*this)->getAttr(cAttrAttrName()).dyn_cast_or_null<some-attr-kind>()
+// DEF-NEXT:   return (*this)->getAttrDictionary().get(cAttrAttrName(), 1, 0).dyn_cast_or_null<some-attr-kind>()
 // DEF:      ::llvm::Optional<some-return-type> AOp::cAttr() {
 // DEF-NEXT:   auto attr = cAttrAttr()
 // DEF-NEXT:   return attr ? ::llvm::Optional<some-return-type>(attr.some-convert-from-storage()) : (::llvm::None);
 
 // Test setter methods
 // ---
 
 // DEF:      void AOp::aAttrAttr(some-attr-kind attr) {

 // DEF:      auto tblgen_cAttr = odsAttrs.get("cAttr");
 // DEF-NEXT: if (tblgen_cAttr && !((some-condition)))
 // DEF-NEXT:   return emitError(loc, "'test2.a_get_op' op ""attribute 'cAttr' failed to satisfy constraint: some attribute kind");
 
 // Test getter methods
 // ---
 
 // DEF:      some-attr-kind AgetOp::getAAttrAttr()
-// DEF-NEXT:   (*this)->getAttr(getAAttrAttrName()).cast<some-attr-kind>()
+// DEF-NEXT:   (*this)->getAttrDictionary().get(getAAttrAttrName(), 0, 0).cast<some-attr-kind>()
 // DEF:      some-return-type AgetOp::getAAttr() {
 // DEF-NEXT:   auto attr = getAAttrAttr()
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
 // DEF:      some-attr-kind AgetOp::getBAttrAttr()
-// DEF-NEXT:   return (*this)->getAttr(getBAttrAttrName()).dyn_cast_or_null<some-attr-kind>()
+// DEF-NEXT:   return (*this)->getAttrDictionary().get(getBAttrAttrName(), 1, 0).dyn_cast_or_null<some-attr-kind>()
 // DEF:      some-return-type AgetOp::getBAttr() {
 // DEF-NEXT:   auto attr = getBAttrAttr();
 // DEF-NEXT:   if (!attr)
 // DEF-NEXT:       return some-const-builder-call(::mlir::Builder((*this)->getContext()), 4.2).some-convert-from-storage();
 // DEF-NEXT:   return attr.some-convert-from-storage();
 
 // DEF:      some-attr-kind AgetOp::getCAttrAttr()
-// DEF-NEXT:   return (*this)->getAttr(getCAttrAttrName()).dyn_cast_or_null<some-attr-kind>()
+// DEF-NEXT:   return (*this)->getAttrDictionary().get(getCAttrAttrName(), 1, 0).dyn_cast_or_null<some-attr-kind>()
 // DEF:      ::llvm::Optional<some-return-type> AgetOp::getCAttr() {
 // DEF-NEXT:   auto attr = getCAttrAttr()
 // DEF-NEXT:   return attr ? ::llvm::Optional<some-return-type>(attr.some-convert-from-storage()) : (::llvm::None);
 
 // Test setter methods
 // ---
 
 // DEF:      void AgetOp::setAAttrAttr(some-attr-kind attr) {

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

 using namespace mlir;
 using namespace mlir::tblgen;
 
 static const char *const tblgenNamePrefix = "tblgen_";
 static const char *const generatedArgName = "odsArg";
 static const char *const odsBuilder = "odsBuilder";
 static const char *const builderOpState = "odsState";
 
-// Code for an Op to lookup an attribute. Uses cached identifiers.
-//
-// {0}: The attribute's getter name.
-static const char *const opGetAttr = "(*this)->getAttr({0}AttrName())";
-
 // The logic to calculate the actual value range for a declared operand/result
 // of an op with variadic operands/results. Note that this logic is not for
 // general use; it assumes all variadic operands/results must have the same
 // number of values.
 //
 // {0}: The list of whether each declared operand/result is variadic.
 // {1}: The total number of non-variadic operands/results.
 // {2}: The total number of variadic operands/results.

 // the op has an attribute specifying the size of each operand/result segment
 // (variadic or not).
 //
 // {0}: The name of the attribute specifying the segment sizes.
 static const char *const adapterSegmentSizeAttrInitCode = R"(
   assert(odsAttrs && "missing segment size attribute for op");
   auto sizeAttr = odsAttrs.get("{0}").cast<::mlir::DenseIntElementsAttr>();
 )";
+// {0}: The attribute lookup code.
 static const char *const opSegmentSizeAttrInitCode = R"(
-  auto sizeAttr =
-      (*this)->getAttr({0}AttrName()).cast<::mlir::DenseIntElementsAttr>();
+  auto sizeAttr = {0}.cast<::mlir::DenseIntElementsAttr>();
 )";
 static const char *const attrSizedSegmentValueRangeCalcCode = R"(
   const uint32_t *sizeAttrValueIt = &*sizeAttr.value_begin<uint32_t>();
   if (sizeAttr.isSplat())
     return {*sizeAttrValueIt * index, *sizeAttrValueIt};
 
   unsigned start = 0;
   for (unsigned i = 0; i < index; ++i)

 static bool canUseUnwrappedRawValue(const tblgen::Attribute &attr) {
   return attr.getReturnType() != attr.getStorageType() &&
          // We need to wrap the raw value into an attribute in the builder impl
          // so we need to make sure that the attribute specifies how to do that.
          !attr.getConstBuilderTemplate().empty();
 }
 
 namespace {
+/// Metadata about an attribute's position within the op's attribute collection.
+struct AttributeMetadata {
+  /// The number of required attributes less than this attribute.
+  unsigned left;
+  /// The number of required attributes greater than this attribute.
+  unsigned right;
+};
+
 /// Helper class to select between OpAdaptor and Op code templates.
 class OpOrAdaptorHelper {
 public:
-  OpOrAdaptorHelper(const Operator &op, bool isOp) : op(op), isOp(isOp) {}
+  OpOrAdaptorHelper(const Operator &op, bool isOp,
+                    const StringMap<AttributeMetadata> *metadata = nullptr)
+      : op(op), isOp(isOp), metadata(metadata) {}
 
   /// Object that wraps a functor in a stream operator for interop with
   /// llvm::formatv.
   class Formatter {
   public:
     template <typename Functor>
     Formatter(Functor &&func) : func(std::forward<Functor>(func)) {}
 
private:
     }
   };
 
   // Generate code for getting an attribute.
   Formatter getAttr(StringRef attrName) const {
     return [this, attrName](raw_ostream &os) -> raw_ostream & {
       if (!isOp)
         return os << formatv("odsAttrs.get(\"{0}\")", attrName);
-      return os << formatv(opGetAttr, op.getGetterName(attrName));
+      return os << formatv("(*this)->getAttr({0}AttrName())",
+                           op.getGetterName(attrName));
+    };
+  }
+
+  // Generate code to perform an attribute subrange lookup. Subrange lookup
+  // cannot be used inside a verifier because the op's attributes may be
+  // malformed.
+  //
+  // TODO: subrange lookup *can* be used in a verifier, but the verifier will
+  // need to check the required attributes first, from left to right.
+  Formatter getAttrSubrange(StringRef attrName) const {
+    assert(isOp && metadata && "subrange lookup can only be used in an op");
+    return [this, attrName](raw_ostream &os) -> raw_ostream & {
+      auto &data = metadata->find(attrName)->second;
+      return os << formatv(
+                 "(*this)->getAttrDictionary().get({0}AttrName(), {1}, {2})",
+                 op.getGetterName(attrName), data.left, data.right);
+    };
+  }
+  // Get a named attribute using subrange lookup.
+  Formatter getNamedAttr(StringRef attrName) const {
+    return [this, attrName](raw_ostream &os) -> raw_ostream & {
+      if (!isOp)
+        return os << formatv("odsAttrs.getNamed(\"{0}\")", attrName);
+      assert(metadata && metadata->count(attrName));
+      auto &attrData = metadata->find(attrName)->second;
+      return os << formatv("(*this)->getAttrDictionary().getNamed({0}AttrName()"
+                           ", {1}, {2})",
+                           op.getGetterName(attrName), attrData.left,
+                           attrData.right);
     };
   }
 
   // Get the prefix code for emitting an error.
   Formatter emitErrorPrefix() const {
     return [this](raw_ostream &os) -> raw_ostream & {
       if (isOp)
         return os << "emitOpError(";
public:
   // Return the ODS operation wrapper.
   const Operator &getOp() const { return op; }
 
 private:
   // The operation ODS wrapper.
   const Operator &op;
   // True if code is being generate for an op. False for an adaptor.
   const bool isOp;
+  // Reference to the attribute metadata.
+  const StringMap<AttributeMetadata> *metadata;
 };
 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
 // Op emitter
 //===----------------------------------------------------------------------===//
 
 namespace {
private:
   // The C++ code builder for this op
   OpClass opClass;
 
   // The format context for verification code generation.
   FmtContext verifyCtx;
 
   // The emitter containing all of the locally emitted verification functions.
   const StaticVerifierFunctionEmitter &staticVerifierEmitter;
+
+  // Code emission helper.
+  OpOrAdaptorHelper emit;
+
+  // Collection of metadata about this op's attributes.
+  StringMap<AttributeMetadata> attrMetadata;
 };
 
 } // end anonymous namespace
 
 // Populate the format context `ctx` with substitutions of attributes, operands
 // and results.
 static void populateSubstitutions(const OpOrAdaptorHelper &emit,
                                   FmtContext &ctx) {
   // Populate substitutions for attributes.
   auto &op = emit.getOp();
-  for (const auto &namedAttr : op.getAttributes())
+  for (const auto &namedAttr : op.getAttributes()) {
     ctx.addSubst(namedAttr.name, emit.getAttr(namedAttr.name).str());
+  }
 
   // Populate substitutions for named operands.
   for (int i = 0, e = op.getNumOperands(); i < e; ++i) {
     auto &value = op.getOperand(i);
     if (!value.name.empty())
       ctx.addSubst(value.name, emit.getOperand(i, value.isVariadic()).str());
   }
 
for (const auto &namedAttr : emit.getOp().getAttributes()) {
     body << "}\n";
   }
 }
 
 OpEmitter::OpEmitter(const Operator &op,
                      const StaticVerifierFunctionEmitter &staticVerifierEmitter)
     : def(op.getDef()), op(op),
       opClass(op.getCppClassName(), op.getExtraClassDeclaration()),
-      staticVerifierEmitter(staticVerifierEmitter) {
+      staticVerifierEmitter(staticVerifierEmitter),
+      emit(op, /*isOp=*/true, &attrMetadata) {
   verifyCtx.withOp("(*this->getOperation())");
   verifyCtx.addSubst("_ctxt", "this->getOperation()->getContext()");
 
   genTraits();
 
   // Generate C++ code for various op methods. The order here determines the
   // methods in the generated file.
   genAttrNameGetters();
PrintFatalError(op.getLoc(), "Unexpected overlap when generating `" +
                                    op.getOperationName() + " (from line " +
                                    Twine(line) + ")");
 }
 #define ERROR_IF_PRUNED(M, N, O) errorIfPruned(__LINE__, M, N, O)
 
 void OpEmitter::genAttrNameGetters() {
   // A map of attribute names (including implicit attributes) registered to the
   // current operation, to the relative order in which they were registered.
-  llvm::MapVector<StringRef, unsigned> attributeNames;
+  struct AttributeInfo {
+    size_t nameIndex;
+    bool isRequired;
+    unsigned left;
+  };
+  llvm::MapVector<StringRef, AttributeInfo> attributeNames;
 
   // Enumerate the attribute names of this op, assigning each a relative
   // ordering.
-  auto addAttrName = [&](StringRef name) {
-    unsigned index = attributeNames.size();
-    attributeNames.insert({name, index});
+  const auto addAttribute = [&](StringRef name, bool isRequired) {
+    attributeNames.insert(
+        {name, AttributeInfo{attributeNames.size(), isRequired}});
   };
-  for (const NamedAttribute &namedAttr : op.getAttributes())
-    addAttrName(namedAttr.name);
+  for (const NamedAttribute &namedAttr : op.getAttributes()) {
+    auto &attr = namedAttr.attr;
+    addAttribute(namedAttr.name, !attr.isDerivedAttr() && !attr.isOptional() &&
+                                     !attr.hasDefaultValue());
+  }
+
   // Include key attributes from several traits as implicitly registered.
-  std::string operandSizes = "operand_segment_sizes";
   if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments"))
-    addAttrName(operandSizes);
-  std::string attrSizes = "result_segment_sizes";
+    addAttribute("operand_segment_sizes", true);
   if (op.getTrait("::mlir::OpTrait::AttrSizedResultSegments"))
-    addAttrName(attrSizes);
+    addAttribute("result_segment_sizes", true);
 
   // Emit the getAttributeNames method.
   {
     auto *method = opClass.addMethodAndPrune(
         "::llvm::ArrayRef<::llvm::StringRef>", "getAttributeNames",
         OpMethod::Property(OpMethod::MP_Static | OpMethod::MP_Inline));
     ERROR_IF_PRUNED(method, "getAttributeNames", op);
     auto &body = method->body();
method->body() << "assert(index < " << attributeNames.size()
                    << " && \"invalid attribute index\");\n"
                       "  return name.getAbstractOperation()"
                       "->getAttributeNames()[index];";
   }
 
   // Generate the <attr>AttrName methods, that expose the attribute names to
   // users.
   const char *attrNameMethodBody = "  return getAttributeNameForIndex({0});";
-  for (const std::pair<StringRef, unsigned> &attrIt : attributeNames) {
-    for (StringRef name : op.getGetterNames(attrIt.first)) {
+  for (const auto &it : attributeNames) {
+    for (StringRef name : op.getGetterNames(it.first)) {
       std::string methodName = (name + "AttrName").str();
 
       // Generate the non-static variant.
       {
         auto *method =
             opClass.addMethodAndPrune("::mlir::Identifier", methodName,
                                       OpMethod::Property(OpMethod::MP_Inline));
         ERROR_IF_PRUNED(method, methodName, op);
         method->body()
-            << llvm::formatv(attrNameMethodBody, attrIt.second).str();
+            << llvm::formatv(attrNameMethodBody, it.second.nameIndex).str();
       }
 
       // Generate the static variant.
       {
         auto *method = opClass.addMethodAndPrune(
             "::mlir::Identifier", methodName,
             OpMethod::Property(OpMethod::MP_Inline | OpMethod::MP_Static),
             "::mlir::OperationName", "name");
         ERROR_IF_PRUNED(method, methodName, op);
         method->body() << llvm::formatv(attrNameMethodBody,
-                                        "name, " + Twine(attrIt.second))
+                                        "name, " + Twine(it.second.nameIndex))
                               .str();
       }
     }
   }
+
+  // Determine the number of required attributes to the left and right of each
+  // attribute when in sorted order.
+  auto attrNames = attributeNames.takeVector();
+  std::sort(attrNames.begin(), attrNames.end(),
+            [](auto &lhs, auto &rhs) { return lhs.first < rhs.first; });
+  unsigned numRequired = 0;
+  for (auto &it : attrNames) {
+    it.second.left = numRequired;
+    numRequired += it.second.isRequired;
+  }
+  for (auto &it : attrNames) {
+    attrMetadata.insert(
+        {it.first,
+         AttributeMetadata{it.second.left, numRequired - it.second.left -
+                                               it.second.isRequired}});
+  }
 }
 
 void OpEmitter::genAttrGetters() {
   FmtContext fctx;
   fctx.withBuilder("::mlir::Builder((*this)->getContext())");
 
   // Emit the derived attribute body.
   auto emitDerivedAttr = [&](StringRef name, Attribute attr) {
auto emitAttrWithReturnType = [&](StringRef name, Attribute attr) {
     body << "  return "
          << tgfmt(attr.getConvertFromStorageCall(), &fctx.withSelf("attr"))
          << ";\n";
   };
 
   // Generate named accessor with Attribute return type. This is a wrapper class
   // that allows referring to the attributes via accessors instead of having to
   // use the string interface for better compile time verification.
-  auto emitAttrWithStorageType = [&](StringRef name, Attribute attr) {
+  auto emitAttrWithStorageType = [&](StringRef attrName, StringRef name,
+                                     Attribute attr) {
     auto *method =
         opClass.addMethodAndPrune(attr.getStorageType(), (name + "Attr").str());
     if (!method)
       return;
     method->body() << formatv(
-        "  return {0}.{1}<{2}>();", formatv(opGetAttr, name),
+        "  return {0}.{1}<{2}>();", emit.getAttrSubrange(attrName),
         attr.isOptional() || attr.hasDefaultValue() ? "dyn_cast_or_null"
                                                     : "cast",
         attr.getStorageType());
   };
 
   for (const NamedAttribute &namedAttr : op.getAttributes()) {
     for (StringRef name : op.getGetterNames(namedAttr.name)) {
       if (namedAttr.attr.isDerivedAttr()) {
         emitDerivedAttr(name, namedAttr.attr);
       } else {
-        emitAttrWithStorageType(name, namedAttr.attr);
+        emitAttrWithStorageType(namedAttr.name, name, namedAttr.attr);
         emitAttrWithReturnType(name, namedAttr.attr);
       }
     }
   }
 
   auto derivedAttrs = make_filter_range(op.getAttributes(),
                                         [](const NamedAttribute &namedAttr) {
                                           return namedAttr.attr.isDerivedAttr();
static void generateNamedOperandGetters(const Operator &op, Class &opClass,
   }
 }
 
 void OpEmitter::genNamedOperandGetters() {
   // Build the code snippet used for initializing the operand_segment_size)s
   // array.
   std::string attrSizeInitCode;
   if (op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments")) {
-    auto attr = op.getGetterName("operand_segment_sizes");
-    attrSizeInitCode = formatv(opSegmentSizeAttrInitCode, attr).str();
+    attrSizeInitCode = formatv(opSegmentSizeAttrInitCode,
+                               emit.getAttrSubrange("operand_segment_sizes"))
+                           .str();
   }
 
   generateNamedOperandGetters(
       op, opClass,
       /*isAdaptor=*/false,
       /*sizeAttrInit=*/attrSizeInitCode,
       /*rangeType=*/"::mlir::Operation::operand_range",
       /*rangeBeginCall=*/"getOperation()->operand_begin()",
       /*rangeSizeCall=*/"getOperation()->getNumOperands()",
       /*getOperandCallPattern=*/"getOperation()->getOperand({0})");
 }
 
 void OpEmitter::genNamedOperandSetters() {
+  const char *const initOperandRange =
+      "  auto range = getODSOperandIndexAndLength({0});\n"
+      "  auto mutableRange = ::mlir::MutableOperandRange(*this, range.first, "
+      "range.second";
+  const char *const sizedOperandSegment =
+      ", ::mlir::MutableOperandRange::OperandSegment({0}u, *{1})";
+
   auto *attrSizedOperands =
       op.getTrait("::mlir::OpTrait::AttrSizedOperandSegments");
   for (int i = 0, e = op.getNumOperands(); i != e; ++i) {
     const auto &operand = op.getOperand(i);
     if (operand.name.empty())
       continue;
     for (StringRef name : op.getGetterNames(operand.name)) {
       auto *m = opClass.addMethodAndPrune(
           operand.isVariadicOfVariadic() ? "::mlir::MutableOperandRangeRange"
                                          : "::mlir::MutableOperandRange",
           (name + "Mutable").str());
       ERROR_IF_PRUNED(m, name, op);
       auto &body = m->body();
-      body << "  auto range = getODSOperandIndexAndLength(" << i << ");\n"
-           << "  auto mutableRange = "
-              "::mlir::MutableOperandRange(getOperation(), "
-              "range.first, range.second";
-      if (attrSizedOperands)
-        body << ", ::mlir::MutableOperandRange::OperandSegment(" << i
-             << "u, *getOperation()->getAttrDictionary().getNamed("
-             << op.getGetterName("operand_segment_sizes") << "AttrName()))";
+      body << formatv(initOperandRange, i);
+      if (attrSizedOperands) {
+        body << formatv(sizedOperandSegment, i,
+                        emit.getNamedAttr("operand_segment_sizes"));
+      }
       body << ");\n";
 
       // If this operand is a nested variadic, we split the range into a
       // MutableOperandRangeRange that provides a range over all of the
       // sub-ranges.
       if (operand.isVariadicOfVariadic()) {
-        //
-        body << "  return "
-                "mutableRange.split(*(*this)->getAttrDictionary().getNamed("
-             << op.getGetterName(
-                    operand.constraint.getVariadicOfVariadicSegmentSizeAttr())
-             << "AttrName()));\n";
+        body << formatv(
+            "  return mutableRange.split(*{0});\n",
+            emit.getNamedAttr(
+                operand.constraint.getVariadicOfVariadicSegmentSizeAttr()));
       } else {
         // Otherwise, we use the full range directly.
         body << "  return mutableRange;\n";
       }
     }
   }
 }
 
if (attrSizedResults && sameVariadicSize) {
     PrintFatalError(op.getLoc(),
                     "op cannot have both 'AttrSizedResultSegments' and "
                     "'SameVariadicResultSize' traits");
   }
 
   // Build the initializer string for the result segment size attribute.
   std::string attrSizeInitCode;
   if (attrSizedResults) {
-    auto attr = op.getGetterName("result_segment_sizes");
-    attrSizeInitCode = formatv(opSegmentSizeAttrInitCode, attr).str();
+    attrSizeInitCode = formatv(opSegmentSizeAttrInitCode,
+                               emit.getAttrSubrange("result_segment_sizes"))
+                           .str();
   }
 
   generateValueRangeStartAndEnd(
       opClass, "getODSResultIndexAndLength", numVariadicResults,
       numNormalResults, "getOperation()->getNumResults()", attrSizedResults,
       attrSizeInitCode, op.getResults());
 
   auto *m = opClass.addMethodAndPrune("::mlir::Operation::result_range",
for (auto &trait : op.getTraits()) {
     }
   }
 }
 
 void OpEmitter::genVerifier() {
   auto *method = opClass.addMethodAndPrune("::mlir::LogicalResult", "verify");
   ERROR_IF_PRUNED(method, "verify", op);
   auto &body = method->body();
-
-  OpOrAdaptorHelper emit(op, /*isOp=*/true);
   genNativeTraitAttrVerifier(body, emit);
 
   auto *valueInit = def.getValueInit("verifier");
   StringInit *stringInit = dyn_cast<StringInit>(valueInit);
   bool hasCustomVerify = stringInit && !stringInit->getValue().empty();
   populateSubstitutions(emit, verifyCtx);
 
   genAttributeVerifier(emit, verifyCtx, body);