[PATCH] D22467: [ThinLTO] Avoid backwards references to renamed locals in distributed backend case

[Teresa Johnson via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: mehdi_amini, pcc.
tejohnson added a subscriber: llvm-commits.
tejohnson added a dependency: D22356: [ThinLTO] Perform conservative weak/linkonce resolution in distributed backend case.
Herald added a subscriber: mehdi_amini.

Address another issue related to distributed backends and
--start-lib/--end-lib (see D22356 for more background).

We can also run into trouble when importing the prevailing copy of a
weak symbol if it references a local value. The local value will be
promoted and renamed using the module hash of the prevailing copy. If
that prevailing copy is not needed by the final native link, due to
intervening imports/inlines, that renamed copy of the local value may
not be linked in, and any importing module in a library listed later on
the link line will have an undefined reference.

Avoid this via a new PreventBackwardsModuleRefs flag to the importer
which prevents linking in any multiply-defined symbol if it references
a local needing renaming. We pass true for the flag in the
thinlto_index_only case (where we will have distributed backends and a
separate native link process).

I considered a couple of other solutions:

1) Always link in the last copy of a multiply defined symbol. However,
in the non-odr case this may change the program semantics as we would be
importing a different copy than the linker would normally select.

2) Force import the referenced local and as a local copy (don't use the
promoted copy). This is the better long term fix (and I have added a
FIXME to gold-plugin to that effect), but it also relies on doing
importing in the distributed backeds which is not yet there (will come
in via D21545). In that case we would still need to detect this case in
the importer, but force import the referenced locals instead of
preventing the referencing weak/linkonce import. We would also need to
either mark all copies of the referenced local for internalization after
importing, or only those copies in the distributed backend individual index
files that don't also have a singly-defined reference to the source module
(meaning the source module must be after the importing module on the
link line).

Added a new test for this case.

Depends on D22356.

https://reviews.llvm.org/D22467

Files:
  include/llvm/Transforms/IPO/FunctionImport.h
  lib/Transforms/IPO/FunctionImport.cpp
  test/tools/gold/X86/Inputs/thinlto_preventbackwardsref-1.ll
  test/tools/gold/X86/Inputs/thinlto_preventbackwardsref-2.ll
  test/tools/gold/X86/Inputs/thinlto_preventbackwardsref-3.ll
  test/tools/gold/X86/thinlto_preventbackwardsref.ll
  tools/gold/gold-plugin.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22467.64348.patch
Type: text/x-patch
Size: 21586 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160718/f1b2bfac/attachment.bin>