Details
- Reviewers
shchenz jsji DiggerLin jhenderson sfertile hubert.reinterpretcast - Group Reviewers
Restricted Project - Commits
- rG6143ec2961d1: [NFC][XCOFF] Refactor and format XCOFFObjectWriter.cpp.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
558–559 | do we still need is64Bit() ? | |
559–561 | " in 32-bit mode" will be deleted ? | |
652–658 | change writeSymbolBody to writeSymbolEntry ? | |
654 | do we want to set a default AuxCnt=1 ? | |
665–666 | delete the comment. | |
669 | change Name to writeSymbolAuxCsectEntry ? | |
671 | I am not sure which ( "x_scnlen" or "SectionOrLength")one is better for comment ? maybe the SectionOrLength from XCOFFObjectFile.h is better. | |
680 | function name change to writeSymbolAuxDwarfEntry ? | |
725–731 | since the header is for 32bit. W.write<uint32_t>(SymbolTableOffset); // f_symptr W.write<int32_t>(SymbolTableEntryCount); // f_nsyms we should modify the code | |
756–758 | I do not think we need the s_size here , Sec->Size can self-explain. |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
816–817 | we can use ".file" directly instead of StringRef(".file") |
Addressed @DiggerLin 's comments. Thanks
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
671 |
| |
671 | I would prefer the x_scnlen one because it is easier for the developer to refer to the documentation, and this comment style references ELFObjectWriter.cpp. | |
725–731 | Thanks! | |
756–758 | Removed // s_size and // s_flags, and kept other comments. |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
201 | change to |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
201 | sorry should be change to const uint64_t MaxRawDataSize = |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
274 | using writeSymbolAuxCsectEntry(uint64_t SecLen, uint8_t Type, uint8_t MapClass) is easy to understand the interface than writeSymbolAuxCsectEntry(uint64_t, uint8_t, uint8_t); | |
652 | uint64_t Value--> uint32_t Value | |
655 | maybe better to change the AuxCnt to NumberOfAuxEntries ? same as function | |
671 | the function looks only support 32bit AuxCsect now . "uint64_t SecLen" should be "uint32_t SecLen"? if we change to "uint32_t SecLen" W.write<uint32_t>(Lo_32(SecLen)); // x_scnlen also need to be changed. | |
682 | "uint64_t SecLen" should be "uint32_t SecLen" ? and "uint64_t RelCnt" should be "uint32_t RelCnt" | |
708 | change "DwarfSectionRef.getSymbolTableName(), 0, SectionIndex, 0," adding comment will make the value "0" easy to understand. | |
718 | add a comment for value 0, ditto in the following value zero or 1 |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
272–275 | Please provide the parameter names here. | |
683 | Default arguments are usually specified in the declaration, not definition. Same elsewhere. | |
690 | For consistency with other names. | |
725–731 | I'd have the assertion: in the future, you may not call this function from the same place. | |
812 | Ditto the line below. |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
699 | Here and similar comments elsewhere, the format is as suggested inline. |
llvm/lib/MC/XCOFFObjectWriter.cpp | ||
---|---|---|
657 | the writeSymbolName(SymbolName) can self explain . |
change to
constexpr uint64_t MaxRawDataSize =