[tblgen] Fix behavior of !isa to prevent premature folding

[krzysz00]

The test included with this commit shows a case where, even though a record's true type was !isa<> some unrelated class, the isa<> operator wolud use the declared type of the argument it was examining in order to conclude that the !isa<> expression had to be be false.

The issues is fixed by checking to make sure that the argument to the !isa operator is fully concrete before declaring its result to be false.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Krzysztof Drewniak (krzysz00)

Changes

The test included with this commit shows a case where, even though a record's true type was !isa<> some unrelated class, the isa<> operator wolud use the declared type of the argument it was examining in order to conclude that the !isa<> expression had to be be false.

The issues is fixed by checking to make sure that the argument to the !isa operator is fully concrete before declaring its result to be false.

---

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

2 Files Affected:

• (modified) llvm/lib/TableGen/Record.cpp (+6-4)
• (added) llvm/test/TableGen/isa-non-primary.td (+33)

diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index 590656786bc66..5e40c8f3cfa62 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -2094,10 +2094,12 @@ const Init *IsAOpInit::Fold() const {
       return IntInit::get(getRecordKeeper(), 1);
 
     if (isa<RecordRecTy>(CheckType)) {
-      // If the target type is not a subclass of the expression type, or if
-      // the expression has fully resolved to a record, we know that it can't
-      // be of the required type.
-      if (!CheckType->typeIsConvertibleTo(TI->getType()) || isa<DefInit>(Expr))
+      // If the target type is not a subclass of the expression type once the
+      // expression has been made concrete, or if the expression has fully
+      // resolved to a record, we know that it can't be of the required type.
+      if ((!CheckType->typeIsConvertibleTo(TI->getType()) &&
+           Expr->isConcrete()) ||
+          isa<DefInit>(Expr))
         return IntInit::get(getRecordKeeper(), 0);
     } else {
       // We treat non-record types as not castable.
diff --git a/llvm/test/TableGen/isa-non-primary.td b/llvm/test/TableGen/isa-non-primary.td
new file mode 100644
index 0000000000000..96f9dfbc29d3a
--- /dev/null
+++ b/llvm/test/TableGen/isa-non-primary.td
@@ -0,0 +1,33 @@
+// RUN: llvm-tblgen %s | FileCheck %s
+
+// CHECK: --- Defs ---
+// CHECK: def Op1 {       // Op
+// CHECK-NEXT:  string res = "yes";
+// CHECK-NEXT: }
+// CHECK: def Op2 {       // Op
+// CHECK-NEXT:  string res = "no";
+// CHECK-NEXT: }
+
+class A<int a> {
+  int x = a;
+}
+
+class B<int a> : A<a> {
+  bit y = 0;
+}
+
+class C<int a> {
+  int z = !add(a, 16);
+}
+
+class D<int a> : B<a>, C<a>;
+
+def E1 : D<5>;
+def E2 : B<2>;
+
+class Op<A value> {
+  string res = !if(!isa<C>(value), "yes", "no");
+}
+
+def Op1 : Op<E1>;
+def Op2 : Op<E2>;

[krzysz00]

Ping

[jurahul]

Seems reasonable to me.