This is an archive of the discontinued LLVM Phabricator instance.

[XCOFF][AIX] Use 'L..' instead of 'L' for PrivateGlobalPrefix
ClosedPublic

Authored by jasonliu on May 29 2020, 12:12 PM.

Details

Summary

Without this change, names start with 'L' will get created as temporary symbol in MCContext::createSymbol.
Some other potential prefix considered:
.L, does not work for AIX, as a function start with L will end up with .L as prefix for its function entry point.
..L could work, but it does not play well with the convention on AIX that anything start with '.' are considered as entry point.
L. could work, but not sure if it's safe enough, as it's possible to have suffixes like .something append to a plain L, giving L.something which is not necessarily a temporary.
That's why we picked L.. for now.

Diff Detail

Event Timeline

jasonliu created this revision.May 29 2020, 12:12 PM
llvm/lib/MC/MCAsmInfoXCOFF.cpp
18

Since . gets appended for function entry points, .L is not all that "reserved". I am not sure that it makes sense to use .L except for inertia. It seems adding the dot after the L would work better?

llvm/lib/MC/MCAsmInfoXCOFF.cpp
18

*"prefixed to"

jasonliu updated this revision to Diff 267614.Jun 1 2020, 7:32 AM

Use L.. as prefix instead.

jasonliu retitled this revision from [XCOFF][AIX] Use '.L' instead of 'L' for PrivateGlobalPrefix to [XCOFF][AIX] Use 'L..' instead of 'L' for PrivateGlobalPrefix.Jun 1 2020, 7:35 AM
jasonliu edited the summary of this revision. (Show Details)
jasonliu edited the summary of this revision. (Show Details)Jun 1 2020, 7:44 AM
daltenty accepted this revision.Jun 1 2020, 2:37 PM

LGTM

This revision is now accepted and ready to land.Jun 1 2020, 2:37 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)Jun 1 2020, 3:37 PM
llvm/test/CodeGen/PowerPC/aix-lower-block-address.ll
72

What exactly is a "temporary" in terms of the object file or the assembly? The TOC entry here clearly would have a symbol table entry.

llvm/test/CodeGen/PowerPC/aix-xcoff-externL.ll
2

s/symbol name starts/a symbol name that starts/;

4

s/resulted/resulting/;

jasonliu marked an inline comment as done.Jun 1 2020, 5:32 PM
jasonliu added inline comments.
llvm/test/CodeGen/PowerPC/aix-lower-block-address.ll
72

I think LLVM treat the "temporary" as compiler created symbol.
We have the logic in XCOFFObjectFileWriter.cpp to query and skip recording the temporary symbols.
In this particular case, L..tmp0 is a temporary symbol/XCOFF label that we do not want to record in our symbol table. L..tmp0[TC] gets treated as temporary symbol as well, and skipped, but its csect still get registered and recorded. So everything "works".
I do agree it's a bit confusing here. But for our current status on AIX, symbols' "temporary" status does not seem to affect any of the assembly generation. And we actually only care about the symbols's temporary status for external symbols and labels, because those info are coming through MCSymbol (others are from MCSection).

jasonliu marked an inline comment as done.Jun 1 2020, 5:39 PM
jasonliu added inline comments.
llvm/test/CodeGen/PowerPC/aix-lower-block-address.ll
72

In terms of object file generation we actually only care about the correctness of symbols's temporary status for external symbols and labels, because those info are coming through MCSymbol (others are from MCSection) .

llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
181–182

Maybe as a separate patch, should we update the test cases to use a different "mangling mode" with a matching private global prefix?

jasonliu marked an inline comment as done.Jun 2 2020, 6:47 AM
jasonliu added inline comments.
llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
181–182

Yes, we should make it consistent in a separate patch.

jasonliu updated this revision to Diff 267872.Jun 2 2020, 6:53 AM
jasonliu marked 2 inline comments as done.

Modify comments in test case.

This revision was automatically updated to reflect the committed changes.