Revert D135427 "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally"

This reverts commit 8ef3fd8d59.

I mentioned that GlobalAlias was not handled. It turns out GlobalAlias has to be handled in the same patch (as opposed to in a follow-up),
as otherwise clang codegen of C5/D5 constructor/destructor would regress (https://reviews.llvm.org/D135427#3869003).
This commit is contained in:
Fangrui Song 2022-10-19 11:24:12 -07:00
parent 26eb2c6b42
commit c80b12d352
5 changed files with 22 additions and 82 deletions

View File

@ -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);

View File

@ -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,18 +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);
}
}
}
/// Run internalization on \p TheModule based on symmary analysis.

View File

@ -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"

View File

@ -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
@g_private = private global i32 41, comdat($g)
define linkonce_odr i32 @f(i8*) unnamed_addr comdat($f) {
define linkonce_odr i32 @f(i8*) unnamed_addr comdat($c2) {
ret i32 41
}
define linkonce_odr i32 @g() unnamed_addr comdat($g) {
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
}

View File

@ -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
$c1 = comdat any
@g_private = private global i32 43, comdat($g)
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
}