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.