[llvm] 885e610 - Revert "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally"
Alan Zhao via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 10 17:50:27 PST 2022
Author: Alan Zhao
Date: 2022-11-10T17:48:18-08:00
New Revision: 885e6105b4a6cb5fa8a80c33695c3a225d70fd45
URL: https://github.com/llvm/llvm-project/commit/885e6105b4a6cb5fa8a80c33695c3a225d70fd45
DIFF: https://github.com/llvm/llvm-project/commit/885e6105b4a6cb5fa8a80c33695c3a225d70fd45.diff
LOG: Revert "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally"
This reverts commit 89ddcff1d2d6e9f4de78f3a563a8b1987bf7ea8f.
Reason: This breaks bootstrapping builds of LLVM on Windows using
ThinLTO; see https://crbug.com/1382839
Added:
Modified:
llvm/lib/LTO/LTO.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll
llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
Removed:
llvm/test/ThinLTO/X86/constructor-alias.ll
################################################################################
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index dc28b681a1515..286d3ca3e2cc0 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -712,11 +712,11 @@ handleNonPrevailingComdat(GlobalValue &GV,
if (!NonPrevailingComdats.count(C))
return;
- // Additionally need to drop all global values from the comdat to
- // available_externally, to satisfy the COMDAT requirement that all members
- // are discarded as a unit. The non-local linkage global values avoid
- // duplicate definition linker errors.
- GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
+ // Additionally need to drop externally visible global values from the comdat
+ // to available_externally, so that there aren't multiply defined linker
+ // errors.
+ if (!GV.hasLocalLinkage())
+ GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
if (auto GO = dyn_cast<GlobalObject>(&GV))
GO->setComdat(nullptr);
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 844ee19e1e6a5..b589ec798caa1 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -1051,7 +1051,6 @@ bool llvm::convertToDeclaration(GlobalValue &GV) {
void llvm::thinLTOFinalizeInModule(Module &TheModule,
const GVSummaryMapTy &DefinedGlobals,
bool PropagateAttrs) {
- DenseSet<Comdat *> NonPrevailingComdats;
auto FinalizeInModule = [&](GlobalValue &GV, bool Propagate = false) {
// See if the global summary analysis computed a new resolved linkage.
const auto &GS = DefinedGlobals.find(GV.getGUID());
@@ -1129,10 +1128,8 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
// as this is a declaration for the linker, and will be dropped eventually.
// It is illegal for comdats to contain declarations.
auto *GO = dyn_cast_or_null<GlobalObject>(&GV);
- if (GO && GO->isDeclarationForLinker() && GO->hasComdat()) {
- NonPrevailingComdats.insert(GO->getComdat());
+ if (GO && GO->isDeclarationForLinker() && GO->hasComdat())
GO->setComdat(nullptr);
- }
};
// Process functions and global now
@@ -1142,36 +1139,6 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
FinalizeInModule(GV);
for (auto &GV : TheModule.aliases())
FinalizeInModule(GV);
-
- // For a non-prevailing comdat, all its members must be available_externally.
- // FinalizeInModule has handled non-local-linkage GlobalValues. Here we handle
- // local linkage GlobalValues.
- if (NonPrevailingComdats.empty())
- return;
- for (auto &GO : TheModule.global_objects()) {
- if (auto *C = GO.getComdat(); C && NonPrevailingComdats.count(C)) {
- GO.setComdat(nullptr);
- GO.setLinkage(GlobalValue::AvailableExternallyLinkage);
- }
- }
- bool Changed;
- do {
- Changed = false;
- // If an alias references a GlobalValue in a non-prevailing comdat, change
- // it to available_externally. For simplicity we don't handle ConstantExpr
- // aliasee, which is unlikely used in a COMDAT.
- for (auto &GA : TheModule.aliases()) {
- if (GA.hasAvailableExternallyLinkage())
- continue;
- assert(isa<GlobalValue>(GA.getAliasee()) &&
- "non-GlobalValue aliasee is unimplemented");
- if (const auto *GV = dyn_cast<GlobalValue>(GA.getAliasee()))
- if (GV->hasAvailableExternallyLinkage()) {
- GA.setLinkage(GlobalValue::AvailableExternallyLinkage);
- Changed = true;
- }
- }
- } while (Changed);
}
/// Run internalization on \p TheModule based on symmary analysis.
diff --git a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll
index 96d8f3157b996..d3730f4e9bcda 100644
--- a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll
+++ b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll
@@ -8,7 +8,7 @@
; The copy of C from this module is prevailing. The copy of C from the
; regular LTO module is not prevailing, and will be dropped to
; available_externally.
-; RUN: llvm-lto2 run -r=%t1.o,C,pl -r=%t2.o,C,l -r=%t1.o,testglobfunc,lxp -r=%t2.o,testglobfunc,lx -o %t3 %t1.o %t2.o -save-temps
+; RUN: llvm-lto2 run -r=%t1.o,C,pl -r=%t2.o,C,l -r=%t2.o,testglobfunc,lxp -r=%t1.o,testglobfunc,lx -o %t3 %t1.o %t2.o -save-temps
; The Input module (regular LTO) is %t3.0. Check to make sure that we removed
; __cxx_global_var_init and testglobfunc from comdat. Also check to ensure
@@ -16,21 +16,8 @@
; have linker multiply defined errors as it is no longer in a comdat and
; would clash with the copy from this module.
; RUN: llvm-dis %t3.0.0.preopt.bc -o - | FileCheck %s
-
-; CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init, ptr @C }]
-; CHECK: @C = available_externally dso_local global %"class.Test::ptr" zeroinitializer, align 4
-; CHECK-NOT: declare
-; CHECK: declare dso_local void @__cxx_global_var_init() section ".text.startup"
-; CHECK-NOT: declare
-
-; Check the behavior with the prevailing testglobfunc in %t2.o.
-; RUN: llvm-lto2 run -r=%t1.o,C,pl -r=%t2.o,C,l -r=%t1.o,testglobfunc,lx -r=%t2.o,testglobfunc,plx -o %t4 %t1.o %t2.o -save-temps
-; RUN: llvm-dis %t4.0.0.preopt.bc -o - | FileCheck %s --check-prefix=CHECK2
-
-; CHECK2: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init, ptr @C }]
-; CHECK2: @C = available_externally dso_local global %"class.Test::ptr" zeroinitializer, align 4
-; CHECK2: declare dso_local void @__cxx_global_var_init() section ".text.startup"
-; CHECK2: define available_externally dso_local void @testglobfunc() section ".text.startup" {
+; CHECK: define internal void @__cxx_global_var_init() section ".text.startup" {
+; CHECK: define available_externally dso_local void @testglobfunc() section ".text.startup" {
; ModuleID = 'comdat-mixed-lto.o'
source_filename = "comdat-mixed-lto.cpp"
diff --git a/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll b/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
index f5b3130fd1520..92b5182315943 100644
--- a/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
+++ b/llvm/test/ThinLTO/X86/Inputs/linkonce_resolution_comdat.ll
@@ -1,24 +1,13 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-$f = comdat any
-$g = comdat any
+$c2 = comdat any
- at g_private = private global i32 41, comdat($g)
-
-define linkonce_odr i32 @f(i8*) unnamed_addr comdat($f) {
- ret i32 41
-}
-
-define linkonce_odr i32 @g() unnamed_addr comdat($g) {
+define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c2) {
ret i32 41
}
-define internal void @g_internal() unnamed_addr comdat($g) {
- ret void
-}
-
-define i32 @h() {
+define i32 @g() {
%i = call i32 @f(i8* null)
ret i32 %i
}
diff --git a/llvm/test/ThinLTO/X86/constructor-alias.ll b/llvm/test/ThinLTO/X86/constructor-alias.ll
deleted file mode 100644
index 212ff7c425725..0000000000000
--- a/llvm/test/ThinLTO/X86/constructor-alias.ll
+++ /dev/null
@@ -1,44 +0,0 @@
-;; The constructor alias example is reduced from
-;;
-;; template <typename T>
-;; struct A { A() {} virtual ~A() {} };
-;; template struct A<void>;
-;; void *foo() { return new A<void>; }
-;;
-;; clang -c -fpic -O1 -flto=thin a.cc && cp a.o b.o && ld.lld -shared a.o b.so
-
-; RUN: opt -module-summary %s -o %t1.bc
-; RUN: cp %t1.bc %t2.bc
-; RUN: llvm-lto2 run %t1.bc %t2.bc -r=%t1.bc,_ZTV1A,pl -r=%t1.bc,_ZN1AD0Ev,pl -r=%t1.bc,_ZN1AD1Ev,pl -r=%t1.bc,_ZN1AD2Ev,pl -r=%t1.bc,D1_a,pl -r=%t1.bc,D1_a_a,pl \
-; RUN: -r=%t2.bc,_ZTV1A,l -r=%t2.bc,_ZN1AD0Ev,l -r=%t2.bc,_ZN1AD1Ev,l -r=%t2.bc,_ZN1AD2Ev,l -r=%t2.bc,D1_a,l -r=%t2.bc,D1_a_a,l -o %t3 --save-temps
-; RUN: llvm-dis < %t3.2.1.promote.bc | FileCheck %s
-
-; CHECK: @_ZTV1A = available_externally dso_local unnamed_addr constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN1AD1Ev, ptr @_ZN1AD0Ev] }
-; CHECK: @D1_a = available_externally dso_local unnamed_addr alias void (ptr), ptr @_ZN1AD1Ev
-; CHECK: @_ZN1AD1Ev = available_externally dso_local unnamed_addr alias void (ptr), ptr @_ZN1AD2Ev
-; CHECK: @D1_a_a = available_externally dso_local unnamed_addr alias void (ptr), ptr @D1_a
-; CHECK: define available_externally dso_local void @_ZN1AD2Ev(ptr noundef nonnull %0) unnamed_addr {
-; CHECK: define available_externally dso_local void @_ZN1AD0Ev(ptr noundef nonnull %0) unnamed_addr {
-
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-$_ZN1AD5Ev = comdat any
-$_ZTV1A = comdat any
-
- at _ZTV1A = weak_odr unnamed_addr constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr null, ptr @_ZN1AD1Ev, ptr @_ZN1AD0Ev] }, comdat
-
- at D1_a = weak_odr unnamed_addr alias void (ptr), ptr @_ZN1AD1Ev
- at _ZN1AD1Ev = weak_odr unnamed_addr alias void (ptr), ptr @_ZN1AD2Ev
- at D1_a_a = weak_odr unnamed_addr alias void (ptr), ptr @D1_a
-
-define weak_odr void @_ZN1AD2Ev(ptr noundef nonnull %0) unnamed_addr comdat($_ZN1AD5Ev) {
- ret void
-}
-
-define weak_odr void @_ZN1AD0Ev(ptr noundef nonnull %0) unnamed_addr comdat($_ZN1AD5Ev) {
- call void @D1_a(ptr noundef nonnull %0)
- call void @D1_a_a(ptr noundef nonnull %0)
- call void @_ZN1AD1Ev(ptr noundef nonnull %0)
- ret void
-}
diff --git a/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll b/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
index 2fb226046ea9f..7b22180132e6a 100644
--- a/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
+++ b/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll
@@ -1,54 +1,33 @@
-; This test ensures that we drop the preempted copy of @f/@g from %t2.bc from their
-; comdats after making it available_externally. If not we would get a
-; verification error. g_internal/g_private are changed to available_externally
-; as well since it is in the same comdat of g.
+; This test ensures that we drop the preempted copy of @f from %t2.bc from its
+; comdat after making it available_externally. If not we would get a
+; verification error.
; RUN: opt -module-summary %s -o %t1.bc
; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -exported-symbol=h -thinlto-save-temps=%t3.
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
; Copy from first module is prevailing and converted to weak_odr, copy
; from second module is preempted and converted to available_externally and
; removed from comdat.
-; IMPORT1: @g_private = private global i32 43, comdat($g)
-; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] comdat {
-; IMPORT1: define weak_odr i32 @g() unnamed_addr [[ATTR]] comdat {
-; IMPORT1: define internal void @g_internal() unnamed_addr comdat($g) {
-
-; IMPORT2: @g_private = available_externally dso_local global i32 41{{$}}
+; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] comdat($c1) {
; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] {
-; IMPORT2: define available_externally i32 @g() unnamed_addr [[ATTR]] {
-; IMPORT2: define available_externally dso_local void @g_internal() unnamed_addr {
; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
-; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
+; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
; NM1: W f
-; NM1: W g
-; RUN: llvm-nm %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
+; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
; f() would have been turned into available_externally since it is preempted,
-; and inlined into h()
+; and inlined into g()
; NM2-NOT: f
-; NM2-NOT: g
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-$f = comdat any
-$g = comdat any
-
- at g_private = private global i32 43, comdat($g)
+$c1 = comdat any
-define linkonce_odr i32 @f(i8*) unnamed_addr comdat {
+define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c1) {
ret i32 43
}
-
-define linkonce_odr i32 @g() unnamed_addr comdat {
- ret i32 43
-}
-
-define internal void @g_internal() unnamed_addr comdat($g) {
- ret void
-}
More information about the llvm-commits
mailing list