Skip to content

Commit 15482c8

Browse files
authored
[ElimAvailExtern] Add an option to allow to convert global variables in a specified address space to local (#144287)
Currently, the `EliminateAvailableExternallyPass` only converts certain available externally functions to local if `avail-extern-to-local` is set or in contextual profiling mode. For global variables, it only drops their initializers. This PR adds an option to allow the pass to convert global variables in a specified address space to local. The motivation for this change is to correctly support lowering of LDS variables (`__shared__` variables, in more generic terminology) when ThinLTO is enabled for AMDGPU. A `__shared__` variable is lowered to a hidden global variable in a particular address space by the frontend, which is roughly same as a `static` local variable. To properly lower it in the backend, the compiler needs to check all its uses. Enabling ThinLTO currently breaks this when a function containing a `__shared__` variable is imported from another module. Even though the global variable is imported along with its associated function, and the function is privatized by the `EliminateAvailableExternallyPass`, the global variable itself is not. It's safe to privatize such global variables, because they're _local_ to their associated functions. If the function itself is privatized, its associated global variables should also be privatized accordingly.
1 parent c21a4c6 commit 15482c8

File tree

2 files changed

+53
-4
lines changed

2 files changed

+53
-4
lines changed

llvm/lib/Transforms/IPO/ElimAvailExtern.cpp

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,15 @@ static cl::opt<bool> ConvertToLocal(
3535
cl::desc("Convert available_externally into locals, renaming them "
3636
"to avoid link-time clashes."));
3737

38+
static cl::opt<unsigned> ConvertGlobalVariableInAddrSpace(
39+
"avail-extern-gv-in-addrspace-to-local", cl::Hidden,
40+
cl::desc(
41+
"Convert available_externally global variables into locals if they are "
42+
"in specificed addrspace, renaming them to avoid link-time clashes."));
43+
3844
STATISTIC(NumRemovals, "Number of functions removed");
39-
STATISTIC(NumConversions, "Number of functions converted");
45+
STATISTIC(NumFunctionsConverted, "Number of functions converted");
46+
STATISTIC(NumGlobalVariablesConverted, "Number of global variables converted");
4047
STATISTIC(NumVariables, "Number of global variables removed");
4148

4249
void deleteFunction(Function &F) {
@@ -45,6 +52,10 @@ void deleteFunction(Function &F) {
4552
++NumRemovals;
4653
}
4754

55+
static std::string getNewName(Module &M, const GlobalValue &GV) {
56+
return GV.getName().str() + ".__uniq" + getUniqueModuleId(&M);
57+
}
58+
4859
/// Create a copy of the thinlto import, mark it local, and redirect direct
4960
/// calls to the copy. Only direct calls are replaced, so that e.g. indirect
5061
/// call function pointer tests would use the global identity of the function.
@@ -68,7 +79,7 @@ static void convertToLocalCopy(Module &M, Function &F) {
6879
// functions with the same name, but that just creates more trouble than
6980
// necessary e.g. distinguishing profiles or debugging. Instead, we append the
7081
// module identifier.
71-
auto NewName = OrigName + ".__uniq" + getUniqueModuleId(&M);
82+
std::string NewName = getNewName(M, F);
7283
F.setName(NewName);
7384
if (auto *SP = F.getSubprogram())
7485
SP->replaceLinkageName(MDString::get(F.getParent()->getContext(), NewName));
@@ -85,16 +96,33 @@ static void convertToLocalCopy(Module &M, Function &F) {
8596
F.getAddressSpace(), OrigName, F.getParent());
8697
F.replaceUsesWithIf(Decl,
8798
[&](Use &U) { return !isa<CallBase>(U.getUser()); });
88-
++NumConversions;
99+
++NumFunctionsConverted;
100+
}
101+
102+
/// Similar to the function above, this is to convert an externally available
103+
/// global variable to local.
104+
static void convertToLocalCopy(Module &M, GlobalVariable &GV) {
105+
assert(GV.hasAvailableExternallyLinkage());
106+
GV.setName(getNewName(M, GV));
107+
GV.setLinkage(GlobalValue::InternalLinkage);
108+
++NumGlobalVariablesConverted;
89109
}
90110

91111
static bool eliminateAvailableExternally(Module &M, bool Convert) {
92112
bool Changed = false;
93113

94-
// Drop initializers of available externally global variables.
114+
// If a global variable is available externally and in the specified address
115+
// space, convert it to local linkage; otherwise, drop its initializer.
95116
for (GlobalVariable &GV : M.globals()) {
96117
if (!GV.hasAvailableExternallyLinkage())
97118
continue;
119+
if (ConvertGlobalVariableInAddrSpace.getNumOccurrences() &&
120+
GV.getAddressSpace() == ConvertGlobalVariableInAddrSpace &&
121+
!GV.use_empty()) {
122+
convertToLocalCopy(M, GV);
123+
Changed = true;
124+
continue;
125+
}
98126
if (GV.hasInitializer()) {
99127
Constant *Init = GV.getInitializer();
100128
GV.setInitializer(nullptr);
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 5
2+
; RUN: opt -S -passes=elim-avail-extern -avail-extern-gv-in-addrspace-to-local=3 %s -o - | FileCheck %s
3+
4+
@shared = internal addrspace(3) global i32 undef, align 4
5+
@shared.imported = available_externally hidden unnamed_addr addrspace(3) global i32 undef, align 4
6+
7+
;.
8+
; CHECK: @shared = internal addrspace(3) global i32 undef, align 4
9+
; CHECK: @shared.imported.__uniq.[[UUID:.*]] = internal unnamed_addr addrspace(3) global i32 undef, align 4
10+
;.
11+
define void @foo(i32 %v) {
12+
; CHECK-LABEL: define void @foo(
13+
; CHECK-SAME: i32 [[V:%.*]]) {
14+
; CHECK-NEXT: store i32 [[V]], ptr addrspace(3) @shared, align 4
15+
; CHECK-NEXT: store i32 [[V]], ptr addrspace(3) @shared.imported.__uniq.[[UUID]], align 4
16+
; CHECK-NEXT: ret void
17+
;
18+
store i32 %v, ptr addrspace(3) @shared, align 4
19+
store i32 %v, ptr addrspace(3) @shared.imported, align 4
20+
ret void
21+
}

0 commit comments

Comments
 (0)