Skip to content

Commit

Permalink
[GlobalMerge] Allow merging globals with explicit section markings.
Browse files Browse the repository at this point in the history
At least on ELF, it's impossible to tell from the object file whether
two globals with the same section marking were merged: the merged global
uses "private" linkage to hide its symbol, and the aliases look like
regular symbols. I can't think of any other reason to disallow it.
(Of course, we can only merge globals in the same section.)

The weird alignment handling matches AsmPrinter; our alignment handling
for global variables should probably be refactored.

Differential Revision: https://reviews.llvm.org/D49822

llvm-svn: 338791
  • Loading branch information
Eli Friedman committed Aug 2, 2018
1 parent 366a49d commit 1ba5e9a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 15 deletions.
32 changes: 22 additions & 10 deletions llvm/lib/CodeGen/GlobalMerge.cpp
Expand Up @@ -461,7 +461,18 @@ bool GlobalMerge::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
unsigned CurIdx = 0;
for (j = i; j != -1; j = GlobalSet.find_next(j)) {
Type *Ty = Globals[j]->getValueType();

// Make sure we use the same alignment AsmPrinter would use. There
// currently isn't any helper to compute that, so we compute it
// explicitly here.
//
// getPreferredAlignment will sometimes return an alignment higher
// than the explicitly specified alignment; we must ignore that
// if the section is explicitly specified, to avoid inserting extra
// padding into that section.
unsigned Align = DL.getPreferredAlignment(Globals[j]);
if (Globals[j]->hasSection() && Globals[j]->getAlignment())
Align = Globals[j]->getAlignment();
unsigned Padding = alignTo(MergedSize, Align) - MergedSize;
MergedSize += Padding;
MergedSize += DL.getTypeAllocSize(Ty);
Expand Down Expand Up @@ -516,6 +527,7 @@ bool GlobalMerge::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
GlobalVariable::NotThreadLocal, AddrSpace);

MergedGV->setAlignment(MaxAlign);
MergedGV->setSection(Globals[i]->getSection());

const StructLayout *MergedLayout = DL.getStructLayout(MergedTy);
for (ssize_t k = i, idx = 0; k != j; k = GlobalSet.find_next(k), ++idx) {
Expand Down Expand Up @@ -599,16 +611,15 @@ bool GlobalMerge::doInitialization(Module &M) {
IsMachO = Triple(M.getTargetTriple()).isOSBinFormatMachO();

auto &DL = M.getDataLayout();
DenseMap<unsigned, SmallVector<GlobalVariable *, 16>> Globals, ConstGlobals,
BSSGlobals;
DenseMap<std::pair<unsigned, StringRef>, SmallVector<GlobalVariable *, 16>>
Globals, ConstGlobals, BSSGlobals;
bool Changed = false;
setMustKeepGlobalVariables(M);

// Grab all non-const globals.
for (auto &GV : M.globals()) {
// Merge is safe for "normal" internal or external globals only
if (GV.isDeclaration() || GV.isThreadLocal() ||
GV.hasSection() || GV.hasImplicitSection())
if (GV.isDeclaration() || GV.isThreadLocal() || GV.hasImplicitSection())
continue;

// It's not safe to merge globals that may be preempted
Expand All @@ -623,6 +634,7 @@ bool GlobalMerge::doInitialization(Module &M) {
assert(PT && "Global variable is not a pointer!");

unsigned AddressSpace = PT->getAddressSpace();
StringRef Section = GV.getSection();

// Ignore all 'special' globals.
if (GV.getName().startswith("llvm.") ||
Expand All @@ -637,26 +649,26 @@ bool GlobalMerge::doInitialization(Module &M) {
if (DL.getTypeAllocSize(Ty) < MaxOffset) {
if (TM &&
TargetLoweringObjectFile::getKindForGlobal(&GV, *TM).isBSSLocal())
BSSGlobals[AddressSpace].push_back(&GV);
BSSGlobals[{AddressSpace, Section}].push_back(&GV);
else if (GV.isConstant())
ConstGlobals[AddressSpace].push_back(&GV);
ConstGlobals[{AddressSpace, Section}].push_back(&GV);
else
Globals[AddressSpace].push_back(&GV);
Globals[{AddressSpace, Section}].push_back(&GV);
}
}

for (auto &P : Globals)
if (P.second.size() > 1)
Changed |= doMerge(P.second, M, false, P.first);
Changed |= doMerge(P.second, M, false, P.first.first);

for (auto &P : BSSGlobals)
if (P.second.size() > 1)
Changed |= doMerge(P.second, M, false, P.first);
Changed |= doMerge(P.second, M, false, P.first.first);

if (EnableGlobalMergeOnConst)
for (auto &P : ConstGlobals)
if (P.second.size() > 1)
Changed |= doMerge(P.second, M, true, P.first);
Changed |= doMerge(P.second, M, true, P.first.first);

return Changed;
}
Expand Down
21 changes: 16 additions & 5 deletions llvm/test/Transforms/GlobalMerge/basic.ll
Expand Up @@ -3,18 +3,29 @@
target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"

; CHECK: @_MergedGlobals = private global <{ i32, i32 }> <{ i32 1, i32 2 }>, align 4
; CHECK: @_MergedGlobals = private global <{ i32, i32 }> <{ i32 3, i32 4 }>, section "foo", align 4
; CHECK: @_MergedGlobals.1 = private global <{ i32, i32 }> <{ i32 1, i32 2 }>, align 4

; CHECK: @a = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 0)
; CHECK-DAG: @a = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals.1, i32 0, i32 0)
@a = internal global i32 1

; CHECK: @b = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 1)
; CHECK-DAG: @b = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals.1, i32 0, i32 1)
@b = internal global i32 2

; CHECK-DAG: @c = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 0)
@c = internal global i32 3, section "foo"

; CHECK-DAG: @d = internal alias i32, getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 1)
@d = internal global i32 4, section "foo"

define void @use() {
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 0)
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals.1, i32 0, i32 0)
%x = load i32, i32* @a
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 1)
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals.1, i32 0, i32 1)
%y = load i32, i32* @b
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 0)
%z1 = load i32, i32* @c
; CHECK: load i32, i32* getelementptr inbounds (<{ i32, i32 }>, <{ i32, i32 }>* @_MergedGlobals, i32 0, i32 1)
%z2 = load i32, i32* @d
ret void
}

0 comments on commit 1ba5e9a

Please sign in to comment.