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.
Details
- Reviewers
jsji shchenz jhenderson DiggerLin hubert.reinterpretcast sfertile - Group Reviewers
Restricted Project - Commits
- rGde20a3b67752: [XCOFF] support XCOFFObjectWriter for fileHeader and sectionHeaders in 64-bit…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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. 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. |
llvm/test/CodeGen/PowerPC/aix-available-externally-linkage.ll | ||
---|---|---|
14 | please ignore above comment |
llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll | ||
---|---|---|
15 | Indent slightly to make it obvious this is a continuation of the line before. |
No further comments from me. @DiggerLin should be the one to finish off this review thouh.
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
1046–1052 | why there is no "+auxiliaryHeaderSize() " |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
1046–1052 | As I understand, we don't need to support auxiliary header in ObjectWriter? |
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. |
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. |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
316 | change to return (is64Bit()) ? W.write<uint64_t>(Word): W.write<uint32_t>(Word); ? |
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 |
LGTM.
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
297–298 | I said either inside, or immediately before, so this is fine. |
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.