[tablegen] Resolve complex def identifiers when instanciating multiclass

[deadalnix]

Summary

Complex name paterns are not handled properly in multiclass delcarations. This diff makes it work.

I have no idea what Chris Lattner or Daved Greene phab account are :/

[hfinkel]

lib/TableGen/TGParser.cpp

2391

If so, resolve it.

2398

Line is too long.

2407

option -> options

test/TableGen/MultiClassDefName.td

47

Can you please add a test with both a strong concatenation and also a concatenation with NAME?

[silvas]

What do you need this for?

test/TableGen/MultiClassDefName.td

47

Also, the case where the template argument is a complex type. E.g.:

multiclass MC3<Foo f> {
  def ZFizz#f.s : C<f.s>;
}

[deadalnix]

Style change as per @hfinkel request.
Added requested test cases.

[deadalnix]

lib/TableGen/TGParser.cpp

2391

Well that why we have plenty of green code below.

test/TableGen/MultiClassDefName.td

47

What do you mean by strong concatenation ?

[hfinkel]

lib/TableGen/TGParser.cpp

2391

No, I meant please reword the comment from

// If yes, resolve !

to

// If so, resolve it.

test/TableGen/MultiClassDefName.td

47

Was a typo. I meant string concatenation.

def NAME#ZFizz#f.s

[deadalnix]

@hfinkel comments

[hfinkel]

LGTM.

[deadalnix]

Rebase, also, can someone with merge right can emrge this ?

[deadalnix]

Ping, can we get this merged ?

[hfinkel]

r237877

