Skip to content

[ElimAvailExtern] Add an option to allow to convert global variables in a specified address space to local #144287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −3
Open

[ElimAvailExtern] Add an option to allow to convert global variables in a specified address space to local #144287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion llvm/lib/Transforms/IPO/ElimAvailExtern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ static cl::opt<bool> ConvertToLocal(
cl::desc("Convert available_externally into locals, renaming them "
"to avoid link-time clashes."));

static cl::opt<unsigned> ConvertGlobalVariableInAddrSpace(
"avail-extern-gv-in-addrspace-to-local", cl::Hidden,
cl::desc(
"Convert available_externally global variables into locals if they are "
"in specificed addrspace, renaming them to avoid link-time clashes."));

STATISTIC(NumRemovals, "Number of functions removed");
STATISTIC(NumConversions, "Number of functions converted");
STATISTIC(NumConversions, "Number of functions and globals converted");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it help if we split this statistic in NumFunctionsConverted and NumGlobalVariablesConverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do

STATISTIC(NumVariables, "Number of global variables removed");

void deleteFunction(Function &F) {
Expand Down Expand Up @@ -88,9 +94,32 @@ static void convertToLocalCopy(Module &M, Function &F) {
++NumConversions;
}

static void convertToLocalCopy(Module &M, GlobalValue &GV) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe factor out the calculation and setting of the new name and linkage and reuse it at line 77

assert(GV.hasAvailableExternallyLinkage());
std::string OrigName = GV.getName().str();
std::string NewName = OrigName + ".__uniq" + getUniqueModuleId(&M);
GV.setName(NewName);
GV.setLinkage(GlobalValue::InternalLinkage);
++NumConversions;
}

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

// Convert global variables in specified address space before changing it to
// external linkage below.
if (ConvertGlobalVariableInAddrSpace.getNumOccurrences()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this rather be done as part of the existing loop - for readability (the policies applied to GlobalVariables are in one place)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, although that means we will have to have the if-statement in each iteration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't see why that would be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thought it might be less efficient.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it - likely some loop optimization would handle that; if it appears on some compile-time benchmark, we can revisit.

for (GlobalVariable &GV : M.globals()) {
if (!GV.hasAvailableExternallyLinkage() || GV.use_empty())
continue;

if (GV.getAddressSpace() == ConvertGlobalVariableInAddrSpace) {
convertToLocalCopy(M, GV);
Changed = true;
}
}
}

// Drop initializers of available externally global variables.
for (GlobalVariable &GV : M.globals()) {
if (!GV.hasAvailableExternallyLinkage())
Expand Down
21 changes: 21 additions & 0 deletions llvm/test/Transforms/EliminateAvailableExternally/convert-global-variables-to-local.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 5
; RUN: opt -S -passes=elim-avail-extern -avail-extern-gv-in-addrspace-to-local=3 %s -o - | FileCheck %s

@shared = internal addrspace(3) global i32 undef, align 4
@shared.imported = available_externally hidden unnamed_addr addrspace(3) global i32 undef, align 4

;.
; CHECK: @shared = internal addrspace(3) global i32 undef, align 4
; CHECK: @shared.imported.__uniq.[[UUID:.*]] = internal unnamed_addr addrspace(3) global i32 undef, align 4
;.
define void @foo(i32 %v) {
; CHECK-LABEL: define void @foo(
; CHECK-SAME: i32 [[V:%.*]]) {
; CHECK-NEXT: store i32 [[V]], ptr addrspace(3) @shared, align 4
; CHECK-NEXT: store i32 [[V]], ptr addrspace(3) @shared.imported.__uniq.[[UUID]], align 4
; CHECK-NEXT: ret void
;
store i32 %v, ptr addrspace(3) @shared, align 4
store i32 %v, ptr addrspace(3) @shared.imported, align 4
ret void
}
4 changes: 2 additions & 2 deletions llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ define void @hello(ptr %g) !guid !0 {
; CHECK-NEXT: load ptr, ptr @f
; CHECK-NEXT: call void @call_out(ptr %f)
; CHECK: Statistics Collected
; CHECK: 1 elim-avail-extern - Number of functions converted
; CHECK: 1 elim-avail-extern - Number of functions and globals converted
; CHECK: 1 elim-avail-extern - Number of functions removed

; NOOP: 2 elim-avail-extern - Number of functions removed
; NOOP: 2 elim-avail-extern - Number of functions removed
Loading