[LLVM][TableGen] Check name conflicts between target dep and independent intrinsics

[jurahul]

Validate that for target independent intrinsics the second dotted component of their name (after the llvm.) does not match any existing target names (for which atleast one intrinsic has been defined). Doing so is invalid as LLVM will search for that intrinsic in that target's intrinsic table and not find it, and conclude that its an unknown intrinsic.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Validate that for target independent intrinsics the second dotted component of their name (after the llvm.) does not match any existing target names (for which atleast one intrinsic has been defined). Doing so is invalid as LLVM will search for that intrinsic in that target's intrinsic table and not find it, and conclude that its an unknown intrinsic.

---

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

3 Files Affected:

• (added) llvm/test/TableGen/intrinsic-target-prefix-for-target-independent.td (+9)
• (modified) llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp (+23)
• (modified) llvm/utils/TableGen/Basic/CodeGenIntrinsics.h (+1)

diff --git a/llvm/test/TableGen/intrinsic-target-prefix-for-target-independent.td b/llvm/test/TableGen/intrinsic-target-prefix-for-target-independent.td
new file mode 100644
index 00000000000000..157b9490439fca
--- /dev/null
+++ b/llvm/test/TableGen/intrinsic-target-prefix-for-target-independent.td
@@ -0,0 +1,9 @@
+// RUN: not llvm-tblgen -gen-intrinsic-enums -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+
+include "llvm/IR/Intrinsics.td"
+
+// Check that target independent intrinsics with a prefix that matches a target
+// name are flagged.
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: target independent intrinsic `llvm.aarch64.foo' has prefix `llvm.aarch64` that conflicts with target intrinsics for target `aarch64`
+def int_aarch64_foo : Intrinsic<[],[]>;
+
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index a30dc72a83154a..b727795cb74db5 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -75,6 +75,7 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
   Targets.back().Count = Intrinsics.size() - Targets.back().Offset;
 
   CheckDuplicateIntrinsics();
+  CheckTargetIndependentIntrinsics();
 }
 
 // Check for duplicate intrinsic names.
@@ -101,6 +102,28 @@ void CodeGenIntrinsicTable::CheckDuplicateIntrinsics() const {
   PrintFatalNote(First.TheDef, "Previous definition here");
 }
 
+// For target independent intrinsics, check that their second dotted component
+// does not match any target name.
+void CodeGenIntrinsicTable::CheckTargetIndependentIntrinsics() const {
+  SmallDenseSet<StringRef> TargetNames;
+  for (const auto &Target : ArrayRef(Targets).drop_front())
+    TargetNames.insert(Target.Name);
+
+  // Set of target independent intrinsics.
+  const auto &Set = Targets[0];
+  for (const auto &Int : ArrayRef(&Intrinsics[Set.Offset], Set.Count)) {
+    StringRef Name = Int.Name;
+    StringRef Prefix = Name.drop_front(5).split('.').first;
+    if (!TargetNames.contains(Prefix))
+      continue;
+    PrintFatalError(Int.TheDef,
+                    "target independent intrinsic `" + Name +
+                        "' has prefix `llvm." + Prefix +
+                        "` that conflicts with target intrinsics for target `" +
+                        Prefix + "`");
+  }
+}
+
 CodeGenIntrinsic &CodeGenIntrinsicMap::operator[](const Record *Record) {
   if (!Record->isSubClassOf("Intrinsic"))
     PrintFatalError("Intrinsic defs should be subclass of 'Intrinsic' class");
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
index 2df598da3f2507..1cdeaacd52dcdb 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
@@ -192,6 +192,7 @@ class CodeGenIntrinsicTable {
 
 private:
   void CheckDuplicateIntrinsics() const;
+  void CheckTargetIndependentIntrinsics() const;
 };
 
 // This class builds `CodeGenIntrinsic` on demand for a given Def.

[arsenm]

This conceptually isn't ideal, but probably isn't a practical concern

[jurahul]

This conceptually isn't ideal, but probably isn't a practical concern

WDYM? We could fall back to looking up in the target independent list if the lookup in the target specific one fails? And this case will actually work as expected? If so, yeah, I agree it's possible, but no one is asking for that.

[arsenm]

WDYM? We could fall back to looking up in the target independent list if the lookup in the target specific one fails? And this case will actually work as expected? If so, yeah, I agree it's possible, but no one is asking for that.

It's restricting the set of possible intrinsic names. It just happens that it's unlikely that a real operation name doesn't conflict with a target name