This is an archive of the discontinued LLVM Phabricator instance.

[XCOFF][AIX] Support relocation generation for large code model
ClosedPublic

Authored by jasonliu on Jul 24 2020, 12:59 PM.

Details

Summary

Support TOCU and TOCL relocation type for object file generation.

Diff Detail

Event Timeline

jasonliu created this revision.Jul 24 2020, 12:59 PM
jasonliu edited the summary of this revision. (Show Details)Jul 24 2020, 1:02 PM
jasonliu added inline comments.Aug 6 2020, 12:24 PM
llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-large.ll
75

Notice when D85455 lands, this would be a[TE] instead of a[TC].
Depending on which patch lands first, and I will rebase the other dependant patch (as it's a simple rebasing and shouldn't affect the review too much).

jasonliu updated this revision to Diff 284495.Aug 10 2020, 1:44 PM

Rebase on lastest master to resolve potential merge conflict.

DiggerLin added inline comments.Aug 12 2020, 10:53 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
438

I just wonder why there is no Type == XCOFF::RelocationType::R_TOCU ? we do not need to applyFixup for the instruction ?
addis 3, L..C0@u(2) ?

440–445

I am not sure whether I understand is correct or not ?
FixedValue of intructions
addis 3, L..C0@u(2) . FixedValue should get the high 16bits of SectionMap[SymASec]->Address - TOCCsects.front().Address;

lwz 3, L..C0@l(3) FixedValue should get the low 16bits of SectionMap[SymASec]->Address - TOCCsects.front().Address;

if I understand correct , the code need to change.

jasonliu marked 2 inline comments as done.Aug 12 2020, 11:06 AM
jasonliu added inline comments.
llvm/lib/MC/XCOFFObjectWriter.cpp
440–445

Could you give an example that would generated non-0 FixedValue in TOCU position?
The example (the test case I have below) I tried before, TOCU position always have 0 as its FixedValue with system assembler generated object.

jasonliu marked an inline comment as done.Aug 12 2020, 11:06 AM
DiggerLin added inline comments.Aug 17 2020, 6:14 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
440–445
1ae98: 3c 62 00 01                   addis 3, 2, 1
                        0001ae9a:  R_TOCU       (idx: 36364) a10995[TE]
   1ae9c: 80 63 8d f8                   lwz 3, -29192(3)
                        0001ae9e:  R_TOCL       (idx: 36364) a10995[TE]
   1aea0: 90 83 00 00                   stw 4, 0(3)
   1aea4: 3c 62 00 01                   addis 3, 2, 1
                        0001aea6:  R_TOCU       (idx: 36366) a10996[TE]
   1aea8: 80 63 8d fc                   lwz 3, -29188(3)
                        0001aeaa:  R_TOCL       (idx: 36366) a10996[TE]
   1aeac: 90 83 00 00                   stw 4, 0(3)
   1aeb0: 3c 62 00 01                   addis 3, 2, 1
jasonliu added inline comments.Aug 18 2020, 8:29 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
440–445

It seems the system as would generate a non-0 FixedValue for you when your TOC region is larger than 64KB.
But xlclang (which do not use system as to generate object) would always generate 0 for the TOCU region, and linker would know how to change the fix-up '0' to the proper value that points to the correct TOC entry.

jasonliu updated this revision to Diff 286844.Aug 20 2020, 9:55 AM

Report fatal error when we know the initial 64kb TOC region is not enough to contain all the TOC entries.

jasonliu added inline comments.Aug 20 2020, 10:02 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
440–445

Some update on this:
I talked with people from AIX OS team, and it seems indeed what we put in the TOCU's fix-up region does not matter for the linker.
But aside from that, I think our current logic for TOCL region might not work well when we overflow the initial TOC region, because we might need to wrap around the value in that case. For example, we would need to put 0 in TOCL region when it's the first TOC entry in the second TOC region.
And since we are not able to produce that case yet (we have a report_fatal_error somewhere else that would get triggered before this one because we haven't figured out a plan on how to deal with "minus" offset TOC entries in small code model mode.), I will report a fatal error here as well to remind us when we run into this issue in the future.

DiggerLin accepted this revision.Aug 24 2020, 11:58 AM

LGTM

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-large.ll
2

minor: do we need -verify-machineinstrs here ?

This revision is now accepted and ready to land.Aug 24 2020, 11:58 AM
jasonliu added inline comments.Aug 25 2020, 7:47 AM
llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-large.ll
2

I think it's good to always verify if the machine instruction generated during the obj gen process are valid.
It doesn't really hurt to add it in.