Page MenuHomePhabricator

[ThinLTO/WPD] Ensure devirtualized targets use promoted symbol when necessary
ClosedPublic

Authored by tejohnson on Sep 27 2019, 1:22 PM.

Details

Summary

This fixes a hole in the handling of devirtualized targets that were
local but need promoting due to devirtualization in another module. We
were not correctly referencing the promoted symbol in some cases. Make
sure the code that updates the name also looks at the ExportedGUIDs set
by utilizing a callback that checks all conditions (the callback
utilized by the internalization/promotion code).

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson created this revision.Sep 27 2019, 1:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2019, 1:22 PM
hiraditya added inline comments.Oct 1 2019, 2:40 PM
lib/LTO/LTO.cpp
1335 ↗(On Diff #222235)

Maybe we could make this a separate function, it seems we are using similar checks in llvm/lib/LTO/ThinLTOCodeGenerator.cpp:528,

tejohnson marked an inline comment as done.Oct 1 2019, 4:14 PM
tejohnson added inline comments.
lib/LTO/LTO.cpp
1335 ↗(On Diff #222235)

Do you mean the isExported lambda (ThinLTOCodeGenerator.cpp:528 is unrelated code at HEAD)? We use a call back in part because the different LTO APIs handle exported symbols differently. The ExportedGUIDs set here in the new API is set up differently than the GUIDPreservedSymbols in the old API. Also, the old LTO API doesn't currently do WPD.

hiraditya accepted this revision.Oct 1 2019, 5:54 PM
hiraditya added inline comments.
lib/LTO/LTO.cpp
1335 ↗(On Diff #222235)

oh sorry, i was referring to: https://github.com/llvm-mirror/llvm/blob/master/lib/LTO/ThinLTOCodeGenerator.cpp#L586. But i understood the point. thanks!

This revision is now accepted and ready to land.Oct 1 2019, 5:54 PM
This revision was automatically updated to reflect the committed changes.