[TableGen] Fix source location for anonymous records

[jurahul]

Fix source location for anonymous records to be the one of the locations where that record is instantiated as opposed to the location of the class that was anonymously instantiated.

Currently, when a record is anonymously instantiated (via VarDefInit::instantiate), we use the location of the class for the record, which is not correct. Instead, pass in the SMLoc for the location where the anonymous instantiation happens and use that location when the record is instantiated. If there are multiple anonymous instantiations with the same parameters, the location for the (single) record created will be one of these instantiation locations as opposed to the class location.

[llvmbot]

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-testing-tools

Author: Rahul Joshi (jurahul)

Changes

Fix source location for anonymous records to be the one of the locations where that record is instantiated as opposed to the location of the class that was anonymously instantiated.

Currently, when a record is anonymously instantiated (via VarDefInit::instantiate), we use the location of the class for the record, which is not correct. Instead, pass in the SMLoc for the location where the anonymous instantiation happens and use that location when the record is instantiated. If there are multiple anonymous instantiations with the same parameters, the location for the (single) record created will be one of these instantiation locations as opposed to the class location.

Added unit test using detailed record printer. Since it only prints the base name of the file when printing locations, also added support for %basename_s in LLVM's LIT testing framework to get the base name of the source file.

---

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

6 Files Affected:

• (modified) llvm/docs/CommandGuide/lit.rst (+1)
• (modified) llvm/include/llvm/TableGen/Record.h (+4-2)
• (modified) llvm/lib/TableGen/Record.cpp (+8-8)
• (modified) llvm/lib/TableGen/TGParser.cpp (+1-1)
• (added) llvm/test/TableGen/anonymous-location.td (+16)
• (modified) llvm/utils/lit/lit/TestRunner.py (+4-2)

diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst
index c9d5baba3e2f49..9216eb223d1491 100644
--- a/llvm/docs/CommandGuide/lit.rst
+++ b/llvm/docs/CommandGuide/lit.rst
@@ -535,6 +535,7 @@ TestRunner.py:
  %{fs-tmp-root}          root component of file system paths pointing to the test's temporary directory
  %{fs-sep}               file system path separator
  %t                      temporary file name unique to the test
+ %basename_s             The last path component of %s
  %basename_t             The last path component of %t but without the ``.tmp`` extension
  %T                      parent directory of %t (not unique, deprecated, do not use)
  %%                      %
diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index 106fee39cb9f64..ed9855e0180ebd 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1347,11 +1347,12 @@ class DefInit : public TypedInit {
 class VarDefInit final : public TypedInit,
                          public FoldingSetNode,
                          public TrailingObjects<VarDefInit, ArgumentInit *> {
+  SMLoc Loc;
   Record *Class;
   DefInit *Def = nullptr; // after instantiation
   unsigned NumArgs;
 
-  explicit VarDefInit(Record *Class, unsigned N);
+  explicit VarDefInit(SMLoc Loc, Record *Class, unsigned N);
 
   DefInit *instantiate();
 
@@ -1365,7 +1366,8 @@ class VarDefInit final : public TypedInit,
   static bool classof(const Init *I) {
     return I->getKind() == IK_VarDefInit;
   }
-  static VarDefInit *get(Record *Class, ArrayRef<ArgumentInit *> Args);
+  static VarDefInit *get(SMLoc Loc, Record *Class,
+                         ArrayRef<ArgumentInit *> Args);
 
   void Profile(FoldingSetNodeID &ID) const;
 
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index c0c89836171b12..e432949ef360be 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -2292,11 +2292,12 @@ static void ProfileVarDefInit(FoldingSetNodeID &ID, Record *Class,
     ID.AddPointer(I);
 }
 
-VarDefInit::VarDefInit(Record *Class, unsigned N)
-    : TypedInit(IK_VarDefInit, RecordRecTy::get(Class)), Class(Class),
+VarDefInit::VarDefInit(SMLoc Loc, Record *Class, unsigned N)
+    : TypedInit(IK_VarDefInit, RecordRecTy::get(Class)), Loc(Loc), Class(Class),
       NumArgs(N) {}
 
-VarDefInit *VarDefInit::get(Record *Class, ArrayRef<ArgumentInit *> Args) {
+VarDefInit *VarDefInit::get(SMLoc Loc, Record *Class,
+                            ArrayRef<ArgumentInit *> Args) {
   FoldingSetNodeID ID;
   ProfileVarDefInit(ID, Class, Args);
 
@@ -2307,7 +2308,7 @@ VarDefInit *VarDefInit::get(Record *Class, ArrayRef<ArgumentInit *> Args) {
 
   void *Mem = RK.Allocator.Allocate(
       totalSizeToAlloc<ArgumentInit *>(Args.size()), alignof(VarDefInit));
-  VarDefInit *I = new (Mem) VarDefInit(Class, Args.size());
+  VarDefInit *I = new (Mem) VarDefInit(Loc, Class, Args.size());
   std::uninitialized_copy(Args.begin(), Args.end(),
                           I->getTrailingObjects<ArgumentInit *>());
   RK.TheVarDefInitPool.InsertNode(I, IP);
@@ -2323,9 +2324,8 @@ DefInit *VarDefInit::instantiate() {
     return Def;
 
   RecordKeeper &Records = Class->getRecords();
-  auto NewRecOwner =
-      std::make_unique<Record>(Records.getNewAnonymousName(), Class->getLoc(),
-                               Records, Record::RK_AnonymousDef);
+  auto NewRecOwner = std::make_unique<Record>(
+      Records.getNewAnonymousName(), Loc, Records, Record::RK_AnonymousDef);
   Record *NewRec = NewRecOwner.get();
 
   // Copy values from class to instance
@@ -2389,7 +2389,7 @@ Init *VarDefInit::resolveReferences(Resolver &R) const {
   }
 
   if (Changed) {
-    auto New = VarDefInit::get(Class, NewArgs);
+    auto New = VarDefInit::get(Loc, Class, NewArgs);
     if (!UR.foundUnresolved())
       return New->instantiate();
     return New;
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 6793205e09380f..2df84742c73b99 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -2719,7 +2719,7 @@ Init *TGParser::ParseSimpleValue(Record *CurRec, const RecTy *ItemType,
 
     if (TrackReferenceLocs)
       Class->appendReferenceLoc(NameLoc);
-    return VarDefInit::get(Class, Args)->Fold();
+    return VarDefInit::get(NameLoc.Start, Class, Args)->Fold();
   }
   case tgtok::l_brace: {           // Value ::= '{' ValueList '}'
     SMLoc BraceLoc = Lex.getLoc();
diff --git a/llvm/test/TableGen/anonymous-location.td b/llvm/test/TableGen/anonymous-location.td
new file mode 100644
index 00000000000000..5485d2027b9fd0
--- /dev/null
+++ b/llvm/test/TableGen/anonymous-location.td
@@ -0,0 +1,16 @@
+// RUN: llvm-tblgen --print-detailed-records %s | FileCheck %s -DFILE=%basename_s
+
+class A<int a> {
+  int Num = a;
+}
+
+// Verify that the location of the anonymous record instantiated
+// for A<10> and A<11> is correct. It should show the line where the
+// anonymous record was instantiated and not the line where the class
+// was defined.
+def y {
+  // CHECK: anonymous_0 |[[FILE]]:[[@LINE+1]]|
+  int x = A<10>.Num;
+  // CHECK: anonymous_1 |[[FILE]]:[[@LINE+1]]|
+  int y = A<11>.Num;
+}
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index a1785073547ad0..159d25bf4245be 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1394,10 +1394,12 @@ def getDefaultSubstitutions(test, tmpDir, tmpBase, normalize_slashes=False):
     substitutions = []
     substitutions.extend(test.config.substitutions)
     tmpName = tmpBase + ".tmp"
-    baseName = os.path.basename(tmpBase)
+    tmpBaseName = os.path.basename(tmpBase)
+    srcBaseName = os.path.basename(sourcepath)
 
     substitutions.append(("%{pathsep}", os.pathsep))
-    substitutions.append(("%basename_t", baseName))
+    substitutions.append(("%basename_t", tmpBaseName))
+    substitutions.append(("%basename_s", srcBaseName))
 
     substitutions.extend(
         [