Use reference for large object passed by value at the moment in MLIR TableGen (NFC)

[mehdi_amini]

Two more cases

[rriddle]

LG, but the description is a bit off. Most of the changes here are for using const, not really about using references.

[mehdi_amini]

Should I say "const reference"? The motivation is really reference, then the "const" came up on the iterator aspect just to enable the change.

I think the "const" thing is only in Operator.h/.cpp right? I can also pre-commit this as NFC

[rriddle]

In D117516#3250224, @mehdi_amini wrote:

Should I say "const reference"? The motivation is really reference, then the "const" came up on the iterator aspect just to enable the change.

I think the "const" thing is only in Operator.h/.cpp right? I can also pre-commit this as NFC

I see const sprinkled throughout the commit on existing references, e.g OpInterfacesGen.cpp:381.

I don't have a huge preference on how the description is spelled (could be two sentences). Just stuck out when I read the commit say "passed by value" and saw references already on most.

[mehdi_amini]

Alright, I added a longer description in the commit message!

mlir/include/mlir/TableGen/Operator.h

VariableDecoratorIterator(llvm::Init *const *it)
                                 VariableDecorator (*)(llvm::Init *)>(it,
                                                                      &unwrap) {}
     static VariableDecorator unwrap(llvm::Init *init);
   };
   using var_decorator_iterator = VariableDecoratorIterator;
   using var_decorator_range = llvm::iterator_range<VariableDecoratorIterator>;
 
   using value_iterator = NamedTypeConstraint *;
+  using const_value_iterator = const NamedTypeConstraint *;
   using value_range = llvm::iterator_range<value_iterator>;
+  using const_value_range = llvm::iterator_range<const_value_iterator>;
 
   // Returns true if this op has variable length operands or results.
   bool isVariadic() const;
 
   // Returns true if default builders should not be generated.
   bool skipDefaultBuilders() const;
 
   // Op result iterators.
-  value_iterator result_begin();
-  value_iterator result_end();
-  value_range getResults();
+  const_value_iterator result_begin() const;
+  const_value_iterator result_end() const;
+  const_value_range getResults() const;
 
   // Returns the number of results this op produces.
   int getNumResults() const;
 
   // Returns the op result at the given `index`.
   NamedTypeConstraint &getResult(int index) { return results[index]; }
   const NamedTypeConstraint &getResult(int index) const {
     return results[index];
public:
 
   // Op attribute accessors.
   NamedAttribute &getAttribute(int index) { return attributes[index]; }
   const NamedAttribute &getAttribute(int index) const {
     return attributes[index];
   }
 
   // Op operand iterators.
-  value_iterator operand_begin();
-  value_iterator operand_end();
-  value_range getOperands();
+  const_value_iterator operand_begin() const;
+  const_value_iterator operand_end() const;
+  const_value_range getOperands() const;
 
   int getNumOperands() const { return operands.size(); }
   NamedTypeConstraint &getOperand(int index) { return operands[index]; }
   const NamedTypeConstraint &getOperand(int index) const {
     return operands[index];
   }
 
   // Returns the number of variadic operands in this operation.

mlir/lib/TableGen/Operator.cpp

 }
 
 const llvm::Record &Operator::getDef() const { return def; }
 
 bool Operator::skipDefaultBuilders() const {
   return def.getValueAsBit("skipDefaultBuilders");
 }
 
-auto Operator::result_begin() -> value_iterator { return results.begin(); }
+auto Operator::result_begin() const -> const_value_iterator {
+  return results.begin();
+}
 
-auto Operator::result_end() -> value_iterator { return results.end(); }
+auto Operator::result_end() const -> const_value_iterator {
+  return results.end();
+}
 
