This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Infer the correct option to ld64 for -fembed-bitcode
ClosedPublic

Authored by steven_wu on Nov 15 2016, 12:28 PM.

Details

Summary

-fembed-bitcode infers -bitcode_bundle to ld64 but it is not correctly
passed when using LTO. LTO is a special case of -fembed-bitcode which
it doesn't require embed the bitcode in a special section in the object
file but it requires linker to save that as part of the final executable.

rdar://problem/29274226

Event Timeline

steven_wu updated this revision to Diff 78051.Nov 15 2016, 12:28 PM
steven_wu retitled this revision from to [Driver] Infer the correct option to ld64 for -fembed-bitcode.
steven_wu updated this object.
steven_wu added a reviewer: mehdi_amini.
steven_wu added a subscriber: cfe-commits.
steven_wu updated this object.Nov 15 2016, 1:15 PM
mehdi_amini added inline comments.Nov 15 2016, 1:20 PM
lib/Driver/Tools.cpp
8326

Why aren't these tests returning true when LTO is enabled?
I'm not sure why this is not the part that should be fixed instead.

steven_wu added inline comments.Nov 15 2016, 1:25 PM
lib/Driver/Tools.cpp
8326

These flags controls if there should be bitcode embedded in the object file. For the case of LTO, there is no object file thus there is no embedding.
If change the meaning of this flag, it will complicate the logic around line 6355 and 4169. The other option is to add another flag but it seems little too much for passing one flag to ld.

mehdi_amini added inline comments.Nov 15 2016, 2:50 PM
lib/Driver/Tools.cpp
8326

From the driver point of view, having a method embedBitcodeEnabled() returning false when ... it is actually enabled does not make sense to me.

At minima it should be renamed.

steven_wu updated this revision to Diff 78111.Nov 15 2016, 5:12 PM

Rename the functions to clarify the intention.
embedBitcodeEnabled means if any flag in -fembed-bitcode family is used so the
corresponding linker flag should be inferred. embedBitcodeInObject means if
the bitcode should be embedded in the object file which returns false for LTO.

mehdi_amini accepted this revision.Nov 15 2016, 9:17 PM
mehdi_amini edited edge metadata.

LGTM!

I feel the code is cleaner like that at uses of these API :)

This revision is now accepted and ready to land.Nov 15 2016, 9:17 PM
This revision was automatically updated to reflect the committed changes.