This is an archive of the discontinued LLVM Phabricator instance.

LTO: Don't internalize available_externally globals.
ClosedPublic

Authored by pcc on Dec 3 2018, 3:28 PM.

Details

Summary

This breaks C and C++ semantics because it can cause the address
of the global inside the module to differ from the address outside
of the module.

Diff Detail

Event Timeline

pcc created this revision.Dec 3 2018, 3:28 PM
pirama added inline comments.Dec 3 2018, 3:32 PM
llvm/lib/LTO/LTO.cpp
363

Use !GLobalValue::isAvailableExternallyLinkage(S->linkage()) instead?

Also, we should update the earlier comment.

pcc updated this revision to Diff 176503.Dec 3 2018, 3:44 PM
pcc marked an inline comment as done.

Add comment

llvm/lib/LTO/LTO.cpp
363

I'd prefer not to since it boils down to a comparison and it's slightly clearer to be explicit. Added comment and changed the other condition to be explicit as well.

srhines added a subscriber: srhines.Dec 3 2018, 5:25 PM
tejohnson accepted this revision.Dec 4 2018, 4:05 PM

Lgtm

llvm/lib/LTO/LTO.cpp
362

This change seems unnecessary?

This revision is now accepted and ready to land.Dec 4 2018, 4:05 PM
pcc added inline comments.Dec 4 2018, 4:09 PM
llvm/lib/LTO/LTO.cpp
362

Just making it consistent with the code I'm adding below.

This revision was automatically updated to reflect the committed changes.