Page MenuHomePhabricator

[CodeGen] don't emit addrsig symbol if it's used only by metadata
ClosedPublic

Authored by zequanwu on Apr 28 2021, 9:42 PM.

Details

Summary

Value only used by metadata can be removed from .addrsig table.
This solves the undefined symbol error when enabling addrsig table on COFF LTO.

Diff Detail

Event Timeline

zequanwu created this revision.Apr 28 2021, 9:42 PM
zequanwu requested review of this revision.Apr 28 2021, 9:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 9:42 PM
rnk added inline comments.Apr 29 2021, 12:12 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1853

I see, so, the GV is used by a constant, which is then used by metadata. Metadata uses do not appear in the use list.

I think this condition is slightly incorrect, though, the bitcast constant expression could be used by metadata *and* a real Value. So, consider your test case, but then add in a real instruction that uses the same bitcast. In this case, we should still put GV into the addrsig table.

One refinement would be to check that each user is a Constant with no uses, instead of checking if it is used by metadata. This could be:

if (isa<Constant>(U) && U->use_empty())

However, this will not handle nested constant expressions. Consider a series of getelementptr and bitcast constant expressions. To be fully general, you need a worklist to check that all transitive users of GV are non-GlobalValue Constants that are only used by other constants.

zequanwu added inline comments.Apr 29 2021, 2:20 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1853

Yea, I see the problem.

if (isa<Constant>(U) && U->use_empty())

I understand this, but confused by below.

check that all transitive users of GV are non-GlobalValue Constants that are only used by other constants.

IIUC, if all transitive users of the GV are non-GlobalValue constants that are not transitively used by any non-GlobalValue constants, the GV is not address significant. For example, in the test case below, @a1 and @i1 are not address significant, right?

rnk added inline comments.Apr 29 2021, 2:34 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1853

In the test below, I see two Instructions that use @a1 and @i1, so I think they are address significant.

I mentioned GlobalValues because they are also subclasses of Constant, but if a GlobalVariable uses a Function, for example, that is a real use and we should consider the function address significant. Consider:

int someFunc(int x) { return x + 1; }
void *p = (void *)&someFunc;

The only user of someFunc will be a bitcast constant, and then the only use of that bitcast will be p.

zequanwu updated this revision to Diff 341650.Apr 29 2021, 2:44 PM

Address comment.

rnk accepted this revision.Apr 29 2021, 3:00 PM

Looks good, but please factor it out for readability.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1850

Please factor this out into its own function and give it some comments. I can suggest a name of hasTransitiveNonMetadataUse, or maybe just hasNonMetadataUse.

This revision is now accepted and ready to land.Apr 29 2021, 3:00 PM
zequanwu updated this revision to Diff 341681.Apr 29 2021, 3:38 PM

Refactor.

This revision was landed with ongoing or failed builds.Apr 29 2021, 3:40 PM
This revision was automatically updated to reflect the committed changes.