[PATCH] D34803: [LTO] Remove values from non-prevailing comdats
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 12:49:51 PDT 2017
I am probably missing something.
But if a symbol in a comdat is prevailing and another one is not, that is
a bug in the linker, no?
Cheers,
Rafael
Teresa Johnson via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:
> tejohnson created this revision.
> Herald added subscribers: inglorion, mehdi_amini.
>
> When linking a regular LTO module, if it has any non-prevailing values
> (dropped to available_externally) in comdats, we need to do more than
> just remove those values from their comdat. We also remove all values
> from that comdat, so as to avoid leaving an incomplete comdat.
>
> This is necessary in case we are compiling in mixed regular and ThinLTO
> mode, since the resulting regularLTO native object is always linked into
> the final binary first. We need to prevent the linker from selecting an
> incomplete comdat that was not the prevailing copy.
>
> Fixes PR32980.
>
>
> https://reviews.llvm.org/D34803
>
> Files:
> lib/LTO/LTO.cpp
> test/LTO/Resolution/X86/Inputs/comdat-mixed-lto.ll
> test/LTO/Resolution/X86/comdat-mixed-lto.ll
>
> Index: test/LTO/Resolution/X86/comdat-mixed-lto.ll
> ===================================================================
> --- /dev/null
> +++ test/LTO/Resolution/X86/comdat-mixed-lto.ll
> @@ -0,0 +1,42 @@
> +; Test of comdat handling with mixed thinlto and regular lto compilation.
> +
> +; This module is compiled with ThinLTO
> +; RUN: opt -module-summary -o %t1.o %s
> +; Input module compiled for regular LTO
> +; RUN: opt -o %t2.o %p/Inputs/comdat-mixed-lto.ll
> +
> +; 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=%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
> +; that testglobfunc was dropped to available_externally. Otherwise we would
> +; 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: define internal void @__cxx_global_var_init() section ".text.startup" {
> +; CHECK: define available_externally void @testglobfunc() section ".text.startup" {
> +
> +; ModuleID = 'comdat-mixed-lto.o'
> +source_filename = "comdat-mixed-lto.cpp"
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +%"class.Test::ptr" = type { i32 }
> +
> +$C = comdat any
> +
> + at C = linkonce_odr global %"class.Test::ptr" zeroinitializer, comdat, align 4
> + at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__cxx_global_var_init, i8* bitcast (%"class.Test::ptr"* @C to i8*) }]
> +define void @testglobfunc() #1 section ".text.startup" comdat($C) {
> +entry:
> + ret void
> +}
> +
> +; Function Attrs: noinline uwtable
> +define internal void @__cxx_global_var_init() #1 section ".text.startup" comdat($C) {
> +entry:
> + ret void
> +}
> Index: test/LTO/Resolution/X86/Inputs/comdat-mixed-lto.ll
> ===================================================================
> --- /dev/null
> +++ test/LTO/Resolution/X86/Inputs/comdat-mixed-lto.ll
> @@ -0,0 +1,23 @@
> +; ModuleID = 'comdat-mixed-lto1.o'
> +source_filename = "comdat-mixed-lto1.cpp"
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +%"class.Test::ptr" = type { i32 }
> +
> +$C = comdat any
> +
> + at C = linkonce_odr global %"class.Test::ptr" zeroinitializer, comdat, align 4
> + at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__cxx_global_var_init, i8* bitcast (%"class.Test::ptr"* @C to i8*) }]
> +
> +define void @testglobfunc() #1 section ".text.startup" comdat($C) {
> +entry:
> + ret void
> +}
> +
> +; Function Attrs: noinline uwtable
> +define internal void @__cxx_global_var_init() #1 section ".text.startup" comdat($C) {
> +entry:
> + store i32 0, i32* getelementptr inbounds (%"class.Test::ptr", %"class.Test::ptr"* @C, i32 0, i32 0), align 4
> + ret void
> +}
> Index: lib/LTO/LTO.cpp
> ===================================================================
> --- lib/LTO/LTO.cpp
> +++ lib/LTO/LTO.cpp
> @@ -472,6 +472,36 @@
> return Error::success();
> }
>
> +// Checks whether the given global value is in a non-prevailing comdat
> +// (comdat containing values the linker indicated were not prevailing,
> +// which we then dropped to available_externally), and if so, removes
> +// it from the comdat. This is called for all global values to ensure the
> +// comdat is empty rather than leaving an incomplete comdat. It is needed for
> +// regular LTO modules, in case we are in a mixed-LTO mode (both regular
> +// and thin LTO modules) compilation. Since the regular LTO module will be
> +// linked first in the final native link, we want to make sure the linker
> +// doesn't select any of these incomplete comdats that would be left
> +// in the regular LTO module without this cleanup.
> +static void
> +handleNonPrevailingComdat(GlobalValue &GV,
> + std::set<const Comdat *> &NonPrevailingComdats) {
> + Comdat *C = GV.getComdat();
> + if (!C)
> + return;
> +
> + if (!NonPrevailingComdats.count(C))
> + return;
> +
> + // 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);
> +}
> +
> // Add a regular LTO object to the link.
> // The resulting module needs to be linked into the combined LTO module with
> // linkRegularLTO.
> @@ -523,6 +553,7 @@
> };
> Skip();
>
> + std::set<const Comdat *> NonPrevailingComdats;
> for (const InputFile::Symbol &Sym : Syms) {
> assert(ResI != ResE);
> SymbolResolution Res = *ResI++;
> @@ -557,6 +588,8 @@
> // module (in linkRegularLTO), based on whether it is undefined.
> Mod.Keep.push_back(GV);
> GV->setLinkage(GlobalValue::AvailableExternallyLinkage);
> + if (GV->hasComdat())
> + NonPrevailingComdats.insert(GV->getComdat());
> cast<GlobalObject>(GV)->setComdat(nullptr);
> }
> }
> @@ -574,6 +607,9 @@
>
> // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
> }
> + if (!M.getComdatSymbolTable().empty())
> + for (GlobalValue &GV : M.global_values())
> + handleNonPrevailingComdat(GV, NonPrevailingComdats);
> assert(MsymI == MsymE);
> return std::move(Mod);
> }
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list