This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Import linkonce_odr as available_externally when possible
Needs ReviewPublic

Authored by tejohnson on Feb 3 2017, 12:57 PM.

Details

Reviewers
pcc
mehdi_amini
Summary

We no longer need to import linkonce_odr as linkonce_odr because
with weak symbol resolution we now promote the prevailing copy to
weak_odr, and therefore we are guaranteed to have an externally
available copy somewhere. We also no longer force import linkonce_odr
on reference, so the comment was wrong.

This change is theoretically necessary due to r294014 which changed
the importer to use the IRMover directly instead of the ModuleLinker.
The ModuleLinker had some magic to link in the rest of the comdat
including any values marked for link. This had other issues as we don't
generally want to import values not selected by the thin link, but it
would have protected against creating an incomplete comdat group. Since
with this change we will now always import as available_externally
(which are condidered declarations and dropped from any comdat), we
avoid this issue.

Note that other changes are required to the fact that we can no longer
import aliases that have a linkonce_odr aliasee, so some special casing
is removed and tests changed accordingly.

Event Timeline

tejohnson created this revision.Feb 3 2017, 12:57 PM
tejohnson updated this revision to Diff 87007.Feb 3 2017, 1:01 PM

Update assertion message.

mehdi_amini edited edge metadata.Feb 3 2017, 1:03 PM

Does this mean that now linkonce_odr > weak_odr promotion is required for correctness?

Does this mean that now linkonce_odr > weak_odr promotion is required for correctness?

Yes

Does this mean that now linkonce_odr > weak_odr promotion is required for correctness?

Yes

OK, I need to wrap my head around this first.

tejohnson edited the summary of this revision. (Show Details)Feb 7 2017, 3:01 PM