This is an archive of the discontinued LLVM Phabricator instance.

[XCOFF][1/3] support fileHeader and sectionHeaders in 64-bit XCOFF.
ClosedPublic

Authored by Esme on Mar 2 2022, 5:54 PM.

Details

Summary

This is the first patch to enable the XCOFF64 object writer.
Currently only fileHeader and sectionHeaders are supported.
The second patch will be to support 64-bit section data and relocations, followed by the third patch to support 64-bit symbols.

Diff Detail

Event Timeline

Esme created this revision.Mar 2 2022, 5:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 5:54 PM
Esme requested review of this revision.Mar 2 2022, 5:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 5:54 PM
DiggerLin added inline comments.Mar 7 2022, 1:52 PM
llvm/lib/MC/XCOFFObjectWriter.cpp
765–766
if (!IsDwarf) {
  writeWord(Sec->Address); // s_paddr = s_vaddr
  writeWord(Sec->Address);
} else {
  writeWord(0);
  writeWord(0);
}

change to

writeWord(IsDwarf? Sec->Address:0);
writeWord(IsDwarf? Sec->Address:0);
767

from line 756 ~ 758 , all the field name are self_explain, we amy not need comments from them .

768

ditto

771

ditto for comment.

773

ditto for comment.

llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
14

I do not think there is anything to remind us to delete the ";; FIXME:..." in later patch and and add 64bit object file test.
maybe we can add writeSymbolEntry for 64 bit in the patch , and using

if (TargetObjectWriter->is64Bit())
   report_fatal_error("64-bit XCOFF object files are not supported yet.");

in the function.

and then you do not need to modify all the test cases.

DiggerLin added inline comments.Mar 7 2022, 2:02 PM
llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll
14

please ignore above comment

jhenderson added inline comments.Mar 8 2022, 12:29 AM
llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
15

Indent slightly to make it obvious this is a continuation of the line before.

Esme updated this revision to Diff 414605.Mar 11 2022, 12:33 AM

Addressed comments.

No further comments from me. @DiggerLin should be the one to finish off this review thouh.

DiggerLin added inline comments.Mar 14 2022, 2:17 PM
llvm/lib/MC/XCOFFObjectWriter.cpp
1046–1052

why there is no "+auxiliaryHeaderSize() "

Esme added inline comments.Mar 14 2022, 8:51 PM
llvm/lib/MC/XCOFFObjectWriter.cpp
1046–1052

As I understand, we don't need to support auxiliary header in ObjectWriter?

DiggerLin added inline comments.Mar 15 2022, 6:08 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
1046–1052

I think we need the support support auxiliary header later when we want to support visibility in symbol.

the function auxiliaryHeaderSize() only be used here. I think you can keep "+auxiliaryHeaderSize()" here. otherwise the definition of the function will be deleted too.

Esme updated this revision to Diff 415682.Mar 15 2022, 8:18 PM

Addressed @DiggerLin 's comments, ie. auxiliaryHeaderSize() is kept.

jhenderson added inline comments.Mar 16 2022, 1:07 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
297–298

Put the TODO comment either on a new line within the function (as suggested inline), or immediately before it as a commetnt, using // rather than block comments, then use clang-format to reformat it appropriately.

DiggerLin added inline comments.Mar 17 2022, 6:30 AM
llvm/lib/MC/XCOFFObjectWriter.cpp
316

change to

return (is64Bit()) ? W.write<uint64_t>(Word): W.write<uint32_t>(Word);

?

Esme updated this revision to Diff 416406.Mar 18 2022, 12:00 AM

Addressed comments.

DiggerLin accepted this revision.Mar 18 2022, 8:44 AM

LGTM.
I do not have further comment, but please wait for James approve.

llvm/lib/MC/XCOFFObjectWriter.cpp
297–298

James ask for put the comment within the function , not sure whether meet James comment. @jhenderson

This revision is now accepted and ready to land.Mar 18 2022, 8:44 AM
jhenderson accepted this revision.Mar 18 2022, 1:17 PM

LGTM.

llvm/lib/MC/XCOFFObjectWriter.cpp
297–298

I said either inside, or immediately before, so this is fine.

This revision was landed with ongoing or failed builds.Mar 20 2022, 6:32 AM
This revision was automatically updated to reflect the committed changes.