TableGen: Streamline the semantics of NAME

Summary:
The new rules are straightforward. The main rules to keep in mind
are:

1. NAME is an implicit template argument of class and multiclass,
   and will be substituted by the name of the instantiating def/defm.

2. The name of a def/defm in a multiclass must contain a reference
   to NAME. If such a reference is not present, it is automatically
   prepended.

And for some additional subtleties, consider these:

3. defm with no name generates a unique name but has no special
   behavior otherwise.

4. def with no name generates an anonymous record, whose name is
   unique but undefined. In particular, the name won't contain a
   reference to NAME.

Keeping rules 1&2 in mind should allow a predictable behavior of
name resolution that is simple to follow.

The old "rules" were rather surprising: sometimes (but not always),
NAME would correspond to the name of the toplevel defm. They were
also plain bonkers when you pushed them to their limits, as the old
version of the TableGen test case shows.

Having NAME correspond to the name of the toplevel defm introduces
"spooky action at a distance" and breaks composability:
refactoring the upper layers of a hierarchy of nested multiclass
instantiations can cause unexpected breakage by changing the value
of NAME at a lower level of the hierarchy. The new rules don't
suffer from this problem.

Some existing .td files have to be adjusted because they ended up
depending on the details of the old implementation.

Change-Id: I694095231565b30f563e6fd0417b41ee01a12589

Reviewers: tra, simon_tatham, craig.topper, MartinO, arsenm, javed.absar

Subscribers: wdng, llvm-commits

Differential Revision: https://reviews.llvm.org/D47430

3 files changed, 119 insertions, 162 deletions

diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index 4f1ca04316b..867dd006ed6 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -1572,10 +1572,8 @@ DefInit *VarDefInit::instantiate() {
     Record *NewRec = NewRecOwner.get();
 
     // Copy values from class to instance
-    for (const RecordVal &Val : Class->getValues()) {
-      if (Val.getName() != "NAME")
-        NewRec->addValue(Val);
-    }
+    for (const RecordVal &Val : Class->getValues())
+      NewRec->addValue(Val);
 
     // Substitute and resolve template arguments
     ArrayRef<Init *> TArgs = Class->getTemplateArgs();
@@ -1845,14 +1843,6 @@ void RecordVal::print(raw_ostream &OS, bool PrintSem) const {
 
 unsigned Record::LastID = 0;
 
-void Record::init() {
-  checkName();
-
-  // Every record potentially has a def at the top.  This value is
-  // replaced with the top-level def name at instantiation time.
-  addValue(RecordVal(StringInit::get("NAME"), StringRecTy::get(), false));
-}
-
 void Record::checkName() {
   // Ensure the record name has string type.
   const TypedInit *TypedName = cast<const TypedInit>(Name);
@@ -2260,3 +2250,10 @@ Init *TrackUnresolvedResolver::resolve(Init *VarName) {
     FoundUnresolved = true;
   return I;
 }
+
+Init *HasReferenceResolver::resolve(Init *VarName)
+{
+  if (VarName == VarNameToTrack)
+    Found = true;
+  return nullptr;
+}
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 223d1337322..a0ec4dbfdc7 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -110,6 +110,16 @@ static void checkConcrete(Record &R) {
   }
 }
 
+/// Return the qualified version of the implicit 'NAME' template argument.
+static Init *QualifiedNameOfImplicitName(Record &Rec,
+                                         MultiClass *MC = nullptr) {
+  return QualifyName(Rec, MC, StringInit::get("NAME"), MC ? "::" : ":");
+}
+
+static Init *QualifiedNameOfImplicitName(MultiClass *MC) {
+  return QualifiedNameOfImplicitName(MC->Rec, MC);
+}
+
 bool TGParser::AddValue(Record *CurRec, SMLoc Loc, const RecordVal &RV) {
   if (!CurRec)
     CurRec = &CurMultiClass->Rec;
@@ -234,6 +244,14 @@ bool TGParser::AddSubClass(Record *CurRec, SubClassReference &SubClass) {
     CurRec->removeValue(TArgs[i]);
   }
 
+  Init *Name;
+  if (CurRec->isClass())
+    Name =
+        VarInit::get(QualifiedNameOfImplicitName(*CurRec), StringRecTy::get());
+  else
+    Name = CurRec->getNameInit();
+  R.set(QualifiedNameOfImplicitName(*SC), Name);
+
   CurRec->resolveReferences(R);
 
   // Since everything went well, we can now set the "superclass" list for the
@@ -259,80 +277,45 @@ bool TGParser::AddSubClass(Record *CurRec, SubClassReference &SubClass) {
 bool TGParser::AddSubMultiClass(MultiClass *CurMC,
                                 SubMultiClassReference &SubMultiClass) {
   MultiClass *SMC = SubMultiClass.MC;
-  Record *CurRec = &CurMC->Rec;
-
-  // Add all of the values in the subclass into the current class.
-  for (const auto &SMCVal : SMC->Rec.getValues())
-    if (AddValue(CurRec, SubMultiClass.RefRange.Start, SMCVal))
-      return true;
-
-  unsigned newDefStart = CurMC->DefPrototypes.size();
-
-  // Add all of the defs in the subclass into the current multiclass.
-  for (const std::unique_ptr<Record> &R : SMC->DefPrototypes) {
-    // Clone the def and add it to the current multiclass
-    auto NewDef = make_unique<Record>(*R);
-
-    // Add all of the values in the superclass into the current def.
-    for (const auto &MCVal : CurRec->getValues())
-      if (AddValue(NewDef.get(), SubMultiClass.RefRange.Start, MCVal))
-        return true;
-
-    CurMC->DefPrototypes.push_back(std::move(NewDef));
-  }
 
   ArrayRef<Init *> SMCTArgs = SMC->Rec.getTemplateArgs();
-
-  // Ensure that an appropriate number of template arguments are
-  // specified.
   if (SMCTArgs.size() < SubMultiClass.TemplateArgs.size())
     return Error(SubMultiClass.RefRange.Start,
                  "More template args specified than expected");
 
-  // Loop over all of the template arguments, setting them to the specified
-  // value or leaving them as the default if necessary.
-  MapResolver CurRecResolver(CurRec);
-
+  // Prepare the mapping of template argument name to value, filling in default
+  // values if necessary.
+  SmallVector<std::pair<Init *, Init *>, 8> TemplateArgs;
   for (unsigned i = 0, e = SMCTArgs.size(); i != e; ++i) {
     if (i < SubMultiClass.TemplateArgs.size()) {
-      // If a value is specified for this template arg, set it in the
-      // superclass now.
-      if (SetValue(CurRec, SubMultiClass.RefRange.Start, SMCTArgs[i],
-                   None, SubMultiClass.TemplateArgs[i]))
-        return true;
-
-      // If a value is specified for this template arg, set it in the
-      // new defs now.
-      for (const auto &Def :
-             makeArrayRef(CurMC->DefPrototypes).slice(newDefStart)) {
-        if (SetValue(Def.get(), SubMultiClass.RefRange.Start, SMCTArgs[i],
-                     None, SubMultiClass.TemplateArgs[i]))
-          return true;
+      TemplateArgs.emplace_back(SMCTArgs[i], SubMultiClass.TemplateArgs[i]);
+    } else {
+      Init *Default = SMC->Rec.getValue(SMCTArgs[i])->getValue();
+      if (!Default->isComplete()) {
+        return Error(SubMultiClass.RefRange.Start,
+                     "value not specified for template argument #" + Twine(i) +
+                         " (" + SMCTArgs[i]->getAsUnquotedString() +
+                         ") of multiclass '" + SMC->Rec.getNameInitAsString() +
+                         "'");
       }
-    } else if (!CurRec->getValue(SMCTArgs[i])->getValue()->isComplete()) {
-      return Error(SubMultiClass.RefRange.Start,
-                   "Value not specified for template argument #" +
-                   Twine(i) + " (" + SMCTArgs[i]->getAsUnquotedString() +
-                   ") of subclass '" + SMC->Rec.getNameInitAsString() + "'!");
+      TemplateArgs.emplace_back(SMCTArgs[i], Default);
     }
-
-    CurRecResolver.set(SMCTArgs[i], CurRec->getValue(SMCTArgs[i])->getValue());
-
-    CurRec->removeValue(SMCTArgs[i]);
   }
 
-  CurRec->resolveReferences(CurRecResolver);
+  TemplateArgs.emplace_back(
+      QualifiedNameOfImplicitName(SMC),
+      VarInit::get(QualifiedNameOfImplicitName(CurMC), StringRecTy::get()));
 
-  for (const auto &Def :
-       makeArrayRef(CurMC->DefPrototypes).slice(newDefStart)) {
-    MapResolver R(Def.get());
+  // Add all of the defs in the subclass into the current multiclass.
+  for (const std::unique_ptr<Record> &Rec : SMC->DefPrototypes) {
+    auto NewDef = make_unique<Record>(*Rec);
 
-    for (Init *SMCTArg : SMCTArgs) {
-      R.set(SMCTArg, Def->getValue(SMCTArg)->getValue());
-      Def->removeValue(SMCTArg);
-    }
+    MapResolver R(NewDef.get());
+    for (const auto &TArg : TemplateArgs)
+      R.set(TArg.first, TArg.second);
+    NewDef->resolveReferences(R);
 
-    Def->resolveReferences(R);
+    CurMC->DefPrototypes.push_back(std::move(NewDef));
   }
 
   return false;
@@ -343,18 +326,18 @@ bool TGParser::AddSubMultiClass(MultiClass *CurMC,
 ///
 /// Apply foreach loops, resolve internal variable references, and add to the
 /// current multi class or the global record keeper as appropriate.
-bool TGParser::addDef(std::unique_ptr<Record> Rec, Init *DefmName) {
+bool TGParser::addDef(std::unique_ptr<Record> Rec) {
   IterSet IterVals;
 
   if (Loops.empty())
-    return addDefOne(std::move(Rec), DefmName, IterVals);
+    return addDefOne(std::move(Rec), IterVals);
 
-  return addDefForeach(Rec.get(), DefmName, IterVals);
+  return addDefForeach(Rec.get(), IterVals);
 }
 
 /// Recursive helper function for addDef/addDefOne to resolve references to
 /// foreach variables.
-bool TGParser::addDefForeach(Record *Rec, Init *DefmName, IterSet &IterVals) {
+bool TGParser::addDefForeach(Record *Rec, IterSet &IterVals) {
   if (IterVals.size() != Loops.size()) {
     assert(IterVals.size() < Loops.size());
     ForeachLoop &CurLoop = Loops[IterVals.size()];
@@ -363,7 +346,7 @@ bool TGParser::addDefForeach(Record *Rec, Init *DefmName, IterSet &IterVals) {
     // Process each value.
     for (unsigned i = 0; i < List->size(); ++i) {
       IterVals.push_back(IterRecord(CurLoop.IterVar, List->getElement(i)));
-      if (addDefForeach(Rec, DefmName, IterVals))
+      if (addDefForeach(Rec, IterVals))
         return true;
       IterVals.pop_back();
     }
@@ -374,13 +357,12 @@ bool TGParser::addDefForeach(Record *Rec, Init *DefmName, IterSet &IterVals) {
   // for this point in the iteration space.  Instantiate a new record to
   // reflect this combination of values.
   auto IterRec = make_unique<Record>(*Rec);
-  return addDefOne(std::move(IterRec), DefmName, IterVals);
+  return addDefOne(std::move(IterRec), IterVals);
 }
 
 /// After resolving foreach loops, add the record as a prototype to the
 /// current multiclass, or resolve fully and add to the record keeper.
-bool TGParser::addDefOne(std::unique_ptr<Record> Rec, Init *DefmName,
-                         IterSet &IterVals) {
+bool TGParser::addDefOne(std::unique_ptr<Record> Rec, IterSet &IterVals) {
   MapResolver R(Rec.get());
 
   for (IterRecord &IR : IterVals)
@@ -389,32 +371,21 @@ bool TGParser::addDefOne(std::unique_ptr<Record> Rec, Init *DefmName,
   Rec->resolveReferences(R);
 
   if (CurMultiClass) {
-    for (const auto &Proto : CurMultiClass->DefPrototypes) {
-      if (Proto->getNameInit() == Rec->getNameInit()) {
-        if (!Rec->isAnonymous()) {
+    if (!Rec->isAnonymous()) {
+      for (const auto &Proto : CurMultiClass->DefPrototypes) {
+        if (Proto->getNameInit() == Rec->getNameInit()) {
           PrintError(Rec->getLoc(),
                     Twine("def '") + Rec->getNameInitAsString() +
                         "' already defined in this multiclass!");
           PrintNote(Proto->getLoc(), "location of previous definition");
           return true;
         }
-        Rec->setName(Records.getNewAnonymousName());
-        break;
       }
     }
     CurMultiClass->DefPrototypes.emplace_back(std::move(Rec));
     return false;
   }
 
-  // Name construction is an incoherent mess. Unfortunately, existing .td
-  // files rely on pretty much all the quirks and implementation details of
-  // this.
-  if (DefmName) {
-    MapResolver R(Rec.get());
-    R.set(StringInit::get("NAME"), DefmName);
-    Rec->resolveReferences(R);
-  }
-
   if (Record *Prev = Records.getDef(Rec->getNameInitAsString())) {
     if (!Rec->isAnonymous()) {
       PrintError(Rec->getLoc(),
@@ -488,7 +459,20 @@ Init *TGParser::ParseObjectName(MultiClass *CurMultiClass) {
   if (CurMultiClass)
     CurRec = &CurMultiClass->Rec;
 
-  return ParseValue(CurRec, StringRecTy::get(), ParseNameMode);
+  Init *Name = ParseValue(CurRec, StringRecTy::get(), ParseNameMode);
+  if (!Name)
+    return nullptr;
+
+  if (CurMultiClass) {
+    Init *NameStr = QualifiedNameOfImplicitName(CurMultiClass);
+    HasReferenceResolver R(NameStr);
+    Name->resolveReferences(R);
+    if (!R.found())
+      Name = BinOpInit::getStrConcat(VarInit::get(NameStr, StringRecTy::get()),
+                                     Name);
+  }
+
+  return Name;
 }
 
 /// ParseClassID - Parse and resolve a reference to a class name.  This returns
@@ -798,30 +782,23 @@ Init *TGParser::ParseIDValue(Record *CurRec, StringInit *Name, SMLoc NameLoc,
   if (CurRec) {
     if (const RecordVal *RV = CurRec->getValue(Name))
       return VarInit::get(Name, RV->getType());
-
-    Init *TemplateArgName = QualifyName(*CurRec, CurMultiClass, Name, ":");
-
-    if (CurMultiClass)
-      TemplateArgName = QualifyName(CurMultiClass->Rec, CurMultiClass, Name,
-                                    "::");
-
-    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 ((CurRec && CurRec->isClass()) || CurMultiClass) {
+    Init *TemplateArgName;
+    if (CurMultiClass) {
+      TemplateArgName =
+          QualifyName(CurMultiClass->Rec, CurMultiClass, Name, "::");
+    } else
+      TemplateArgName = QualifyName(*CurRec, CurMultiClass, Name, ":");
 
-    if (CurMultiClass->Rec.isTemplateArg(MCName)) {
-      const RecordVal *RV = CurMultiClass->Rec.getValue(MCName);
+    Record *TemplateRec = CurMultiClass ? &CurMultiClass->Rec : CurRec;
+    if (TemplateRec->isTemplateArg(TemplateArgName)) {
+      const RecordVal *RV = TemplateRec->getValue(TemplateArgName);
       assert(RV && "Template arg doesn't exist??");
-      return VarInit::get(MCName, RV->getType());
+      return VarInit::get(TemplateArgName, RV->getType());
+    } else if (Name->getValue() == "NAME") {
+      return VarInit::get(TemplateArgName, StringRecTy::get());
     }
   }
 
@@ -840,15 +817,12 @@ Init *TGParser::ParseIDValue(Record *CurRec, StringInit *Name, SMLoc NameLoc,
 
   // Allow self-references of concrete defs, but delay the lookup so that we
   // get the correct type.
-  if (CurRec && !CurMultiClass && CurRec->getNameInit() == Name)
+  if (CurRec && !CurRec->isClass() && !CurMultiClass &&
+      CurRec->getNameInit() == Name)
     return UnOpInit::get(UnOpInit::CAST, Name, CurRec->getType());
 
-  if (Mode == ParseValueMode) {
-    Error(NameLoc, "Variable not defined: '" + Name->getValue() + "'");
-    return nullptr;
-  }
-
-  return Name;
+  Error(NameLoc, "Variable not defined: '" + Name->getValue() + "'");
+  return nullptr;
 }
 
 /// ParseOperation - Parse an operator.  This returns null on error.
@@ -2169,8 +2143,14 @@ Init *TGParser::ParseDeclaration(Record *CurRec,
     return nullptr;
   }
 
+  std::string Str = Lex.getCurStrVal();
+  if (Str == "NAME") {
+    TokError("'" + Str + "' is a reserved variable name");
+    return nullptr;
+  }
+
   SMLoc IdLoc = Lex.getLoc();
-  Init *DeclName = StringInit::get(Lex.getCurStrVal());
+  Init *DeclName = StringInit::get(Str);
   Lex.Lex();
 
   if (ParsingTemplateArgs) {
@@ -2471,7 +2451,7 @@ bool TGParser::ParseDef(MultiClass *CurMultiClass) {
   if (ParseObjectBody(CurRec.get()))
     return true;
 
-  return addDef(std::move(CurRec), nullptr);
+  return addDef(std::move(CurRec));
 }
 
 /// ParseDefset - Parse a defset statement.
@@ -2585,7 +2565,7 @@ bool TGParser::ParseClass() {
   Record *CurRec = Records.getClass(Lex.getCurStrVal());
   if (CurRec) {
     // If the body was previously defined, this is an error.
-    if (CurRec->getValues().size() > 1 ||  // Account for NAME.
+    if (!CurRec->getValues().empty() ||
         !CurRec->getSuperClasses().empty() ||
         !CurRec->getTemplateArgs().empty())
       return TokError("Class '" + CurRec->getNameInitAsString() +
@@ -2593,7 +2573,8 @@ bool TGParser::ParseClass() {
   } else {
     // If this is the first reference to this class, create and add it.
     auto NewRec =
-        llvm::make_unique<Record>(Lex.getCurStrVal(), Lex.getLoc(), Records);
+        llvm::make_unique<Record>(Lex.getCurStrVal(), Lex.getLoc(), Records,
+                                  /*Class=*/true);
     CurRec = NewRec.get();
     Records.addClass(std::move(NewRec));
   }
@@ -2604,7 +2585,6 @@ bool TGParser::ParseClass() {
     if (ParseTemplateArgList(CurRec))
       return true;
 
-  // Finally, parse the object body.
   return ParseObjectBody(CurRec);
 }
 
@@ -2802,8 +2782,14 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
   Init *DefmName = ParseObjectName(CurMultiClass);
   if (!DefmName)
     return true;
-  if (isa<UnsetInit>(DefmName))
+  if (isa<UnsetInit>(DefmName)) {
     DefmName = Records.getNewAnonymousName();
+    if (CurMultiClass)
+      DefmName = BinOpInit::getStrConcat(
+          VarInit::get(QualifiedNameOfImplicitName(CurMultiClass),
+                       StringRecTy::get()),
+          DefmName);
+  }
 
   if (Lex.getCode() != tgtok::colon)
     return TokError("expected ':' after defm identifier");
@@ -2836,10 +2822,10 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
       return Error(SubClassLoc,
                    "more template args specified than multiclass expects");
 
-    DenseMap<Init *, Init *> TemplateArgs;
+    SmallVector<std::pair<Init *, Init *>, 8> TemplateArgs;
     for (unsigned i = 0, e = TArgs.size(); i != e; ++i) {
       if (i < TemplateVals.size()) {
-        TemplateArgs.insert({TArgs[i], TemplateVals[i]});
+        TemplateArgs.emplace_back(TArgs[i], TemplateVals[i]);
       } else {
         Init *Default = MC->Rec.getValue(TArgs[i])->getValue();
         if (!Default->isComplete()) {
@@ -2849,45 +2835,20 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
                            ") of multiclass '" + MC->Rec.getNameInitAsString() +
                            "'");
         }
-        TemplateArgs.insert({TArgs[i], Default});
+        TemplateArgs.emplace_back(TArgs[i], Default);
       }
     }
 
+    TemplateArgs.emplace_back(QualifiedNameOfImplicitName(MC), DefmName);
+
     // Loop over all the def's in the multiclass, instantiating each one.
     for (const std::unique_ptr<Record> &DefProto : MC->DefPrototypes) {
-      bool ResolveName = true;
       auto CurRec = make_unique<Record>(*DefProto);
       CurRec->appendLoc(SubClassLoc);
 
-      if (StringInit *NameString =
-              dyn_cast<StringInit>(CurRec->getNameInit())) {
-        // We have a fully expanded string so there are no operators to
-        // resolve.  We should concatenate the given prefix and name.
-        //
-        // TODO: This MUST happen before template argument resolution. This
-        //       does not make sense and should be changed, but at the time of
-        //       writing, there are existing .td files which rely on this
-        //       implementation detail. It's a bad idea and should be fixed.
-        //       See test/TableGen/name-resolution-consistency.td for some
-        //       examples.
-        CurRec->setName(BinOpInit::getStrConcat(DefmName, NameString));
-        ResolveName = false;
-      }
-
       MapResolver R(CurRec.get());
-
-      if (ResolveName) {
-        // If the proto's name wasn't resolved, we probably have a reference to
-        // NAME and need to replace it.
-        //
-        // TODO: Whether the name is resolved is basically determined by magic.
-        //       Unfortunately, existing .td files depend on it.
-        R.set(StringInit::get("NAME"), DefmName);
-      }
-
       for (const auto &TArg : TemplateArgs)
         R.set(TArg.first, TArg.second);
-
       CurRec->resolveReferences(R);
 
       NewRecDefs.emplace_back(std::move(CurRec));
@@ -2937,7 +2898,7 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
     if (ApplyLetStack(CurRec.get()))
       return true;
 
-    addDef(std::move(CurRec), DefmName);
+    addDef(std::move(CurRec));
   }
 
   if (Lex.getCode() != tgtok::semi)
diff --git a/llvm/lib/TableGen/TGParser.h b/llvm/lib/TableGen/TGParser.h
index 7948df27208..a457aab3ff6 100644
--- a/llvm/lib/TableGen/TGParser.h
+++ b/llvm/lib/TableGen/TGParser.h
@@ -126,10 +126,9 @@ private:  // Semantic analysis methods.
   // iteration space.
   typedef std::vector<IterRecord> IterSet;
 
-  bool addDefOne(std::unique_ptr<Record> Rec, Init *DefmName,
-                 IterSet &IterVals);
-  bool addDefForeach(Record *Rec, Init *DefmName, IterSet &IterVals);
-  bool addDef(std::unique_ptr<Record> Rec, Init *DefmName);
+  bool addDefOne(std::unique_ptr<Record> Rec, IterSet &IterVals);
+  bool addDefForeach(Record *Rec, IterSet &IterVals);
+  bool addDef(std::unique_ptr<Record> Rec);
 
 private:  // Parser methods.
   bool ParseObjectList(MultiClass *MC = nullptr);