TableGen: Allow NAME in template arguments in defm in multiclass

[nhaehnle]

Summary

NAME has already worked for def in a multiclass, since the (protoype)
record including its NAME variable is created before parsing the
superclasses. Since defm's do not have an associated single record,
support for NAME has to be implemented differently here.

Original test cases provided by Artem Belevich (tra)

Change-Id: I933b74f328c0ff202e7dc23a35b78f3505760cc9

[tra]

I'm glad the tests work now, though I suspect it's mostly due to your other changes.

I'd replace the test cases with something that does not have unrelated things and only demonstrates that NAME can be used on the right hand side of def[m] : now.

It looks like use of NAME would also be allowed for def inside multiclass. If that's indeed the case, I'd change the description appropriately.

test/TableGen/MultiClass-defm-fail.td

16 ↗

(On Diff #135551)

Presumably you wanted to replace s with NAME here and below to illustrate the effect of the patch.

[nhaehnle]

Add test for NAME on the right-hand side of def in multiclass.

[nhaehnle]

In D43656#1018010, @tra wrote:

I'm glad the tests work now, though I suspect it's mostly due to your other changes.

I admit I haven't tested the change standalone on top of trunk, but I suspect it does work standalone.

I'd replace the test cases with something that does not have unrelated things and only demonstrates that NAME can be used on the right hand side of def[m] : now.

I've just added additional test cases for those, I think there is some use in keeping the other test cases around as well based on the experience I've had refactoring how records are instantiated (I haven't cleaned up those changes yet, they're to come later).

It looks like use of NAME would also be allowed for def inside multiclass. If that's indeed the case, I'd change the description appropriately.

Yes, and that part has more or less worked already without this change. I've updated the summary to note that.

test/TableGen/MultiClass-defm-fail.td

16 ↗

(On Diff #135551)

Hmm, this is a negative test case, so I think it's actually beneficial that it doesn't depend on NAME: it should fail in the same way, whether or not NAME is supported.

[tra]

Great! Thank you for including different uses of NAME inside a multiclass.

llvm/trunk/lib/TableGen/TGParser.cpp

if (CurRec) {
     if (CurRec->isTemplateArg(TemplateArgName)) {
       const RecordVal *RV = CurRec->getValue(TemplateArgName);
       assert(RV && "Template arg doesn't exist??");
       return VarInit::get(TemplateArgName, RV->getType());
     }
   }
 
   if (CurMultiClass) {
+    if (Name->getValue() == "NAME")
+      return VarInit::get(Name, StringRecTy::get());
+
     Init *MCName = QualifyName(CurMultiClass->Rec, CurMultiClass, Name, "::");
 
     if (CurMultiClass->Rec.isTemplateArg(MCName)) {
       const RecordVal *RV = CurMultiClass->Rec.getValue(MCName);
       assert(RV && "Template arg doesn't exist??");
       return VarInit::get(MCName, RV->getType());
     }
   }

llvm/trunk/test/TableGen/MultiClass-defm-fail.td

+// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s
+// XFAIL: vg_leak
+
+// This test verifies that tablegen does fail if it can't resolve an unresolved
+// !cast() during processing top-level defm.
+
+class A {}
+class B<A a> {
+  A ba = a;
+}
+
+multiclass M0<string s> {
+  // This should work fine.
+  def _m00 : B<!cast<A>(s)>;
+  // CHECK: error: Undefined reference:'d1_r1_no_such_record'
+  def _m01: B<!cast<A>(s#"_no_such_record")>;
+}
+
+multiclass M1<string s> {
+  def _r1 : A;
+  // It would be nice if we could refer to _r1's name without having to pass it
+  // explicitly via 's'.
+  // XCHECK-DAG: note: instantiated from multiclass
+  defm _m1: M0<s # "_r1">;
+}
+
+// CHECK: defm d1: M1
+// CHECK: note: instantiated from multiclass
+// CHECK: defm _m1: M0
+// CHECK: note: instantiated from multiclass
+// CHECK: def _m01: B
+defm d1: M1<"d1">;

llvm/trunk/test/TableGen/MultiClass-defm.td

+// RUN: llvm-tblgen %s | FileCheck %s
+// XFAIL: vg_leak
+
+class A {}
+class B<A a> {
+  A ba = a;
+}
+
+multiclass M0<string s> {
+  def _m0 : B<!cast<A>(s)>;
+
+  // Uncomment to test that !cast will eventually fail if the record it refers
+  // to does not exist by the time we instantiate this record from the top
+  // level.
+  //def _m1 : B<!cast<A>(s#"X")>;
+}
+
+multiclass M1<string s> {
+  def _r1 : A;
+  // It would be nice if we could refer to _r1's name without having to pass it
+  // explicitly via 's'.
+  defm _m1: M0<s # "_r1">;
+}
+
+multiclass M2 {
+  def _x : A {
+    string n = NAME;
+  }
+
+  def _y : B<!cast<A>(NAME # "_x")>;
+
+  // This used to throw an error during multiclass parsing as NAME was not
+  // recognized when parsing the template arguments.
+  defm NAME: M1<NAME>;
+}
+defm d0: M2;
+// CHECK-LABEL: def d0_m1_m0
+// CHECK: A ba = d0_r1;
+//
+// CHECK-LABEL: def d0_x {
+// CHECK: string n = "d0";
+//
+// CHECK-LABEL: def d0_y {
+// CHECK: A ba = d0_x;
+
+// This always works, because d1_r1 is instantiated before d1_m1 which would
+// attempt to !cast to it.
+defm d1: M1<"d1">;
+// CHECK-LABEL: def d1_m1_m0
+// CHECK: A ba = d1_r1;