[llvm] e0458a2 - [LLVM][TableGen] Add error check for duplicate intrinsic names (#109226)

[via llvm-commits]

Author: Rahul Joshi
Date: 2024-09-19T05:21:00-07:00
New Revision: e0458a24a1d026d1666f82d671e579d5fdce0027

URL: https://github.com/llvm/llvm-project/commit/e0458a24a1d026d1666f82d671e579d5fdce0027
DIFF: https://github.com/llvm/llvm-project/commit/e0458a24a1d026d1666f82d671e579d5fdce0027.diff

LOG: [LLVM][TableGen] Add error check for duplicate intrinsic names (#109226)

Check for duplicate intrinsic names in the intrinsic emitter backend and
issue a fatal error if we find one.

Added: 
    llvm/test/TableGen/intrinsic-duplicate-name.td

Modified: 
    llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
    llvm/utils/TableGen/Basic/CodeGenIntrinsics.h

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/intrinsic-duplicate-name.td b/llvm/test/TableGen/intrinsic-duplicate-name.td
new file mode 100644
index 00000000000000..64d94d3192541d
--- /dev/null
+++ b/llvm/test/TableGen/intrinsic-duplicate-name.td
@@ -0,0 +1,9 @@
+// RUN: not llvm-tblgen -gen-intrinsic-impl -I %p/../../include %s -DTEST_INTRINSICS_SUPPRESS_DEFS 2>&1 | FileCheck %s -DFILE=%s
+
+include "llvm/IR/Intrinsics.td"
+
+def int_foo0 : Intrinsic<[llvm_anyint_ty], [], [], "llvm.foo">;
+
+// CHECK: [[FILE]]:[[@LINE+2]]:5: error: Intrinsic `llvm.foo` is already defined
+// CHECK: [[FILE]]:[[@LINE-3]]:5: note: Previous definition here
+def int_foo1 : Intrinsic<[llvm_anyint_ty], [], [], "llvm.foo">;

diff  --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index 05104e938b8486..e566b7ceedf38e 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -47,13 +47,18 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
   Intrinsics.reserve(Defs.size());
 
   for (const Record *Def : Defs)
-    Intrinsics.push_back(CodeGenIntrinsic(Def, Ctx));
+    Intrinsics.emplace_back(CodeGenIntrinsic(Def, Ctx));
 
+  // To ensure deterministic sorted order when duplicates are present, use
+  // record ID as a tie-breaker similar to sortAndReportDuplicates in Utils.cpp.
   llvm::sort(Intrinsics,
              [](const CodeGenIntrinsic &LHS, const CodeGenIntrinsic &RHS) {
-               return std::tie(LHS.TargetPrefix, LHS.Name) <
-                      std::tie(RHS.TargetPrefix, RHS.Name);
+               unsigned LhsID = LHS.TheDef->getID();
+               unsigned RhsID = RHS.TheDef->getID();
+               return std::tie(LHS.TargetPrefix, LHS.Name, LhsID) <
+                      std::tie(RHS.TargetPrefix, RHS.Name, RhsID);
              });
+
   Targets.push_back({"", 0, 0});
   for (size_t I = 0, E = Intrinsics.size(); I < E; ++I)
     if (Intrinsics[I].TargetPrefix != Targets.back().Name) {
@@ -61,6 +66,32 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
       Targets.push_back({Intrinsics[I].TargetPrefix, I, 0});
     }
   Targets.back().Count = Intrinsics.size() - Targets.back().Offset;
+
+  CheckDuplicateIntrinsics();
+}
+
+// Check for duplicate intrinsic names.
+void CodeGenIntrinsicTable::CheckDuplicateIntrinsics() const {
+  // Since the Intrinsics vector is already sorted by name, if there are 2 or
+  // more intrinsics with duplicate names, they will appear adjacent in sorted
+  // order. Note that if the intrinsic name was derived from the record name
+  // there cannot be be duplicate as TableGen parser would have flagged that.
+  // However, if the name was specified in the intrinsic definition, then its
+  // possible to have duplicate names.
+  auto I = std::adjacent_find(
+      Intrinsics.begin(), Intrinsics.end(),
+      [](const CodeGenIntrinsic &Int1, const CodeGenIntrinsic &Int2) {
+        return Int1.Name == Int2.Name;
+      });
+  if (I == Intrinsics.end())
+    return;
+
+  // Found a duplicate intrinsics.
+  const CodeGenIntrinsic &First = *I;
+  const CodeGenIntrinsic &Second = *(I + 1);
+  PrintError(Second.TheDef,
+             Twine("Intrinsic `") + First.Name + "` is already defined");
+  PrintFatalNote(First.TheDef, "Previous definition here");
 }
 
 CodeGenIntrinsic &CodeGenIntrinsicMap::operator[](const Record *Record) {

diff  --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
index 83282d18789b28..2df598da3f2507 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
@@ -189,6 +189,9 @@ class CodeGenIntrinsicTable {
   const CodeGenIntrinsic &operator[](size_t Pos) const {
     return Intrinsics[Pos];
   }
+
+private:
+  void CheckDuplicateIntrinsics() const;
 };
 
 // This class builds `CodeGenIntrinsic` on demand for a given Def.