Commit Graph

5 Commits

Author SHA1 Message Date
Teresa Johnson 2d5fb8cac4 Ensure ModuleLinker materializes complete comdat groups
Summary:
The module linker lazy links some "discardable if unused" global
values (e.g. linkonce), materializing and linking them only
if they are referenced in the module. If a comdat group contains a
linkonce member that is not referenced, however, it would not be
materialized and linked, leading to an incomplete comdat group.

If there are other object files not part of the same LTO link that also
define and use that comdat group, the linker may select the incomplete
group leading to link time unsats.

To solve this, whenever a global value body is linked, make sure we
materialize any other members of the same comdat group that are not yet
materialized. This ensures they are in the lazy link list and get linked
as well.

Added new test and adjusted old test to remove parts that didn't
make sense with fix.

Reviewers: rafael

Subscribers: dexonsmith, davidxl, llvm-commits

Differential Revision: http://reviews.llvm.org/D14516

llvm-svn: 252647
2015-11-10 21:09:06 +00:00
Teresa Johnson 1063293a89 Restore "Move metadata linking after lazy global materialization/linking."
Summary:
This reverts commit r251965.

Restore "Move metadata linking after lazy global materialization/linking."

This restores commit r251926, with fixes for the LTO bootstrapping bot
failure.

The bot failure was caused by references from debug metadata to
otherwise unreferenced globals. Previously, this caused the lazy linking
to link in their defs, which is unnecessary. With this patch, because
lazy linking is complete when we encounter the metadata reference, the
materializer created a declaration. For definitions such as aliases and
comdats, it is illegal to have a declaration. Furthermore, metadata
linking should not change code generation. Therefore, when linking of
global value bodies is complete, the materializer will simply return
nullptr as the new reference for the linked metadata.

This change required fixing a different test to ensure there was a
real reference to a linkonce global that was only being reference from
metadata.

Note that the new changes to the only-needed-named-metadata.ll test
illustrate an issue with llvm-link -only-needed handling of comdat
groups, whereby it may result in an incomplete comdat group. I note this
in the test comments, but the issue is orthogonal to this patch (it can
be reproduced without any metadata at head).

Reviewers: dexonsmith, rafael, tra

Subscribers: tobiasvk, joker.eph, llvm-commits

Differential Revision: http://reviews.llvm.org/D14447

llvm-svn: 252320
2015-11-06 17:50:53 +00:00
Teresa Johnson 189b252652 Restore "Move metadata linking after lazy global materialization/linking."
This reverts commit r251965.

llvm-svn: 252319
2015-11-06 17:50:48 +00:00
Teresa Johnson 255787a969 Revert "Move metadata linking after lazy global materialization/linking."
This reverts commit r251926. I believe this is causing an LTO
bootstrapping bot failure
(http://lab.llvm.org:8080/green/job/llvm-stage2-cmake-RgLTO_build/3669/).

Haven't been able to repro it yet, but after looking at the metadata I
am pretty sure I know what is going on.

llvm-svn: 251965
2015-11-03 19:36:04 +00:00
Teresa Johnson 07b825b01c Move metadata linking after lazy global materialization/linking.
Summary:
Currently, named metadata is linked before the LazilyLinkGlobalValues
list is walked and materialized/linked. As a result, references
from DISubprogram and DIGlobalVariable metadata to yet unmaterialized
functions and variables cause them to be added to the lazy linking
list and their definitions are materialized and linked.

This makes the llvm-link -only-needed option not have the intended
effect when debug information is present, as the otherwise unneeded
functions/variables are still linked in.

Additionally, for ThinLTO I have implemented a mechanism to only link
in debug metadata needed by imported functions. Moving named metadata
linking after lazy GV linking will facilitate applying this mechanism
to the LTO and "llvm-link -only-needed" cases as well.

Reviewers: dexonsmith, tra, dblaikie

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D14195

llvm-svn: 251926
2015-11-03 15:11:27 +00:00