TableGen: fix typeIsConvertibleTo for record types

[nhaehnle]

Summary

Only check whether the left-hand side type is a subclass (or equal to)
the right-hand side type.

This requires a further fix in handling !if expressions and in type
resolution.

Furthermore, reverse the order of superclasses so that resolveTypes will
find a least common ancestor at least in simple cases.

Add a test that used to be accepted without flagging the obvious type
error.

Change-Id: Ib366db1a4e6a079f1a0851e469b402cddae76714

[tra]

test/TableGen/if-type.td

10 ↗

(On Diff #135220)

Would A x = !if(cc, b, c); be expected to work? If yes, you may want to add a positive test case for it.

[nhaehnle]

Added positive test.

test/TableGen/if-type.td

10 ↗

(On Diff #135220)

Yes, it's expected to work, and it does work :) I've added a test for it.

[tra]

test/TableGen/if.td

76–85 ↗

(On Diff #135425)

I think there should be some defs and CHECKs to demonstrate and verify what we expect to generate.

It's actually not obvious for me whether a particular instance of EX is going to end up with.
I think it will contain only the subset of fields in E from E1/E2. Or, perhaps, I'm wrong and it will contain full set of records from either E1 or E2, depending on cc. Either way it would be good to illustrate expected behavior.

[nhaehnle]

Add defs for EX test.

test/TableGen/if.td

76–85 ↗

(On Diff #135425)

I don't think the question about fields makes much sense because x will just reference some other record. Of course an expression x.fieldname will only allow access to fields in E.

Anyway, I'm adding some defs to illustrate this, and I suppose it's a good check anyway to make sure things don't break when resolving b and c.

[tra]

test/TableGen/if.td

76–85 ↗

(On Diff #135425)

Right. s/contain/make accessible/.

Thank you for adding the defs.

llvm/trunk/lib/TableGen/Record.cpp

 bool RecordRecTy::typeIsConvertibleTo(const RecTy *RHS) const {
   const RecordRecTy *RTy = dyn_cast<RecordRecTy>(RHS);
   if (!RTy)
     return false;
 
   if (RTy->getRecord() == Rec || Rec->isSubClassOf(RTy->getRecord()))
     return true;
 
-  for (const auto &SCPair : RTy->getRecord()->getSuperClasses())
-    if (Rec->isSubClassOf(SCPair.first))
-      return true;
-
   return false;
 }
 
 RecTy *llvm::resolveTypes(RecTy *T1, RecTy *T2) {
   if (T1->typeIsConvertibleTo(T2))
     return T2;
   if (T2->typeIsConvertibleTo(T1))
     return T1;
if (RecordRecTy *RecTy2 = dyn_cast<RecordRecTy>(T2)) {
     // See if T1 inherits from a type T2 also inherits from
     for (const auto &SuperPair2 : RecTy2->getRecord()->getSuperClasses()) {
       RecordRecTy *SuperRecTy2 = RecordRecTy::get(SuperPair2.first);
       RecTy *NewType2 = resolveTypes(T1, SuperRecTy2);
       if (NewType2)
         return NewType2;
     }
   }
+
+  if (ListRecTy *ListTy1 = dyn_cast<ListRecTy>(T1)) {
+    if (ListRecTy *ListTy2 = dyn_cast<ListRecTy>(T2)) {
+      RecTy* NewType = resolveTypes(ListTy1->getElementType(),
+                                    ListTy2->getElementType());
+      if (NewType)
+        return NewType->getListTy();
+    }
+  }
+
   return nullptr;
 }
 
 //===----------------------------------------------------------------------===//
 //    Initializer implementations
 //===----------------------------------------------------------------------===//
 
 void Init::anchor() {}

llvm/trunk/lib/TableGen/TGParser.cpp

if (i < SubClass.TemplateArgs.size()) {
                    "Value not specified for template argument #" +
                    Twine(i) + " (" + TArgs[i]->getAsUnquotedString() +
                    ") of subclass '" + SC->getNameInitAsString() + "'!");
     }
   }
 
   // Since everything went well, we can now set the "superclass" list for the
   // current record.
+  CurRec->addSuperClass(SC, SubClass.RefRange);
+
   ArrayRef<std::pair<Record *, SMRange>> SCs = SC->getSuperClasses();
   for (const auto &SCPair : SCs) {
     if (CurRec->isSubClassOf(SCPair.first))
       return Error(SubClass.RefRange.Start,
                    "Already subclass of '" + SCPair.first->getName() + "'!\n");
     CurRec->addSuperClass(SCPair.first, SCPair.second);
   }
-
-  if (CurRec->isSubClassOf(SC))
-    return Error(SubClass.RefRange.Start,
-                 "Already subclass of '" + SC->getName() + "'!\n");
-  CurRec->addSuperClass(SC, SubClass.RefRange);
   return false;
 }
 
 /// AddSubMultiClass - Add SubMultiClass as a subclass to
 /// CurMC, resolving its template args as SubMultiClass's
 /// template arguments.
 bool TGParser::AddSubMultiClass(MultiClass *CurMC,
                                 SubMultiClassReference &SubMultiClass) {
case tgtok::XIf: {
       if (isa<UnsetInit>(RHS))
         RHSTy = MHSTy;
 
       if (!MHSTy || !RHSTy) {
         TokError("could not get type for !if");
         return nullptr;
       }
 
-      if (MHSTy->typeIsConvertibleTo(RHSTy)) {
-        Type = RHSTy;
-      } else if (RHSTy->typeIsConvertibleTo(MHSTy)) {
-        Type = MHSTy;
-      } else {
-        TokError("inconsistent types for !if");
+      Type = resolveTypes(MHSTy, RHSTy);
+      if (!Type) {
+        TokError(Twine("inconsistent types '") + MHSTy->getAsString() +
+                 "' and '" + RHSTy->getAsString() + "' for !if");
         return nullptr;
       }
       break;
     }
     case tgtok::XForEach: {
       TypedInit *MHSt = dyn_cast<TypedInit>(MHS);
       if (!MHSt) {
         TokError("could not get type for !foreach");

llvm/trunk/test/TableGen/if-type.td

+// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s
+// XFAIL: vg_leak
+
+class A<int dummy> {}
+class B<int dummy> : A<dummy> {}
+class C<int dummy> : A<dummy> {}
+
+// CHECK: Value 'x' of type 'C' is incompatible with initializer '{{.*}}' of type 'A'
+class X<int cc, B b, C c> {
+  C x = !if(cc, b, c);
+}

llvm/trunk/test/TableGen/if.td

class I1<int c> {
   int i = getInt<c>.ret;
 }
 class I2<int c> : I1<c>;
 
 // CHECK: def DI1 {     // I1
 // CHECK-NEXT: int i = 0;
 def DI1: I1<1>;
 
-// CHECK: def DI2 {     // I1 I2
+// CHECK: def DI2 {
 // CHECK-NEXT: int i = 0;
 def DI2: I2<1>;
 
+// Check that !if with operands of different subtypes can initialize a
+// supertype variable.
+//
+// CHECK: def EXd1 {
+// CHECK:   E x = E1d;
+// CHECK: }
+//
+// CHECK: def EXd2 {
+// CHECK:   E x = E2d;
+// CHECK: }
+class E<int dummy> {}
+class E1<int dummy> : E<dummy> {}
+class E2<int dummy> : E<dummy> {}
+
+class EX<int cc, E1 b, E2 c> {
+  E x = !if(cc, b, c);
+}
+
+def E1d : E1<0>;
+def E2d : E2<0>;
+
+def EXd1 : EX<1, E1d, E2d>;
+def EXd2 : EX<0, E1d, E2d>;
+
 // CHECK:      def One
 // CHECK-NEXT: list<int> first = [1, 2, 3];
 // CHECK-NEXT: list<int> rest = [1, 2, 3];
 
 // CHECK:      def OneB
 // CHECK-NEXT: list<int> vals = [1, 2, 3];
 
 // CHECK:      def Two