[PATCH] D67125: [PowerPC][AIX] Adds support for writing the data section in object files
Digger via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 6 11:00:26 PDT 2019
DiggerLin added inline comments.
================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:478
+ // Write out the control section first and then each symbol in it.
+ writeSymbolTableEntryForControlSection(CSection, Data, XCOFF::C_HIDEXT);
+ for (const auto Sym : CSection.Syms) {
----------------
sfertile wrote:
> Storage class should come from CSection.
I have changed the interface of void writeSymbolTableEntryForSymbol(const Symbol &, const ControlSection &,
const int16_t, uint64_t);
void writeSymbolTableEntryForControlSection(const ControlSection &,
const int16_t,
XCOFF::StorageClass);
in the https://reviews.llvm.org/D66969. I think here also need to modification based on the new API.
================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:480
+ for (const auto Sym : CSection.Syms) {
+ writeSymbolTableEntryForSymbol(Sym, CSection, Data,
+ Layout.getSymbolOffset(*(Sym.MCSym)));
----------------
ditto
================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:537
+ Data.Index = SectionIndex++;
+ assert(alignTo(Address, DefaultSectionAlign) == Address &&
+ "Improperly aligned address for section.");
----------------
there is assert in the https://reviews.llvm.org/D66969 after for (auto &Csect : ProgramCodeCsects) {
}
maybe we can remove this one.
================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:539
+ "Improperly aligned address for section.");
+ uint32_t StartAddress = Address;
+ for (auto &Csect : DataCsects) {
----------------
jasonliu wrote:
> We assigned Index and Size to Data section. But we haven't assign Address to it. By default, the address would be 0. It's correct if you do not have a Text section. But once you have text section, the Data's address as 0 would wrong.
The Address value will been changed in the if (!ProgramCodeCsects.empty()) { }
if there is Text Section.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67125/new/
https://reviews.llvm.org/D67125
More information about the llvm-commits
mailing list