[MLIR][DRR] Rename OperandIndexOrNumValues to ArgIndexOrNumValues (NFC)

[xl4624]

I think the term is misleading because this actually refers to the argument index, as seen here:

llvm-project/mlir/lib/TableGen/Pattern.cpp

Lines 303 to 311 in 6e5ee4a

	case Kind::Operand: {
	assert(index < 0);
	auto *operand = cast<NamedTypeConstraint *>(op->getArg(getArgIndex()));
	if (operand->isOptional()) {
	auto repl = formatv(
	fmt, formatv("({0}.empty() ? ::mlir::Value() : *{0}.begin())", name));
	LLVM_DEBUG(dbgs() << repl << " (OptionalOperand)\n");
	return std::string(repl);
	}

and here:

llvm-project/mlir/lib/TableGen/Pattern.cpp

Lines 253 to 259 in 6e5ee4a

	case Kind::Attr: {
	if (op)
	return cast<NamedAttribute *>(op->getArg(getArgIndex()))
	->attr.getStorageType()
	.str();
	// TODO(suderman): Use a more exact type when available.
	return "::mlir::Attribute";

Especially because in RewriterGen.cpp, operandIndex is also used alongside argIndex and refers to the index counting only operands (ignoring Attributes). Plus, the getter for this variable for Attributes and Operands is called getArgIndex(). So I think renaming this would help remove a possible source of confusion.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Xiaomin Liu (xl4624)

Changes

I think the term is misleading because this actually refers to the argument index, as seen here:

llvm-project/mlir/lib/TableGen/Pattern.cpp

Lines 303 to 311 in 6e5ee4a

	case Kind::Operand: {
	assert(index < 0);
	auto *operand = cast<NamedTypeConstraint *>(op->getArg(getArgIndex()));
	if (operand->isOptional()) {
	auto repl = formatv(
	fmt, formatv("({0}.empty() ? ::mlir::Value() : *{0}.begin())", name));
	LLVM_DEBUG(dbgs() << repl << " (OptionalOperand)\n");
	return std::string(repl);
	}

and here:

llvm-project/mlir/lib/TableGen/Pattern.cpp

Lines 253 to 259 in 6e5ee4a

	case Kind::Attr: {
	if (op)
	return cast<NamedAttribute *>(op->getArg(getArgIndex()))
	->attr.getStorageType()
	.str();
	// TODO(suderman): Use a more exact type when available.
	return "::mlir::Attribute";

Especially because in RewriterGen, operandIndex is also used alongside argIndex and refers to the index counting only operands (ignoring Attributes). The getter for this variable is called getArgIndex(). So I think renaming this would help remove a possible source of confusion.

---

Full diff: https://github.com/llvm/llvm-project/pull/144821.diff

1 Files Affected:

• (modified) mlir/include/mlir/TableGen/Pattern.h (+17-18)

diff --git a/mlir/include/mlir/TableGen/Pattern.h b/mlir/include/mlir/TableGen/Pattern.h
index 1c9e128f0a0fb..abd567ee4bdbc 100644
--- a/mlir/include/mlir/TableGen/Pattern.h
+++ b/mlir/include/mlir/TableGen/Pattern.h
@@ -276,13 +276,12 @@ class SymbolInfoMap {
     // Structure to uniquely distinguish different locations of the symbols.
     //
     // * If a symbol is defined as an operand of an operation, `dag` specifies
-    //   the DAG of the operation, `operandIndexOrNumValues` specifies the
-    //   operand index, and `variadicSubIndex` must be set to `std::nullopt`.
+    //   the DAG of the operation, `argIndexOrNumValues` specifies the
+    //    index, and `variadicSubIndex` must be set to `std::nullopt`.
     //
     // * If a symbol is defined in a `variadic` DAG, `dag` specifies the DAG
-    //   of the parent operation, `operandIndexOrNumValues` specifies the
-    //   declared operand index of the variadic operand in the parent
-    //   operation.
+    //   of the parent operation, `argIndexOrNumValues` specifies the
+    //   declared index of the variadic operand in the parent operation.
     //
     //   - If the symbol is defined as a result of `variadic` DAG, the
     //     `variadicSubIndex` must be set to `std::nullopt`, which means that
@@ -292,9 +291,9 @@ class SymbolInfoMap {
     //     be set to the index within the variadic sub-operand list.
     //
     // * If a symbol is defined in a `either` DAG, `dag` specifies the DAG
-    //   of the parent operation, `operandIndexOrNumValues` specifies the
-    //   operand index in the parent operation (not necessary the index in the
-    //   DAG).
+    //   of the parent operation, `argIndexOrNumValues` specifies the
+    //   operand's index in the parent operation (not necessarily its index 
+    //   in the DAG).
     //
     // * If a symbol is defined as a result, specifies the number of returning
     //   value.
@@ -347,17 +346,17 @@ class SymbolInfoMap {
       // DagNode and DagLeaf are accessed by value which means it can't be used
       // as identifier here. Use an opaque pointer type instead.
       const void *dag;
-      int operandIndexOrNumValues;
+      int argIndexOrNumValues;
       std::optional<int> variadicSubIndex;
 
-      DagAndConstant(const void *dag, int operandIndexOrNumValues,
+      DagAndConstant(const void *dag, int argIndexOrNumValues,
                      std::optional<int> variadicSubIndex)
-          : dag(dag), operandIndexOrNumValues(operandIndexOrNumValues),
+          : dag(dag), argIndexOrNumValues(argIndexOrNumValues),
             variadicSubIndex(variadicSubIndex) {}
 
       bool operator==(const DagAndConstant &rhs) const {
         return dag == rhs.dag &&
-               operandIndexOrNumValues == rhs.operandIndexOrNumValues &&
+               argIndexOrNumValues == rhs.argIndexOrNumValues &&
                variadicSubIndex == rhs.variadicSubIndex;
       }
     };
@@ -385,11 +384,11 @@ class SymbolInfoMap {
       return SymbolInfo(nullptr, Kind::Attr, std::nullopt);
     }
     static SymbolInfo
-    getOperand(DagNode node, const Operator *op, int operandIndex,
+    getOperand(DagNode node, const Operator *op, int index,
                std::optional<int> variadicSubIndex = std::nullopt) {
-      return SymbolInfo(op, Kind::Operand,
-                        DagAndConstant(node.getAsOpaquePointer(), operandIndex,
-                                       variadicSubIndex));
+      return SymbolInfo(
+          op, Kind::Operand,
+          DagAndConstant(node.getAsOpaquePointer(), index, variadicSubIndex));
     }
     static SymbolInfo getResult(const Operator *op) {
       return SymbolInfo(op, Kind::Result, std::nullopt);
@@ -427,10 +426,10 @@ class SymbolInfoMap {
                                const char *separator) const;
 
     // The argument index (for `Attr` and `Operand` only)
-    int getArgIndex() const { return dagAndConstant->operandIndexOrNumValues; }
+    int getArgIndex() const { return dagAndConstant->argIndexOrNumValues; }
 
     // The number of values in the MultipleValue
-    int getSize() const { return dagAndConstant->operandIndexOrNumValues; }
+    int getSize() const { return dagAndConstant->argIndexOrNumValues; }
 
     // The variadic sub-operands index (for variadic `Operand` only)
     std::optional<int> getVariadicSubIndex() const {

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.