[llvm] b9d6231 - generate a name of an unnamed global variable for Instruction Selection (#78293)

[via llvm-commits]

Author: Vyacheslav Levytskyy
Date: 2024-01-30T18:09:52+01:00
New Revision: b9d623105d51f5fc2d120c862b4e2bbff310f728

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

LOG: generate a name of an unnamed global variable for Instruction Selection (#78293)

The goal of this PR is to fix the issue of global unnamed variables
causing SPIR-V Backend code generation to crash:
https://github.com/llvm/llvm-project/issues/78278

The reason for the crash is that GlobalValue's getGlobalIdentifier()
would fail for unnamed global variable when trying to access the first
character of the name (see lib/IR/Globals.cpp:150). This leads to assert
in Debug and undefined behaviour in Release builds.

The proposed fix generates a name of an unnamed global variable as
__unnamed_<unsigned number>, in a style of similar existing LLVM
implementation (see lib/IR/Mangler.cpp:131). A new class member variable
is added into `SPIRVInstructionSelector` class to keep track of the
number we give to anonymous global values to generate the same name
every time when this is needed.

The patch adds a new LIT test with the smallest implementation of
reproducer ll code.

Added: 
    llvm/test/CodeGen/SPIRV/unnamed-global.ll

Modified: 
    llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 5ad47de4fc541..8c1dfc5e626db 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -53,6 +53,10 @@ class SPIRVInstructionSelector : public InstructionSelector {
   SPIRVGlobalRegistry &GR;
   MachineRegisterInfo *MRI;
 
+  /// We need to keep track of the number we give to anonymous global values to
+  /// generate the same name every time when this is needed.
+  mutable DenseMap<const GlobalValue *, unsigned> UnnamedGlobalIDs;
+
 public:
   SPIRVInstructionSelector(const SPIRVTargetMachine &TM,
                            const SPIRVSubtarget &ST,
@@ -1519,7 +1523,17 @@ bool SPIRVInstructionSelector::selectGlobalValue(
   SPIRVType *ResType = GR.getOrCreateSPIRVPointerType(
       PointerBaseType, I, TII,
       addressSpaceToStorageClass(GV->getAddressSpace()));
-  std::string GlobalIdent = GV->getGlobalIdentifier();
+
+  std::string GlobalIdent;
+  if (!GV->hasName()) {
+    unsigned &ID = UnnamedGlobalIDs[GV];
+    if (ID == 0)
+      ID = UnnamedGlobalIDs.size();
+    GlobalIdent = "__unnamed_" + Twine(ID).str();
+  } else {
+    GlobalIdent = GV->getGlobalIdentifier();
+  }
+
   // We have functions as operands in tests with blocks of instruction e.g. in
   // transcoding/global_block.ll. These operands are not used and should be
   // substituted by zero constants. Their type is expected to be always

diff  --git a/llvm/test/CodeGen/SPIRV/unnamed-global.ll b/llvm/test/CodeGen/SPIRV/unnamed-global.ll
new file mode 100644
index 0000000000000..d2cd4ea8cafc3
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/unnamed-global.ll
@@ -0,0 +1,16 @@
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK: %[[TyInt:.*]] = OpTypeInt 8 0
+; CHECK: %[[ConstInt:.*]] = OpConstant %[[TyInt]] 123
+; CHECK: %[[TyPtr:.*]] = OpTypePointer {{[a-zA-Z]+}} %[[TyInt]]
+; CHECK: %[[VarId:.*]] = OpVariable %[[TyPtr]] {{[a-zA-Z]+}} %[[ConstInt]]
+
+ at 0 = addrspace(1) global i8 123
+
+; Function Attrs: nounwind
+define spir_kernel void @foo() #0 {
+  ret void
+}
+
+attributes #0 = { nounwind }