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

[Teresa Johnson via llvm-commits]

On Mon, Feb 8, 2016 at 11:06 AM, David Blaikie <dblaikie at gmail.com> wrote:

> 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?)
>

The comdat will not be emitted in the importing module if we import its
definitions as available_externally, since any such imported defs will be
removed from the comdat with this patch.  Any non-imported members remained
decls and are not part of a comdat in the importing module either.


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


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160208/c7062b98/attachment.html>