lib/TableGen/TGParser.h

 private:  // Parser methods.
   bool ParseObjectList(MultiClass *MC = nullptr);
   bool ParseObject(MultiClass *MC);
   bool ParseClass();
   bool ParseMultiClass();
   Record *InstantiateMulticlassDef(MultiClass &MC,
                                    Record *DefProto,
                                    Init *&DefmPrefix,
-                                   SMRange DefmPrefixRange);
+                                   SMRange DefmPrefixRange,
+                                   const std::vector<Init *> &TArgs,
+                                   std::vector<Init *> &TemplateVals);
   bool ResolveMulticlassDefArgs(MultiClass &MC,
                                 Record *DefProto,
                                 SMLoc DefmPrefixLoc,
                                 SMLoc SubClassLoc,
                                 const std::vector<Init *> &TArgs,
                                 std::vector<Init *> &TemplateVals,
                                 bool DeleteArgs);
   bool ResolveMulticlassDef(MultiClass &MC,

lib/TableGen/TGParser.cpp

bool TGParser::ParseMultiClass() {
   CurMultiClass = nullptr;
   return false;
 }
 
 Record *TGParser::
 InstantiateMulticlassDef(MultiClass &MC,
                          Record *DefProto,
                          Init *&DefmPrefix,
-                         SMRange DefmPrefixRange) {
+                         SMRange DefmPrefixRange,
+                         const std::vector<Init *> &TArgs,
+                         std::vector<Init *> &TemplateVals) {
   // We need to preserve DefProto so it can be reused for later
   // instantiations, so create a new Record to inherit from it.
 
   // Add in the defm name.  If the defm prefix is empty, give each
   // instantiated def a unique name.  Otherwise, if "#NAME#" exists in the
   // name, substitute the prefix for #NAME#.  Otherwise, use the defm name
   // as a prefix.
 
   bool IsAnonymous = false;
   if (!DefmPrefix) {
     DefmPrefix = StringInit::get(GetNewAnonymousName());
     IsAnonymous = true;
   }
 
   Init *DefName = DefProto->getNameInit();
-
   StringInit *DefNameString = dyn_cast<StringInit>(DefName);
 
   if (DefNameString) {
     // We have a fully expanded string so there are no operators to
     // resolve.  We should concatenate the given prefix and name.
     DefName =
       BinOpInit::get(BinOpInit::STRCONCAT,
                      UnOpInit::get(UnOpInit::CAST, DefmPrefix,
InstantiateMulticlassDef(MultiClass &MC,
   }
 
   if (!CurMultiClass) {
     // Now that we're at the top level, resolve all NAME references
     // in the resultant defs that weren't in the def names themselves.
     RecordVal *DefNameRV = CurRec->getValue("NAME");
     CurRec->resolveReferencesTo(DefNameRV);
 
+    // Check if the name is a complex pattern.
+    // If so, resolve it.
+    DefName = CurRec->getNameInit();
+    DefNameString = dyn_cast<StringInit>(DefName);
+
+    // OK the pattern is more complex than simply using NAME.
+    // Let's use the heavy weaponery.
+    if (!DefNameString) {
+      ResolveMulticlassDefArgs(MC, CurRec.get(), DefmPrefixRange.Start,
+                               Lex.getLoc(), TArgs, TemplateVals,
+                               false/*Delete args*/);
+      DefName = CurRec->getNameInit();
+      DefNameString = dyn_cast<StringInit>(DefName);
+
+      if (!DefNameString)
+        DefName = DefName->convertInitializerTo(StringRecTy::get());
+
+      // We ran out of options here...
+      DefNameString = dyn_cast<StringInit>(DefName);
+      if (!DefNameString) {
+        PrintFatalError(CurRec->getLoc()[CurRec->getLoc().size() - 1],
+                        DefName->getAsUnquotedString() + " is not a string.");
+        return nullptr;
+      }
+
+      CurRec->setName(DefName);
+    }
+
     // Now that NAME references are resolved and we're at the top level of
     // any multiclass expansions, add the record to the RecordKeeper. If we are
     // currently in a multiclass, it means this defm appears inside a
     // multiclass and its name won't be fully resolvable until we see
-    // the top-level defm.  Therefore, we don't add this to the
-    // RecordKeeper at this point.  If we did we could get duplicate
+    // the top-level defm. Therefore, we don't add this to the
+    // RecordKeeper at this point. If we did we could get duplicate
     // defs as more than one probably refers to NAME or some other
     // common internal placeholder.
 
     // Ensure redefinition doesn't happen.
     if (Records.getDef(CurRec->getNameInitAsString())) {
       Error(DefmPrefixRange.Start, "def '" + CurRec->getNameInitAsString() +
             "' already defined, instantiating defm with subdef '" + 
             DefProto->getNameInitAsString() + "'");
while (1) {
     // Verify that the correct number of template arguments were specified.
     const std::vector<Init *> &TArgs = MC->Rec.getTemplateArgs();
     if (TArgs.size() < TemplateVals.size())
       return Error(SubClassLoc,
                    "more template args specified than multiclass expects");
 
     // Loop over all the def's in the multiclass, instantiating each one.
     for (const std::unique_ptr<Record> &DefProto : MC->DefPrototypes) {
+      // The record name construction goes as follow:
+      //  - If the def name is a string, prepend the prefix.
+      //  - If the def name is a more complex pattern, use that pattern.
+      // As a result, the record is instanciated before resolving
+      // arguments, as it would make its name a string.
       Record *CurRec = InstantiateMulticlassDef(*MC, DefProto.get(), DefmPrefix,
                                                 SMRange(DefmLoc,
-                                                        DefmPrefixEndLoc));
+                                                        DefmPrefixEndLoc),
+                                                TArgs, TemplateVals);
       if (!CurRec)
         return true;
 
+      // Now that the record is instanciated, we can resolve arguments.
       if (ResolveMulticlassDefArgs(*MC, CurRec, DefmLoc, SubClassLoc,
                                    TArgs, TemplateVals, true/*Delete args*/))
         return Error(SubClassLoc, "could not instantiate def");
 
       if (ResolveMulticlassDef(*MC, CurRec, DefProto.get(), DefmLoc))
         return Error(SubClassLoc, "could not instantiate def");
 
       // Defs that can be used by other definitions should be fully resolved

test/TableGen/MultiClassDefName.td

 }
 
 // Ensure that we've correctly captured the reference to name from the implicit
 // anonymous C def in the template parameter list of Outer.
 // CHECK-NOT: MC2::name
 
 defm : MC2<"bar">;
 
+multiclass MC3<string s> {
+  def ZFizz#s : C<s>;
+}
+
+defm : MC3<"Buzz">;
+
+// CHECK: def ZFizzBuzz
+// CHECK: string name = "Buzz";
+// CHECK-NOT: MC3::s
+
+multiclass MC4<string s> {
+  def NAME#s : C<s>;
+}
+
+defm ZTagazok : MC4<"AToi">;
+
+// CHECK: def ZTagazokAToi
+// CHECK: string name = "AToi";
+// CHECK-NOT: MC4::s
+
+multiclass MC5<C c> {
+  def NAME#c.name : C<c.name>;
+}
+
+def CTiger : C<"Tiger">;
+defm Zebra : MC5<CTiger>;
+
+// CHECK: def ZebraTiger
+// CHECK: string name = "Tiger";
+// CHECK-NOT: MC5::c
+
+multiclass MC6<C c> {
+  def NAME#Tiger#c.name : C<c.name>;
+}
+
+def CAligator : C<"Aligator">;
+defm Zebra : MC6<CAligator>;
+
+// CHECK: def ZebraTigerAligator
+// CHECK: string name = "Aligator";
+// CHECK-NOT: MC6::c
+