Skip to content

Commit a774e7f

Browse files
[SPIR-V] Fix OpName and LinkageAttributes decoration of global variables (#120492)
This PR changes `getGlobalIdentifier()` into `getName()` value when creating a name of a global variable, and fixes generation of LinkageAttributes decoration of global variables by taking into account Private Linkage in addition to Internal. Previous implementation led to an issue with back translation of SPIR-V to LLVM IR, e.g.: ``` @__const.G1 = private unnamed_addr addrspace(1) constant %my_type undef ... Fails to verify module: 'common' global may not be marked constant! ptr addrspace(1) @"llvm-link;__const.G1" ``` A reproducer is included as a new test case.
1 parent 66acb26 commit a774e7f

File tree

4 files changed

+74
-12
lines changed

4 files changed

+74
-12
lines changed

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,20 +1841,20 @@ void SPIRVEmitIntrinsics::processGlobalValue(GlobalVariable &GV,
18411841
// Skip special artifical variable llvm.global.annotations.
18421842
if (GV.getName() == "llvm.global.annotations")
18431843
return;
1844-
if (GV.hasInitializer() && !isa<UndefValue>(GV.getInitializer())) {
1844+
Constant *Init = nullptr;
1845+
if (hasInitializer(&GV)) {
18451846
// Deduce element type and store results in Global Registry.
18461847
// Result is ignored, because TypedPointerType is not supported
18471848
// by llvm IR general logic.
18481849
deduceElementTypeHelper(&GV, false);
1849-
Constant *Init = GV.getInitializer();
1850+
Init = GV.getInitializer();
18501851
Type *Ty = isAggrConstForceInt32(Init) ? B.getInt32Ty() : Init->getType();
18511852
Constant *Const = isAggrConstForceInt32(Init) ? B.getInt32(1) : Init;
18521853
auto *InitInst = B.CreateIntrinsic(Intrinsic::spv_init_global,
18531854
{GV.getType(), Ty}, {&GV, Const});
18541855
InitInst->setArgOperand(1, Init);
18551856
}
1856-
if ((!GV.hasInitializer() || isa<UndefValue>(GV.getInitializer())) &&
1857-
GV.getNumUses() == 0)
1857+
if (!Init && GV.getNumUses() == 0)
18581858
B.CreateIntrinsic(Intrinsic::spv_unref_global, GV.getType(), &GV);
18591859
}
18601860

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3478,7 +3478,7 @@ bool SPIRVInstructionSelector::selectGlobalValue(
34783478
ID = UnnamedGlobalIDs.size();
34793479
GlobalIdent = "__unnamed_" + Twine(ID).str();
34803480
} else {
3481-
GlobalIdent = GV->getGlobalIdentifier();
3481+
GlobalIdent = GV->getName();
34823482
}
34833483

34843484
// Behaviour of functions as operands depends on availability of the
@@ -3541,18 +3541,16 @@ bool SPIRVInstructionSelector::selectGlobalValue(
35413541
auto GlobalVar = cast<GlobalVariable>(GV);
35423542
assert(GlobalVar->getName() != "llvm.global.annotations");
35433543

3544-
bool HasInit = GlobalVar->hasInitializer() &&
3545-
!isa<UndefValue>(GlobalVar->getInitializer());
3546-
// Skip empty declaration for GVs with initilaizers till we get the decl with
3544+
// Skip empty declaration for GVs with initializers till we get the decl with
35473545
// passed initializer.
3548-
if (HasInit && !Init)
3546+
if (hasInitializer(GlobalVar) && !Init)
35493547
return true;
35503548

3551-
bool HasLnkTy = GV->getLinkage() != GlobalValue::InternalLinkage;
3549+
bool HasLnkTy = !GV->hasInternalLinkage() && !GV->hasPrivateLinkage();
35523550
SPIRV::LinkageType::LinkageType LnkType =
3553-
(GV->isDeclaration() || GV->hasAvailableExternallyLinkage())
3551+
GV->isDeclarationForLinker()
35543552
? SPIRV::LinkageType::Import
3555-
: (GV->getLinkage() == GlobalValue::LinkOnceODRLinkage &&
3553+
: (GV->hasLinkOnceODRLinkage() &&
35563554
STI.canUseExtension(SPIRV::Extension::SPV_KHR_linkonce_odr)
35573555
? SPIRV::LinkageType::LinkOnceODR
35583556
: SPIRV::LinkageType::Export);

llvm/lib/Target/SPIRV/SPIRVUtils.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "llvm/Analysis/LoopInfo.h"
1818
#include "llvm/CodeGen/MachineBasicBlock.h"
1919
#include "llvm/IR/Dominators.h"
20+
#include "llvm/IR/GlobalVariable.h"
2021
#include "llvm/IR/IRBuilder.h"
2122
#include "llvm/IR/TypedPointerType.h"
2223
#include <queue>
@@ -236,6 +237,10 @@ Type *parseBasicTypeName(StringRef &TypeName, LLVMContext &Ctx);
236237
// Returns true if the function was changed.
237238
bool sortBlocks(Function &F);
238239

240+
inline bool hasInitializer(const GlobalVariable *GV) {
241+
return GV->hasInitializer() && !isa<UndefValue>(GV->getInitializer());
242+
}
243+
239244
// True if this is an instance of TypedPointerType.
240245
inline bool isTypedPointerTy(const Type *T) {
241246
return T && T->getTypeID() == Type::TypedPointerTyID;
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
; Check names and decoration of global variables.
2+
3+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
4+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
5+
6+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
7+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
8+
9+
; CHECK-DAG: OpName %[[#id18:]] "G1"
10+
; CHECK-DAG: OpName %[[#id22:]] "g1"
11+
; CHECK-DAG: OpName %[[#id23:]] "g2"
12+
; CHECK-DAG: OpName %[[#id27:]] "g4"
13+
; CHECK-DAG: OpName %[[#id30:]] "c1"
14+
; CHECK-DAG: OpName %[[#id31:]] "n_t"
15+
; CHECK-DAG: OpName %[[#id32:]] "w"
16+
; CHECK-DAG: OpName %[[#id34:]] "a.b"
17+
; CHECK-DAG: OpName %[[#id35:]] "e"
18+
; CHECK-DAG: OpName %[[#id36:]] "y.z"
19+
; CHECK-DAG: OpName %[[#id38:]] "x"
20+
21+
; CHECK-NOT: OpDecorate %[[#id18]] LinkageAttributes
22+
; CHECK-DAG: OpDecorate %[[#id18]] Constant
23+
; CHECK-DAG: OpDecorate %[[#id22]] Alignment 4
24+
; CHECK-DAG: OpDecorate %[[#id22]] LinkageAttributes "g1" Export
25+
; CHECK-DAG: OpDecorate %[[#id23]] Alignment 4
26+
; CHECK-DAG: OpDecorate %[[#id27]] Alignment 4
27+
; CHECK-DAG: OpDecorate %[[#id27]] LinkageAttributes "g4" Export
28+
; CHECK-DAG: OpDecorate %[[#id30]] Constant
29+
; CHECK-DAG: OpDecorate %[[#id30]] Alignment 4
30+
; CHECK-DAG: OpDecorate %[[#id30]] LinkageAttributes "c1" Export
31+
; CHECK-DAG: OpDecorate %[[#id31]] Constant
32+
; CHECK-DAG: OpDecorate %[[#id31]] LinkageAttributes "n_t" Import
33+
; CHECK-DAG: OpDecorate %[[#id32]] Constant
34+
; CHECK-DAG: OpDecorate %[[#id32]] Alignment 4
35+
; CHECK-DAG: OpDecorate %[[#id32]] LinkageAttributes "w" Export
36+
; CHECK-DAG: OpDecorate %[[#id34]] Constant
37+
; CHECK-DAG: OpDecorate %[[#id34]] Alignment 4
38+
; CHECK-DAG: OpDecorate %[[#id35]] LinkageAttributes "e" Import
39+
; CHECK-DAG: OpDecorate %[[#id36]] Alignment 4
40+
; CHECK-DAG: OpDecorate %[[#id38]] Constant
41+
; CHECK-DAG: OpDecorate %[[#id38]] Alignment 4
42+
43+
%"class.sycl::_V1::nd_item" = type { i8 }
44+
45+
@G1 = private unnamed_addr addrspace(1) constant %"class.sycl::_V1::nd_item" poison, align 1
46+
@g1 = addrspace(1) global i32 1, align 4
47+
@g2 = internal addrspace(1) global i32 2, align 4
48+
@g4 = common addrspace(1) global i32 0, align 4
49+
@c1 = addrspace(2) constant [2 x i32] [i32 0, i32 1], align 4
50+
@n_t = external addrspace(2) constant [256 x i32]
51+
@w = addrspace(1) constant i32 0, align 4
52+
@a.b = internal addrspace(2) constant [2 x i32] [i32 2, i32 3], align 4
53+
@e = external addrspace(1) global i32
54+
@y.z = internal addrspace(1) global i32 0, align 4
55+
@x = internal addrspace(2) constant float 1.000000e+00, align 4
56+
57+
define internal spir_func void @foo(ptr addrspace(4) align 1 %arg) {
58+
ret void
59+
}

0 commit comments

Comments
 (0)