This is an archive of the discontinued LLVM Phabricator instance.

[gold] Handle modules that are not included in the link.
ClosedPublic

Authored by eugenis on Mar 3 2016, 2:01 PM.

Details

Summary

Gold has a newly added LDPT_GET_SYMBOLS_V3 callback that can
distinguish between a module that is not included in the link, and
one that is included but has its entire interface preempted by others.

Fixes PR26674.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 49775.Mar 3 2016, 2:01 PM
eugenis retitled this revision from to [gold] Handle modules that are not included in the link..
eugenis updated this object.
eugenis added a reviewer: rafael.
eugenis set the repository for this revision to rL LLVM.
eugenis added a subscriber: llvm-commits.

I'm not sure how to test this, other than with an explicit check for the gold version.

rafael added inline comments.
tools/gold/gold-plugin.cpp
58

you need a ifndef, no?

216

git-clang-format.

573

Can this ever happen?

eugenis updated this revision to Diff 49780.Mar 3 2016, 2:53 PM
eugenis marked an inline comment as done.
eugenis added inline comments.
tools/gold/gold-plugin.cpp
58

It's an enum, not a macro. The samewith LDPO_PIE above, we could remove the #ifdef - unless it's meant to be configurable with -DLDPO_PIE or something (but that seems unlikely).

rafael edited edge metadata.Mar 3 2016, 3:04 PM

This formatted more than what I was expecting.

I am OK with this with the ifdef part fixed. Just wait for Teresa to comment on getFunctionIndexForFile.

tools/gold/gold-plugin.cpp
58

They should be consistent. Se either remove the ifdef of LDPO_PIE or add one here.

eugenis updated this revision to Diff 49782.Mar 3 2016, 3:11 PM
eugenis edited edge metadata.

Removed ifndef around LDPO_PIE.
I've reformatted the whole function (git-clang-format did not work for me for some reason). It's probably better to have consistent formatting inside a function (if not across the whole file).

rafael accepted this revision.Mar 3 2016, 3:20 PM
rafael edited edge metadata.

LGTM, but please wait for Teresa to comment on there being any interaction between getFunctionIndexForFile and --start-lib/--end-lib.

This revision is now accepted and ready to land.Mar 3 2016, 3:20 PM
rafael added inline comments.Mar 3 2016, 3:21 PM
tools/gold/gold-plugin.cpp
919

ah, and git-clang-format this one :-)

tejohnson edited edge metadata.Mar 3 2016, 4:14 PM

LGTM, but please wait for Teresa to comment on there being any interaction between getFunctionIndexForFile and --start-lib/--end-lib.

LGTM too. I reviewed PR26674, and it seems correct to me to handle this the same in getFunctionIndexForFile as in getModuleForFile. If we aren't including the object file in the link we don't care about any of its summary info.

eugenis closed this revision.Mar 3 2016, 4:28 PM

Thanks, committed as r262676

eugenis marked an inline comment as done.Mar 3 2016, 4:28 PM