[llvm] r260122 - [ThinLTO] Remove imported available externally defs from comdats.

[David Blaikie via llvm-commits]

I couldn't quite follow the test - but wouldn't removing things (or in any
way modifyidng) from a COMDAT cause problems? (the COMDAT would no longer
match between objects, etc?)

On Mon, Feb 8, 2016 at 10:47 AM, Teresa Johnson via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: tejohnson
> Date: Mon Feb  8 12:47:20 2016
> New Revision: 260122
>
> URL: http://llvm.org/viewvc/llvm-project?rev=260122&view=rev
> Log:
> [ThinLTO] Remove imported available externally defs from comdats.
>
> Summary:
> Available externally definitions are considered declarations for the
> linker and eventually dropped. As such they are not allowed to be
> in comdats. Remove any such imported functions from comdats.
>
> Reviewers: rafael
>
> Subscribers: davidxl, llvm-commits, joker.eph
>
> Differential Revision: http://reviews.llvm.org/D16120
>
> Added:
>     llvm/trunk/test/Linker/Inputs/funcimport_comdat.ll
>     llvm/trunk/test/Linker/funcimport_comdat.ll
> Modified:
>     llvm/trunk/lib/Linker/LinkModules.cpp
>
> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=260122&r1=260121&r2=260122&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> +++ llvm/trunk/lib/Linker/LinkModules.cpp Mon Feb  8 12:47:20 2016
> @@ -722,9 +722,21 @@ void ThinLTOGlobalProcessing::processGlo
>        GV.setVisibility(GlobalValue::HiddenVisibility);
>      if (isModuleExporting())
>        NewExportedValues.insert(&GV);
> -    return;
> +  } else
> +    GV.setLinkage(getLinkage(&GV));
> +
> +  // Remove functions imported as available externally defs from comdats,
> +  // 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()) {
> +    // The IRMover should not have placed any imported declarations in
> +    // a comdat, so the only declaration that should be in a comdat
> +    // at this point would be a definition imported as
> available_externally.
> +    assert(GO->hasAvailableExternallyLinkage() &&
> +           "Expected comdat on definition (possibly available external)");
> +    GO->setComdat(nullptr);
>    }
> -  GV.setLinkage(getLinkage(&GV));
>  }
>
>  void ThinLTOGlobalProcessing::processGlobalsForThinLTO() {
>
> Added: llvm/trunk/test/Linker/Inputs/funcimport_comdat.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/funcimport_comdat.ll?rev=260122&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/funcimport_comdat.ll (added)
> +++ llvm/trunk/test/Linker/Inputs/funcimport_comdat.ll Mon Feb  8 12:47:20
> 2016
> @@ -0,0 +1,4 @@
> +define i32 @main() #0 {
> +entry:
> +  ret i32 0
> +}
>
> Added: llvm/trunk/test/Linker/funcimport_comdat.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/funcimport_comdat.ll?rev=260122&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/funcimport_comdat.ll (added)
> +++ llvm/trunk/test/Linker/funcimport_comdat.ll Mon Feb  8 12:47:20 2016
> @@ -0,0 +1,28 @@
> +; Do setup work for all below tests: generate bitcode and combined index
> +; RUN: llvm-as -function-summary %s -o %t.bc
> +; RUN: llvm-as -function-summary %p/Inputs/funcimport_comdat.ll -o %t2.bc
> +; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
> +
> +; Ensure linking of comdat containing external linkage global and function
> +; removes the imported available_externally defs from comdat.
> +; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc
> -import=comdat1_func1:%t.bc -S | FileCheck %s --check-prefix=IMPORTCOMDAT
> +; IMPORTCOMDAT-NOT: $comdat1 = comdat any
> +; IMPORTCOMDAT-NOT: comdat($comdat1)
> +
> +; Ensure linking of comdat containing internal linkage function with alias
> +; removes the imported and promoted available_externally defs from comdat.
> +; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc
> -import=comdat2_func1:%t.bc -S | FileCheck %s --check-prefix=IMPORTCOMDAT2
> +; IMPORTCOMDAT2-NOT: $comdat2 = comdat any
> +; IMPORTCOMDAT2-NOT: comdat($comdat2)
> +
> +$comdat1 = comdat any
> + at comdat1_glob = global i32 0, comdat($comdat1)
> +define void @comdat1_func1() comdat($comdat1) {
> +  ret void
> +}
> +
> +$comdat2 = comdat any
> + at comdat2_alias = alias void (), void ()* @comdat2_func1
> +define internal void @comdat2_func1() comdat($comdat2) {
> +  ret void
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160208/2cbb4e8c/attachment.html>