-auto Operator::getResults() -> value_range {
+auto Operator::getResults() const -> const_value_range {
   return {result_begin(), result_end()};
 }
 
 TypeConstraint Operator::getResultTypeConstraint(int index) const {
   DagInit *results = def.getValueAsDag("results");
   return TypeConstraint(cast<DefInit>(results->getArg(index)));
 }
 

 auto Operator::attribute_end() const -> attribute_iterator {
   return attributes.end();
 }
 auto Operator::getAttributes() const
     -> llvm::iterator_range<attribute_iterator> {
   return {attribute_begin(), attribute_end()};
 }
 
-auto Operator::operand_begin() -> value_iterator { return operands.begin(); }
-auto Operator::operand_end() -> value_iterator { return operands.end(); }
-auto Operator::getOperands() -> value_range {
+auto Operator::operand_begin() const -> const_value_iterator {
+  return operands.begin();
+}
+auto Operator::operand_end() const -> const_value_iterator {
+  return operands.end();
+}
+auto Operator::getOperands() const -> const_value_range {
   return {operand_begin(), operand_end()};
 }
 
 auto Operator::getArg(int index) const -> Argument { return arguments[index]; }
 
 // Mapping from result index to combined argument and result index. Arguments
 // are indexed to match getArg index, while the result indexes are mapped to
 // avoid overlap.

mlir/tools/mlir-tblgen/AttrOrTypeDefGen.cpp

static void collectAllDefs(StringRef selectedDialect,
   if (records.empty())
     return;
 
   auto defs = llvm::map_range(
       records, [&](const llvm::Record *rec) { return AttrOrTypeDef(rec); });
   if (selectedDialect.empty()) {
     // If a dialect was not specified, ensure that all found defs belong to the
     // same dialect.
-    if (!llvm::is_splat(
-            llvm::map_range(defs, [](auto def) { return def.getDialect(); }))) {
+    if (!llvm::is_splat(llvm::map_range(
+            defs, [](const auto &def) { return def.getDialect(); }))) {
       llvm::PrintFatalError("defs belonging to more than one dialect. Must "
                             "select one via '--(attr|type)defs-dialect'");
     }
     resultDefs.assign(defs.begin(), defs.end());
   } else {
     // Otherwise, generate the defs that belong to the selected dialect.
-    auto dialectDefs = llvm::make_filter_range(defs, [&](auto def) {
+    auto dialectDefs = llvm::make_filter_range(defs, [&](const auto &def) {
       return def.getDialect().getName().equals(selectedDialect);
     });
     resultDefs.assign(dialectDefs.begin(), dialectDefs.end());
   }
 }
 
 //===----------------------------------------------------------------------===//
 // DefGen

mlir/tools/mlir-tblgen/CodeGenHelpers.cpp

void StaticVerifierFunctionEmitter::collectConstraint(ConstraintMap &map,
                                                       Constraint constraint) {
   auto it = map.find(constraint);
   if (it == map.end())
     map.insert({constraint, getUniqueName(kind, map.size())});
 }
 
 void StaticVerifierFunctionEmitter::collectOpConstraints(
     ArrayRef<Record *> opDefs) {
-  const auto collectTypeConstraints = [&](Operator::value_range values) {
+  const auto collectTypeConstraints = [&](Operator::const_value_range values) {
     for (const NamedTypeConstraint &value : values)
       if (value.hasPredicate())
         collectConstraint(typeConstraints, "type", value.constraint);
   };
 
   for (Record *def : opDefs) {
     Operator op(*def);
     /// Collect type constraints.

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

private:
   // Generates the printer for the operation.
   void genPrinter();
 
   // Generates verify method for the operation.
   void genVerifier();
 
   // Generates verify statements for operands and results in the operation.
   // The generated code will be attached to `body`.
-  void genOperandResultVerifier(MethodBody &body, Operator::value_range values,
+  void genOperandResultVerifier(MethodBody &body,
+                                Operator::const_value_range values,
                                 StringRef valueKind);
 
   // Generates verify statements for regions in the operation.
   // The generated code will be attached to `body`.
   void genRegionVerifier(MethodBody &body);
 
   // Generates verify statements for successors in the operation.
   // The generated code will be attached to `body`.
if (hasCustomVerify) {
     auto printer = stringInit->getValue().ltrim().rtrim(" \t\v\f\r");
     body << "  " << tgfmt(printer, &fctx);
   } else {
     body << "  return ::mlir::success();\n";
   }
 }
 
 void OpEmitter::genOperandResultVerifier(MethodBody &body,
-                                         Operator::value_range values,
+                                         Operator::const_value_range values,
                                          StringRef valueKind) {
   // Check that an optional value is at most 1 element.
   //
   // {0}: Value index.
   // {1}: "operand" or "result"
   const char *const verifyOptional = R"(
     if (valueGroup{0}.size() > 1) {
       return emitOpError("{1} group starting at #") << index

mlir/tools/mlir-tblgen/OpDocGen.cpp

if (!interfaces.empty()) {
     os << "\n";
   }
   if (!effects.empty()) {
     llvm::interleaveComma(effects, os << "\nEffects: ");
     os << "\n";
   }
 }
 
-static void emitOpDoc(Operator op, raw_ostream &os) {
+static void emitOpDoc(const Operator &op, raw_ostream &os) {
   os << llvm::formatv("### `{0}` ({1})\n", op.getOperationName(),
                       op.getQualCppClassName());
 
   // Emit the summary, syntax, and description if present.
   if (op.hasSummary())
     os << "\n" << op.getSummary() << "\n\n";
   if (op.hasAssemblyFormat())
     emitAssemblyFormat(op.getOperationName(), op.getAssemblyFormat().trim(),

mlir/tools/mlir-tblgen/OpInterfacesGen.cpp

public:
   bool emitInterfaceDefs();
   bool emitInterfaceDecls();
   bool emitInterfaceDocs();
 
 protected:
   InterfaceGenerator(std::vector<llvm::Record *> &&defs, raw_ostream &os)
       : defs(std::move(defs)), os(os) {}
 
-  void emitConceptDecl(Interface &interface);
-  void emitModelDecl(Interface &interface);
-  void emitModelMethodsDef(Interface &interface);
-  void emitTraitDecl(Interface &interface, StringRef interfaceName,
+  void emitConceptDecl(const Interface &interface);
+  void emitModelDecl(const Interface &interface);
+  void emitModelMethodsDef(const Interface &interface);
+  void emitTraitDecl(const Interface &interface, StringRef interfaceName,
                      StringRef interfaceTraitsName);
-  void emitInterfaceDecl(Interface interface);
+  void emitInterfaceDecl(const Interface &interface);
 
   /// The set of interface records to emit.
   std::vector<llvm::Record *> defs;
   // The stream to emit to.
   raw_ostream &os;
   /// The C++ value type of the interface, e.g. Operation*.
   StringRef valueType;
   /// The C++ base interface type.
for (const auto *def : defs)
     emitInterfaceDef(Interface(def), valueType, os);
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // GEN: Interface declarations
 //===----------------------------------------------------------------------===//
 
-void InterfaceGenerator::emitConceptDecl(Interface &interface) {
+void InterfaceGenerator::emitConceptDecl(const Interface &interface) {
   os << "  struct Concept {\n";
 
   // Insert each of the pure virtual concept methods.
   for (auto &method : interface.getMethods()) {
     os << "    ";
     emitCPPType(method.getReturnType(), os);
     os << "(*" << method.getName() << ")(";
     if (!method.isStatic()) {
       os << "const Concept *impl, ";
       emitCPPType(valueType, os) << (method.arg_empty() ? "" : ", ");
     }
     llvm::interleaveComma(
         method.getArguments(), os,
         [&](const InterfaceMethod::Argument &arg) { os << arg.type; });
     os << ");\n";
   }
   os << "  };\n";
 }
 
-void InterfaceGenerator::emitModelDecl(Interface &interface) {
+void InterfaceGenerator::emitModelDecl(const Interface &interface) {
   // Emit the basic model and the fallback model.
   for (const char *modelClass : {"Model", "FallbackModel"}) {
     os << "  template<typename " << valueTemplate << ">\n";
     os << "  class " << modelClass << " : public Concept {\n  public:\n";
     os << "    using Interface = " << interface.getCppNamespace()
        << (interface.getCppNamespace().empty() ? "" : "::")
        << interface.getName() << ";\n";
     os << "    " << modelClass << "() : Concept{";
for (auto &method : interface.getMethods()) {
     os << ")";
     if (!method.isStatic())
       os << " const";
     os << ";\n";
   }
   os << "  };\n";
 }
 
-void InterfaceGenerator::emitModelMethodsDef(Interface &interface) {
+void InterfaceGenerator::emitModelMethodsDef(const Interface &interface) {
   for (auto &method : interface.getMethods()) {
     os << "template<typename " << valueTemplate << ">\n";
     emitCPPType(method.getReturnType(), os);
     os << "detail::" << interface.getName() << "InterfaceTraits::Model<"
        << valueTemplate << ">::";
     emitMethodNameAndArgs(method, os, valueType,
                           /*addThisArg=*/!method.isStatic(),
                           /*addConst=*/false);
for (auto &method : interface.getMethods()) {
     // Use the empty context for static methods.
     tblgen::FmtContext ctx;
     os << tblgen::tgfmt(method.getDefaultImplementation()->trim(),
                         method.isStatic() ? &ctx : &nonStaticMethodFmt);
     os << "\n}\n";
   }
 }
 
-void InterfaceGenerator::emitTraitDecl(Interface &interface,
+void InterfaceGenerator::emitTraitDecl(const Interface &interface,
                                        StringRef interfaceName,
                                        StringRef interfaceTraitsName) {
   os << llvm::formatv("  template <typename {3}>\n"
                       "  struct {0}Trait : public ::mlir::{2}<{0},"
                       " detail::{1}>::Trait<{3}> {{\n",
                       interfaceName, interfaceTraitsName, interfaceBaseType,
                       valueTemplate);
 
void InterfaceGenerator::emitTraitDecl(const Interface &interface,
   if (auto extraTraitDecls = interface.getExtraTraitClassDeclaration())
     os << tblgen::tgfmt(*extraTraitDecls, &traitMethodFmt) << "\n";
   if (auto extraTraitDecls = interface.getExtraSharedClassDeclaration())
     os << tblgen::tgfmt(*extraTraitDecls, &traitMethodFmt) << "\n";
 
   os << "  };\n";
 }
 
-void InterfaceGenerator::emitInterfaceDecl(Interface interface) {
+void InterfaceGenerator::emitInterfaceDecl(const Interface &interface) {
   llvm::SmallVector<StringRef, 2> namespaces;
   llvm::SplitString(interface.getCppNamespace(), namespaces, "::");
   for (StringRef ns : namespaces)
     os << "namespace " << ns << " {\n";
 
   StringRef interfaceName = interface.getName();
   auto interfaceTraitsName = (interfaceName + "InterfaceTraits").str();