From: Fangrui Song Date: Wed, 19 Oct 2022 18:24:12 +0000 (-0700) Subject: Revert D135427 "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT availab... X-Git-Tag: upstream/17.0.6~30096 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c80b12d352d5c88cd812bf2707672f11b3010984;p=platform%2Fupstream%2Fllvm.git Revert D135427 "[LTO] Make local linkage GlobalValue in non-prevailing COMDAT available_externally" This reverts commit 8ef3fd8d59ba0100bc6e83350ab1e978536aa531. 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). --- diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index dc28b68..286d3ca 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(&GV)) GO->setComdat(nullptr); diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index c5b2135..b589ec7 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 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(&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. diff --git a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll index 96d8f31..d3730f4 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 f5b3130..92b5182 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 -@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/linkonce_resolution_comdat.ll b/llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll index 2fb2260..7b22180 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 - -@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 -}