[LLVM] Change Intrinsic::getBaseName to return std::string

jurahul · jurahul · commit 10c733b2ac5c · 2024-10-09T11:23:18.000-07:00
Change `Intrinsic::getName()` and `Intrinsic::getBaseName()` to return std::string. This is in preparation for reducing the size of the static intrinsic string table as described in https://discourse.llvm.org/t/rfc-compress-intrinsic-name-table/82412
diff --git a/llvm/include/llvm-c/Core.h b/llvm/include/llvm-c/Core.h
@@ -2833,11 +2833,17 @@ LLVMTypeRef LLVMIntrinsicGetType(LLVMContextRef Ctx, unsigned ID,
  */
 const char *LLVMIntrinsicGetName(unsigned ID, size_t *NameLength);
 
+/**
+ * Retrieves the name of an intrinsic. The caller is responsible for freeing the
+ * returned string.
+ *
+ * @see llvm::Intrinsic::getName()
+ */
+char *LLVMIntrinsicCopyName(unsigned ID, size_t *NameLength);
+
 /** Deprecated: Use LLVMIntrinsicCopyOverloadedName2 instead. */
-const char *LLVMIntrinsicCopyOverloadedName(unsigned ID,
-                                            LLVMTypeRef *ParamTypes,
-                                            size_t ParamCount,
-                                            size_t *NameLength);
+char *LLVMIntrinsicCopyOverloadedName(unsigned ID, LLVMTypeRef *ParamTypes,
+                                      size_t ParamCount, size_t *NameLength);
 
 /**
  * Copies the name of an overloaded intrinsic identified by a given list of
@@ -2850,10 +2856,9 @@ const char *LLVMIntrinsicCopyOverloadedName(unsigned ID,
  *
  * @see llvm::Intrinsic::getName()
  */
-const char *LLVMIntrinsicCopyOverloadedName2(LLVMModuleRef Mod, unsigned ID,
-                                             LLVMTypeRef *ParamTypes,
-                                             size_t ParamCount,
-                                             size_t *NameLength);
+char *LLVMIntrinsicCopyOverloadedName2(LLVMModuleRef Mod, unsigned ID,
+                                       LLVMTypeRef *ParamTypes,
+                                       size_t ParamCount, size_t *NameLength);
 
 /**
  * Obtain if the intrinsic identified by the given ID is overloaded.
diff --git a/llvm/include/llvm/IR/Intrinsics.h b/llvm/include/llvm/IR/Intrinsics.h
@@ -52,11 +52,11 @@ namespace Intrinsic {
   /// Return the LLVM name for an intrinsic, such as "llvm.ppc.altivec.lvx".
   /// Note, this version is for intrinsics with no overloads.  Use the other
   /// version of getName if overloads are required.
-  StringRef getName(ID id);
+  std::string getName(ID id);
 
   /// Return the LLVM name for an intrinsic, without encoded types for
   /// overloading, such as "llvm.ssa.copy".
-  StringRef getBaseName(ID id);
+  std::string getBaseName(ID id);
 
   /// Return the LLVM name for an intrinsic, such as "llvm.ppc.altivec.lvx" or
   /// "llvm.ssa.copy.p0s_s.1". Note, this version of getName supports overloads.
diff --git a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp
@@ -147,7 +147,7 @@ void IRInstructionData::setCalleeName(bool MatchByName) {
           Intrinsic::getName(IntrinsicID, FT->params(), II->getModule(), FT);
     // If there is not an overloaded name, we only need to use this version.
     else
-      CalleeName = Intrinsic::getName(IntrinsicID).str();
+      CalleeName = Intrinsic::getName(IntrinsicID);
 
     return;
   }
diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp
@@ -130,7 +130,7 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI,
   std::string ScalarName =
       Intrinsic::isOverloaded(IID)
           ? Intrinsic::getName(IID, ScalarArgTypes, II->getModule())
-          : Intrinsic::getName(IID).str();
+          : Intrinsic::getName(IID);
 
   // Try to find the mapping for the scalar version of this intrinsic and the
   // exact vector width of the call operands in the TargetLibraryInfo. First,
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
@@ -161,7 +161,7 @@ std::string SDNode::getOperationName(const SelectionDAG *G) const {
     unsigned OpNo = getOpcode() == ISD::INTRINSIC_WO_CHAIN ? 0 : 1;
     unsigned IID = getOperand(OpNo)->getAsZExtVal();
     if (IID < Intrinsic::num_intrinsics)
-      return Intrinsic::getBaseName((Intrinsic::ID)IID).str();
+      return Intrinsic::getBaseName((Intrinsic::ID)IID);
     if (!G)
       return "Unknown intrinsic";
     if (const TargetIntrinsicInfo *TII = G->getTarget().getIntrinsicInfo())
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
@@ -39,12 +39,14 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
 #include <cstring>
 #include <system_error>
+#include <unordered_map>
 
 using namespace llvm;
 
@@ -2472,10 +2474,24 @@ LLVMValueRef LLVMGetIntrinsicDeclaration(LLVMModuleRef Mod,
 }
 
 const char *LLVMIntrinsicGetName(unsigned ID, size_t *NameLength) {
+  static std::unordered_map<Intrinsic::ID, std::string> IntrinsicNamePool;
+  static sys::Mutex Mutex;
   auto IID = llvm_map_to_intrinsic_id(ID);
-  auto Str = llvm::Intrinsic::getName(IID);
+
+  std::lock_guard<sys::Mutex> Guard(Mutex);
+  auto [Iter, Inserted] = IntrinsicNamePool.try_emplace(IID);
+  if (Inserted)
+    Iter->second = llvm::Intrinsic::getName(IID);
+  const std::string &Name = Iter->second;
+  *NameLength = Name.size();
+  return Name.data();
+}
+
+char *LLVMIntrinsicCopyName(unsigned ID, size_t *NameLength) {
+  auto IID = llvm_map_to_intrinsic_id(ID);
+  std::string Str = llvm::Intrinsic::getName(IID);
   *NameLength = Str.size();
-  return Str.data();
+  return strdup(Str.c_str());
 }
 
 LLVMTypeRef LLVMIntrinsicGetType(LLVMContextRef Ctx, unsigned ID,
@@ -2485,21 +2501,18 @@ LLVMTypeRef LLVMIntrinsicGetType(LLVMContextRef Ctx, unsigned ID,
   return wrap(llvm::Intrinsic::getType(*unwrap(Ctx), IID, Tys));
 }
 
-const char *LLVMIntrinsicCopyOverloadedName(unsigned ID,
-                                            LLVMTypeRef *ParamTypes,
-                                            size_t ParamCount,
-                                            size_t *NameLength) {
+char *LLVMIntrinsicCopyOverloadedName(unsigned ID, LLVMTypeRef *ParamTypes,
+                                      size_t ParamCount, size_t *NameLength) {
   auto IID = llvm_map_to_intrinsic_id(ID);
   ArrayRef<Type*> Tys(unwrap(ParamTypes), ParamCount);
   auto Str = llvm::Intrinsic::getNameNoUnnamedTypes(IID, Tys);
   *NameLength = Str.length();
   return strdup(Str.c_str());
 }
 
-const char *LLVMIntrinsicCopyOverloadedName2(LLVMModuleRef Mod, unsigned ID,
-                                             LLVMTypeRef *ParamTypes,
-                                             size_t ParamCount,
-                                             size_t *NameLength) {
+char *LLVMIntrinsicCopyOverloadedName2(LLVMModuleRef Mod, unsigned ID,
+                                       LLVMTypeRef *ParamTypes,
+                                       size_t ParamCount, size_t *NameLength) {
   auto IID = llvm_map_to_intrinsic_id(ID);
   ArrayRef<Type *> Tys(unwrap(ParamTypes), ParamCount);
   auto Str = llvm::Intrinsic::getName(IID, Tys, unwrap(Mod));
diff --git a/llvm/lib/IR/Intrinsics.cpp b/llvm/lib/IR/Intrinsics.cpp
@@ -44,12 +44,12 @@ static constexpr const char *const IntrinsicNameTable[] = {
 #undef GET_INTRINSIC_NAME_TABLE
 };
 
-StringRef Intrinsic::getBaseName(ID id) {
+std::string Intrinsic::getBaseName(ID id) {
   assert(id < num_intrinsics && "Invalid intrinsic ID!");
   return IntrinsicNameTable[id];
 }
 
-StringRef Intrinsic::getName(ID id) {
+std::string Intrinsic::getName(ID id) {
   assert(id < num_intrinsics && "Invalid intrinsic ID!");
   assert(!Intrinsic::isOverloaded(id) &&
          "This version of getName does not support overloading");
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
@@ -171,8 +171,8 @@ class PreloadKernelArgInfo {
   // Try to allocate SGPRs to preload implicit kernel arguments.
   void tryAllocImplicitArgPreloadSGPRs(uint64_t ImplicitArgsBaseOffset,
                                        IRBuilder<> &Builder) {
-    StringRef Name = Intrinsic::getName(Intrinsic::amdgcn_implicitarg_ptr);
-    Function *ImplicitArgPtr = F.getParent()->getFunction(Name);
+    Function *ImplicitArgPtr = F.getParent()->getFunction(
+        Intrinsic::getName(Intrinsic::amdgcn_implicitarg_ptr));
     if (!ImplicitArgPtr)
       return;
 
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
@@ -78,8 +78,7 @@ class AMDGPULowerKernelAttributes : public ModulePass {
 Function *getBasePtrIntrinsic(Module &M, bool IsV5OrAbove) {
   auto IntrinsicId = IsV5OrAbove ? Intrinsic::amdgcn_implicitarg_ptr
                                  : Intrinsic::amdgcn_dispatch_ptr;
-  StringRef Name = Intrinsic::getName(IntrinsicId);
-  return M.getFunction(Name);
+  return M.getFunction(Intrinsic::getName(IntrinsicId));
 }
 
 } // end anonymous namespace
diff --git a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
@@ -2278,8 +2278,8 @@ class LowerMatrixIntrinsics {
           return;
         }
         auto *II = cast<IntrinsicInst>(CI);
-        write(Intrinsic::getBaseName(II->getIntrinsicID())
-                  .drop_front(StringRef("llvm.matrix.").size()));
+        std::string IntName = Intrinsic::getBaseName(II->getIntrinsicID());
+        write(StringRef(IntName).drop_front(StringRef("llvm.matrix.").size()));
         write(".");
         std::string Tmp;
         raw_string_ostream SS(Tmp);
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -4389,18 +4389,17 @@ void LoopVectorizationPlanner::emitInvalidCostRemarks(
         OS << (Pair.second == Subset.front().second ? "" : ", ") << Pair.second;
       OS << "):";
       if (Opcode == Instruction::Call) {
-        StringRef Name = "";
+        OS << " call to ";
         if (auto *Int = dyn_cast<VPWidenIntrinsicRecipe>(R)) {
-          Name = Int->getIntrinsicName();
+          OS << Int->getIntrinsicName();
         } else {
           auto *WidenCall = dyn_cast<VPWidenCallRecipe>(R);
           Function *CalledFn =
               WidenCall ? WidenCall->getCalledScalarFunction()
                         : cast<Function>(R->getOperand(R->getNumOperands() - 1)
                                              ->getLiveInIRValue());
-          Name = CalledFn->getName();
+          OS << CalledFn->getName();
         }
-        OS << " call to " << Name;
       } else
         OS << " " << Instruction::getOpcodeName(Opcode);
       reportVectorizationInfo(OutString, "InvalidCost", ORE, OrigLoop, nullptr,
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1670,7 +1670,7 @@ class VPWidenIntrinsicRecipe : public VPRecipeWithIRFlags {
   Type *getResultType() const { return ResultTy; }
 
   /// Return to name of the intrinsic as string.
-  StringRef getIntrinsicName() const;
+  std::string getIntrinsicName() const;
 
   /// Returns true if the intrinsic may read from memory.
   bool mayReadFromMemory() const { return MayReadFromMemory; }
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1038,7 +1038,7 @@ InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF,
   return Ctx.TTI.getIntrinsicInstrCost(CostAttrs, CostKind);
 }
 
-StringRef VPWidenIntrinsicRecipe::getIntrinsicName() const {
+std::string VPWidenIntrinsicRecipe::getIntrinsicName() const {
   return Intrinsic::getBaseName(VectorIntrinsicID);
 }
 
diff --git a/llvm/unittests/IR/IntrinsicsTest.cpp b/llvm/unittests/IR/IntrinsicsTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Intrinsics.h"
+#include "llvm-c/Core.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/IRBuilder.h"
@@ -25,6 +26,7 @@
 #include "llvm/IR/IntrinsicsS390.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/Parallel.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -127,6 +129,87 @@ TEST(IntrinsicNameLookup, MSBuiltinLookup) {
     EXPECT_EQ(ID, getIntrinsicForMSBuiltin(Target, Builtin));
 }
 
+// Test C API to get/copy LLVM intrinsic name.
+TEST(IntrinsicNameLookup, LLVMIntrinsicGetCopyNameSimple) {
+  static constexpr struct {
+    Intrinsic::ID ID;
+    const char *Name;
+  } Tests[] = {{Intrinsic::not_intrinsic, "not_intrinsic"},
+               {Intrinsic::assume, "llvm.assume"},
+               {Intrinsic::coro_free, "llvm.coro.free"},
+               {Intrinsic::aarch64_break, "llvm.aarch64.break"},
+               {Intrinsic::x86_int, "llvm.x86.int"}};
+
+  for (auto [ID, ExpectedName] : Tests) {
+    size_t NameSize = 0;
+    const char *CName = LLVMIntrinsicGetName(ID, &NameSize);
+    StringRef Name(CName, NameSize);
+
+    // Verify we get correct name.
+    EXPECT_EQ(Name, ExpectedName);
+    const char *CName1 = LLVMIntrinsicGetName(ID, &NameSize);
+
+    // Verify we get the same pointer and length the second time.
+    EXPECT_EQ(CName, CName1);
+    EXPECT_EQ(NameSize, Name.size());
+  }
+
+  for (auto [ID, ExpectedName] : Tests) {
+    size_t NameSize = 0;
+    // Now test the copy API.
+    char *CName = LLVMIntrinsicCopyName(ID, &NameSize);
+    StringRef Name(CName, NameSize);
+
+    // Verify we get correct name.
+    EXPECT_EQ(Name, ExpectedName);
+
+    // Verify we get the different pointer and same length the second time.
+    char *CName1 = LLVMIntrinsicCopyName(ID, &NameSize);
+    EXPECT_NE(CName, CName1);
+    EXPECT_EQ(NameSize, Name.size());
+
+    free(CName);
+    free(CName1);
+  }
+}
+
+// Test correctness of `LLVMIntrinsicGetName` when exercised in a multi-threaded
+// manner.
+TEST(IntrinsicNameLookup, LLVMIntrinsicGetNameMultiThreaded) {
+  constexpr unsigned NUM_TASKS = 16;
+  constexpr unsigned STEP = 40;
+  std::map<unsigned, StringRef> LookupResults[NUM_TASKS];
+
+  parallelFor(0, NUM_TASKS, [&](size_t Idx) {
+    for (unsigned ID = 0; ID < Intrinsic::num_intrinsics; ID += STEP) {
+      if (LLVMIntrinsicIsOverloaded(ID))
+        continue;
+      size_t NameSize;
+      const char *Name = LLVMIntrinsicGetName(ID, &NameSize);
+      LookupResults[Idx].insert({ID, StringRef(Name, NameSize)});
+    }
+  });
+
+  // Make sure we have atleast a few intrinsics to test.
+  for (const auto &Map : LookupResults)
+    EXPECT_GE(Map.size(), 10U);
+
+  // Validate data.
+  for (unsigned ID = 0; ID < Intrinsic::num_intrinsics; ID += STEP) {
+    if (LLVMIntrinsicIsOverloaded(ID))
+      continue;
+    size_t NameSize;
+    const char *CName = LLVMIntrinsicGetName(ID, &NameSize);
+    StringRef Name(CName, NameSize);
+    for (const auto &Map : LookupResults) {
+      auto Iter = Map.find(ID);
+      ASSERT_NE(Iter, Map.end());
+      EXPECT_EQ(Iter->second.size(), Name.size());
+      EXPECT_EQ(Iter->second.data(), Name.data());
+    }
+  }
+}
+
 TEST_F(IntrinsicsTest, InstrProfInheritance) {
   auto isInstrProfInstBase = [](const Instruction &I) {
     return isa<InstrProfInstBase>(I);

Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ void IRInstructionData::setCalleeName(bool MatchByName) {`
`147`	`147`	`Intrinsic::getName(IntrinsicID, FT->params(), II->getModule(), FT);`
`148`	`148`	`// If there is not an overloaded name, we only need to use this version.`
`149`	`149`	`else`
`150`		`- CalleeName = Intrinsic::getName(IntrinsicID).str();`
	`150`	`+ CalleeName = Intrinsic::getName(IntrinsicID);`
`151`	`151`
`152`	`152`	`return;`
`153`	`153`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,8 +78,7 @@ class AMDGPULowerKernelAttributes : public ModulePass {`
`78`	`78`	`Function *getBasePtrIntrinsic(Module &M, bool IsV5OrAbove) {`
`79`	`79`	`auto IntrinsicId = IsV5OrAbove ? Intrinsic::amdgcn_implicitarg_ptr`
`80`	`80`	`: Intrinsic::amdgcn_dispatch_ptr;`
`81`		`- StringRef Name = Intrinsic::getName(IntrinsicId);`
`82`		`- return M.getFunction(Name);`
	`81`	`+ return M.getFunction(Intrinsic::getName(IntrinsicId));`
`83`	`82`	`}`
`84`	`83`
`85`	`84`	`} // end anonymous namespace`