This is an archive of the discontinued LLVM Phabricator instance.

[LTO/gold] Support --wrap
ClosedPublic

Authored by tejohnson on Mar 7 2018, 4:37 PM.

Details

Summary

Utilize new gold plugin api interface for obtaining --wrap option
arguments, and LTO API handling (added for --wrap support in lld LTO),
to mark symbols so that LTO does not optimize them inappropriately.

Note the test cases will be in a new gold test subdirectory that
is dependent on the next release of gold which will contain the new
interfaces.

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson created this revision.Mar 7 2018, 4:37 PM
pcc accepted this revision.Mar 13 2018, 10:43 AM

LGTM

tools/gold/gold-plugin.cpp
581 ↗(On Diff #137510)

Seems a little odd to have this loop go in reverse. Unless there is some reason to do that, I'd write it in the conventional way.

This revision is now accepted and ready to land.Mar 13 2018, 10:43 AM
tejohnson updated this revision to Diff 138271.Mar 13 2018, 4:07 PM

Address comments

tejohnson marked an inline comment as done.Mar 13 2018, 4:08 PM
This revision was automatically updated to reflect the committed changes.

I had to revert as the new api is not available:
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/9109/steps/build-stage2-LLVMgold.so/logs/stdio

What is the best way to handle this? I see that there are a couple defines at the top of gold-plugin.cpp for other newer LDPT_* types, so I could add LDPT_GET_WRAP_SYMBOLS there.

For ld_plugin_get_wrap_symbols I guess I can add my own typedef at the top of gold-plugin.cpp?

pcc added a comment.Mar 13 2018, 5:11 PM

I had to revert as the new api is not available:
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/9109/steps/build-stage2-LLVMgold.so/logs/stdio

What is the best way to handle this? I see that there are a couple defines at the top of gold-plugin.cpp for other newer LDPT_* types, so I could add LDPT_GET_WRAP_SYMBOLS there.

For ld_plugin_get_wrap_symbols I guess I can add my own typedef at the top of gold-plugin.cpp?

Sounds good. Of course, the tv_get_wrap_symbols field will not be present so you will need to cast one of the other fields.

tejohnson reopened this revision.Mar 13 2018, 7:40 PM
In D44235#1036553, @pcc wrote:

I had to revert as the new api is not available:
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/9109/steps/build-stage2-LLVMgold.so/logs/stdio

What is the best way to handle this? I see that there are a couple defines at the top of gold-plugin.cpp for other newer LDPT_* types, so I could add LDPT_GET_WRAP_SYMBOLS there.

For ld_plugin_get_wrap_symbols I guess I can add my own typedef at the top of gold-plugin.cpp?

Sounds good. Of course, the tv_get_wrap_symbols field will not be present so you will need to cast one of the other fields.

Ugh, yeah that too. It's a little ugly, but I added FIXME comments in the various places to remove them when we require the binutils 2.31 (which would be the next version). I tested with both the old and new plugin-api.h to make sure it works with both now. I'll submit in the morning so that I can watch the bots.

This revision is now accepted and ready to land.Mar 13 2018, 7:40 PM
tejohnson updated this revision to Diff 138294.Mar 13 2018, 7:40 PM

Workarounds for old plugin-api.h

tejohnson updated this revision to Diff 138295.Mar 13 2018, 7:41 PM

Fix binutils version in FIXME

pcc accepted this revision.Mar 13 2018, 8:47 PM

LGTM

This revision was automatically updated to reflect the committed changes.