[NFC][TableGen] Print topmost def location for Intrinsics/Insts

[jurahul]

When printing location for an intrinsic or an instruction in the generated file, print the location of the topmost/outermost defm. The intent of this location is to easily trace the origin of that particular record, and currently we print the location of the innermost def for records defined using defm/multiclasses, which is not that useful. Printing the location of the topmost defm can allow someone to drill down the defm hierarchy.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

When printing location for an intrinsic or an instruction in the generated file, print the location of the topmost/outermost defm. The intent of this location is to easily trace the origin of that particular record, and currently we print the location of the innermost def for records defined using defm/multiclasses, which is not that useful. Printing the location of the topmost defm can allow someone to drill down the defm hierarchy.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/TableGen/Record.h (+6)
• (modified) llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp (+1-1)
• (modified) llvm/utils/TableGen/InstrInfoEmitter.cpp (+1-1)

diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index d4fa1e5d65749b..18aa313a88500f 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1720,6 +1720,12 @@ class Record {
   ArrayRef<SMLoc> getLoc() const { return Locs; }
   void appendLoc(SMLoc Loc) { Locs.push_back(Loc); }
 
+  // Returns the location of the "top" def or defm that instantiated this
+  // concrete record. For a record defined using `def`, this is the location of
+  // the def. For a record defined using `defm`, this is the location of the
+  // topmost/outermost defm that lead to the instantiation of this record.
+  SMLoc getTopDefLoc() const { return Locs.back(); }
+
   ArrayRef<SMLoc> getForwardDeclarationLocs() const {
     return ForwardDeclarationLocs;
   }
diff --git a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
index 559868dd54efe8..1ae035434d9e54 100644
--- a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
@@ -170,7 +170,7 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
       OS.indent(40 - Int.EnumName.size());
     OS << formatv(
         " // {} ({})\n", Int.Name,
-        SrcMgr.getFormattedLocationNoOffset(Int.TheDef->getLoc().front()));
+        SrcMgr.getFormattedLocationNoOffset(Int.TheDef->getTopDefLoc()));
   }
 
   // Emit num_intrinsics into the target neutral enum.
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 176e4b250b82a7..9b92aeb082dbc6 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -1389,7 +1389,7 @@ void InstrInfoEmitter::emitEnums(
   for (const CodeGenInstruction *Inst : NumberedInstructions) {
     OS << "    " << left_justify(Inst->getName(), MaxNameSize) << " = "
        << Target.getInstrIntValue(Inst->TheDef) << ", // "
-       << SrcMgr.getFormattedLocationNoOffset(Inst->TheDef->getLoc().front())
+       << SrcMgr.getFormattedLocationNoOffset(Inst->TheDef->getTopDefLoc())
        << '\n';
   }
   OS << "    INSTRUCTION_LIST_END = " << NumberedInstructions.size() << '\n';

[jurahul]

@topperc this address your comment on #156927 and make the locations printed more useful.

[jayfoad]

It's not at all clear to me that this is better when there are multiple levels of multiclasses involved. Drilling down the defm hierarchy can involve just as much guesswork as drilling "up". C++ compilers tend to print the whole stack of locations, for analogous situations like nested template instantiations.

[jurahul]

Are you suggesting we print the whole stack? It's just more comments, but maybe possible. We can make it more compact as well by commoning file name when possible.

[jayfoad]

Are you suggesting we print the whole stack? It's just more comments, but maybe possible. We can make it more compact as well by commoning file name when possible.

Oh, I assumed this was for diagnostics, not for comments.

If it's just for comments then I don't mind too much. I'm fine with this version if it happens to make the output more useful for X86.

[jurahul]

Right, for actual tablegen errors ot warnings, we do print the entire stack. The intent of this comment is to give someone a starting seed location to hunt down a particular instruction or intrinsic record