[Mlir-commits] [mlir] [mlir][tblgen] Error on unsubstituted $_self in op trait predicates (PR #227263)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: LiRvs-Miria

<details>
<summary>Changes</summary>

`PredOpTrait` predicates are expanded against the whole operation, so `$_self` is not bound in that context. If an ODS definition uses `$_self` in a trait predicate, tblgen silently emits C++ containing `$_self<no-subst-found>`, which only surfaces as a confusing compile error inside the generated `.inc` file.

Minimal reproduction:

```tablegen
include "mlir/IR/OpBase.td"

def Test_Dialect : Dialect {
  let name = "test_dialect";
}

def BadOp : Op<Test_Dialect, "bad_op", [
    PredOpTrait<"lhs and rhs must have the same type",
                CPred<"$_self.getType() == $_op.getOperand(0).getType()">>
  ]> {
  let arguments = (ins AnyType:$a, AnyType:$b);
}
```

`mlir-tblgen -gen-op-defs` accepts this definition (exit code 0) and generates an op verifier containing:

```cpp
if (!(($_self<no-subst-found>.getType() == (*this->getOperation()).getOperand(0).getType())))
```

which fails to compile with an error pointing into the generated file, leaving no indication that the problem is in the ODS definition.

This patch detects the unsubstituted placeholder while formatting the predicate and reports a TableGen error at the ODS definition site of the offending op instead:

```
error: trait 'lhs and rhs must have the same type' on operation 'test_dialect.bad_op' uses $_self in its predicate, but $_self is not bound in trait context; use $_op to refer to the operation, or pre-substitute $_self with !subst
```

`$_self` cannot be resolved in trait context: a trait covers the whole operation, so there is no single entity for `$_self` to refer to, and guessing a binding would silently change the predicate's meaning. Users should instead reference entities by name (`$lhs`, `$rhs`) or use primitives such as `AllTypesMatch`/`TypesMatchWith`, which pre-substitute `$_self` via `!subst` — exactly what the generated code for those primitives does.

A new `ERROR18` case in `mlir/test/mlir-tblgen/op-error.td` covers the diagnostic (checked while generating op definitions, `-gen-op-defs`). A follow-up documentation patch covering these `$_self` binding rules is in preparation.

AI tool usage: I used an AI assistant to help analyze the root cause and draft this PR description. The code change and testing were done by me.

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


2 Files Affected:

- (modified) mlir/test/mlir-tblgen/op-error.td (+14) 
- (modified) mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp (+12-2) 


``````````diff
diff --git a/mlir/test/mlir-tblgen/op-error.td b/mlir/test/mlir-tblgen/op-error.td
index ec90891034fbe..1337e793dd15a 100644
--- a/mlir/test/mlir-tblgen/op-error.td
+++ b/mlir/test/mlir-tblgen/op-error.td
@@ -15,6 +15,8 @@
 // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR15 %s 2>&1 | FileCheck --check-prefix=ERROR15 %s
 // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR16 %s 2>&1 | FileCheck --check-prefix=ERROR16 %s
 // RUN: not mlir-tblgen -gen-op-decls -I %S/../../include -DERROR17 %s 2>&1 | FileCheck --check-prefix=ERROR17 %s
+// This error is caught while generating the op verifier.
+// RUN: not mlir-tblgen -gen-op-defs -I %S/../../include -DERROR18 %s 2>&1 | FileCheck --check-prefix=ERROR18 %s
 
 include "mlir/IR/OpBase.td"
 
@@ -166,3 +168,15 @@ def OpWithLegacyResultSegmentProperty
   let results = (outs Variadic<AnyType>:$lhs, Variadic<AnyType>:$rhs);
 }
 #endif
+
+#ifdef ERROR18
+// ERROR18: error: trait 'lhs and rhs must have the same type' on operation 'test_dialect.unsubstituted_self' uses $_self in its predicate, but $_self is not bound in trait context
+def UnsubstitutedSelfOp
+    : Op<Test_Dialect, "unsubstituted_self",
+        [PredOpTrait<"lhs and rhs must have the same type",
+                CPred<"$_self.getType() == $_op.getOperand(0).getType()">
+              >
+        ]> {
+  let arguments = (ins AnyType:$a, AnyType:$b);
+}
+#endif
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 48158b6b148aa..d1f9a4f5d8a81 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -3764,10 +3764,20 @@ void OpEmitter::genVerifier() {
 
   for (auto &trait : op.getTraits()) {
     if (auto *t = dyn_cast<tblgen::PredTrait>(&trait)) {
+      // Trait predicates expand against the whole operation, so $_self is not
+      // substituted here; catch it and report at the ODS level instead
+      std::string pred = tgfmt(t->getPredTemplate(), &verifyCtx);
+      if (StringRef(pred).contains("<no-subst-found>"))
+        PrintFatalError(op.getLoc(),
+                        "trait '" + t->getSummary() + "' on operation '" +
+                            op.getOperationName() +
+                            "' uses $_self in its predicate, but $_self is "
+                            "not bound in trait context; use $_op to refer "
+                            "to the operation, or pre-substitute $_self with "
+                            "!subst");
       implBody << tgfmt("  if (!($0))\n    "
                         "return emitOpError(\"failed to verify that $1\");\n",
-                        &verifyCtx, tgfmt(t->getPredTemplate(), &verifyCtx),
-                        t->getSummary());
+                        &verifyCtx, pred, t->getSummary());
     }
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/227263