Skip to content

Commit 9808035

Browse files
Respect linkage type in jl_merge_module (#50678)
We shouldn't merge non-external definitions in `jl_merge_module`, so that we can freely emit internal globals without relying on a global counter. Depends on #50650
1 parent ec8df3d commit 9808035

File tree

2 files changed

+47
-41
lines changed

2 files changed

+47
-41
lines changed

src/codegen.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1796,8 +1796,11 @@ static inline GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G)
17961796
if (!local) {
17971797
// Copy the GlobalVariable, but without the initializer, so it becomes a declaration
17981798
GlobalVariable *proto = new GlobalVariable(*M, G->getValueType(),
1799-
G->isConstant(), GlobalVariable::ExternalLinkage,
1799+
G->isConstant(), G->getLinkage(),
18001800
nullptr, G->getName(), nullptr, G->getThreadLocalMode());
1801+
if (proto->hasLocalLinkage()) {
1802+
proto->setInitializer(G->getInitializer());
1803+
}
18011804
proto->copyAttributesFrom(G);
18021805
// DLLImport only needs to be set for the shadow module
18031806
// it just gets annoying in the JIT

src/jitlayers.cpp

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,21 +1864,22 @@ void jl_merge_module(orc::ThreadSafeModule &destTSM, orc::ThreadSafeModule srcTS
18641864
assert(dest.getDataLayout() == src.getDataLayout() && "Cannot merge modules with different data layouts!");
18651865
assert(dest.getTargetTriple() == src.getTargetTriple() && "Cannot merge modules with different target triples!");
18661866

1867-
for (Module::global_iterator I = src.global_begin(), E = src.global_end(); I != E;) {
1868-
GlobalVariable *sG = &*I;
1869-
GlobalVariable *dG = cast_or_null<GlobalVariable>(dest.getNamedValue(sG->getName()));
1870-
++I;
1867+
for (auto &SG : make_early_inc_range(src.globals())) {
1868+
GlobalVariable *dG = cast_or_null<GlobalVariable>(dest.getNamedValue(SG.getName()));
1869+
if (SG.hasLocalLinkage()) {
1870+
dG = nullptr;
1871+
}
18711872
// Replace a declaration with the definition:
1872-
if (dG) {
1873-
if (sG->isDeclaration()) {
1874-
sG->replaceAllUsesWith(dG);
1875-
sG->eraseFromParent();
1873+
if (dG && !dG->hasLocalLinkage()) {
1874+
if (SG.isDeclaration()) {
1875+
SG.replaceAllUsesWith(dG);
1876+
SG.eraseFromParent();
18761877
continue;
18771878
}
18781879
//// If we start using llvm.used, we need to enable and test this
1879-
//else if (!dG->isDeclaration() && dG->hasAppendingLinkage() && sG->hasAppendingLinkage()) {
1880+
//else if (!dG->isDeclaration() && dG->hasAppendingLinkage() && SG.hasAppendingLinkage()) {
18801881
// auto *dCA = cast<ConstantArray>(dG->getInitializer());
1881-
// auto *sCA = cast<ConstantArray>(sG->getInitializer());
1882+
// auto *sCA = cast<ConstantArray>(SG.getInitializer());
18821883
// SmallVector<Constant *, 16> Init;
18831884
// for (auto &Op : dCA->operands())
18841885
// Init.push_back(cast_or_null<Constant>(Op));
@@ -1890,67 +1891,69 @@ void jl_merge_module(orc::ThreadSafeModule &destTSM, orc::ThreadSafeModule srcTS
18901891
// GlobalValue::AppendingLinkage, ConstantArray::get(ATy, Init), "",
18911892
// dG->getThreadLocalMode(), dG->getType()->getAddressSpace());
18921893
// GV->copyAttributesFrom(dG);
1893-
// sG->replaceAllUsesWith(GV);
1894+
// SG.replaceAllUsesWith(GV);
18941895
// dG->replaceAllUsesWith(GV);
1895-
// GV->takeName(sG);
1896-
// sG->eraseFromParent();
1896+
// GV->takeName(SG);
1897+
// SG.eraseFromParent();
18971898
// dG->eraseFromParent();
18981899
// continue;
18991900
//}
19001901
else {
1901-
assert(dG->isDeclaration() || dG->getInitializer() == sG->getInitializer());
1902-
dG->replaceAllUsesWith(sG);
1902+
assert(dG->isDeclaration() || dG->getInitializer() == SG.getInitializer());
1903+
dG->replaceAllUsesWith(&SG);
19031904
dG->eraseFromParent();
19041905
}
19051906
}
19061907
// Reparent the global variable:
1907-
sG->removeFromParent();
1908-
dest.getGlobalList().push_back(sG);
1908+
SG.removeFromParent();
1909+
dest.getGlobalList().push_back(&SG);
19091910
// Comdat is owned by the Module
1910-
sG->setComdat(nullptr);
1911+
SG.setComdat(nullptr);
19111912
}
19121913

1913-
for (Module::iterator I = src.begin(), E = src.end(); I != E;) {
1914-
Function *sG = &*I;
1915-
Function *dG = cast_or_null<Function>(dest.getNamedValue(sG->getName()));
1916-
++I;
1914+
for (auto &SG : make_early_inc_range(src)) {
1915+
Function *dG = cast_or_null<Function>(dest.getNamedValue(SG.getName()));
1916+
if (SG.hasLocalLinkage()) {
1917+
dG = nullptr;
1918+
}
19171919
// Replace a declaration with the definition:
1918-
if (dG) {
1919-
if (sG->isDeclaration()) {
1920-
sG->replaceAllUsesWith(dG);
1921-
sG->eraseFromParent();
1920+
if (dG && !dG->hasLocalLinkage()) {
1921+
if (SG.isDeclaration()) {
1922+
SG.replaceAllUsesWith(dG);
1923+
SG.eraseFromParent();
19221924
continue;
19231925
}
19241926
else {
19251927
assert(dG->isDeclaration());
1926-
dG->replaceAllUsesWith(sG);
1928+
dG->replaceAllUsesWith(&SG);
19271929
dG->eraseFromParent();
19281930
}
19291931
}
19301932
// Reparent the global variable:
1931-
sG->removeFromParent();
1932-
dest.getFunctionList().push_back(sG);
1933+
SG.removeFromParent();
1934+
dest.getFunctionList().push_back(&SG);
19331935
// Comdat is owned by the Module
1934-
sG->setComdat(nullptr);
1936+
SG.setComdat(nullptr);
19351937
}
19361938

1937-
for (Module::alias_iterator I = src.alias_begin(), E = src.alias_end(); I != E;) {
1938-
GlobalAlias *sG = &*I;
1939-
GlobalAlias *dG = cast_or_null<GlobalAlias>(dest.getNamedValue(sG->getName()));
1940-
++I;
1941-
if (dG) {
1939+
for (auto &SG : make_early_inc_range(src.aliases())) {
1940+
GlobalAlias *dG = cast_or_null<GlobalAlias>(dest.getNamedValue(SG.getName()));
1941+
if (SG.hasLocalLinkage()) {
1942+
dG = nullptr;
1943+
}
1944+
if (dG && !dG->hasLocalLinkage()) {
19421945
if (!dG->isDeclaration()) { // aliases are always definitions, so this test is reversed from the above two
1943-
sG->replaceAllUsesWith(dG);
1944-
sG->eraseFromParent();
1946+
SG.replaceAllUsesWith(dG);
1947+
SG.eraseFromParent();
19451948
continue;
19461949
}
19471950
else {
1948-
dG->replaceAllUsesWith(sG);
1951+
dG->replaceAllUsesWith(&SG);
19491952
dG->eraseFromParent();
19501953
}
19511954
}
1952-
sG->removeFromParent();
1953-
dest.getAliasList().push_back(sG);
1955+
SG.removeFromParent();
1956+
dest.getAliasList().push_back(&SG);
19541957
}
19551958

19561959
// metadata nodes need to be explicitly merged not just copied

0 commit comments

Comments
 (0)