[PATCH] D70271: [AIX][XCOFF] Write Function descriptors and TOC base to data section

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:154
   CsectGroup DataCsects;
+  CsectGroup TOCCsects;
   CsectGroup BSSCsects;
----------------
Xiangling_L wrote:
> I am not quite sure, but my understanding is that we only have one TOC base csect in one translation unit? If I am correct, do we want to name this `TOCCsect` or add a comment to specify that point? It's a little confused to see plural here.
Seconded. Do you intend to map the toc entries into this csect group as well?


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:255
     report_fatal_error("Unhandled mapping of read-write csect to section.");
+  case XCOFF::XMC_DS:
+      return DataCsects;
----------------
DiggerLin wrote:
> jasonliu wrote:
> > jasonliu wrote:
> > > DiggerLin wrote:
> > > > DiggerLin wrote:
> > > > > I am not sure whether all function description data will be group together and than put in the .data section of xcoffobjectfile?  all the variable data and function description will be interleaved into data section.
> > > > > 
> > > > > As I remember, I read some document , it talk about that , we put all variables in the data section first. then function description,s final is TOC entries.   Maybe I wrong.
> > > > > 
> > > > I prefer added CsectGroup DSCsects; for function description csect. 
> > > Interleaving is acceptable for XCOFF format. What it hurts is readability of the symbol table. 
> > > But I think the current implementation would naturally put all the global datas first and then function descriptor later, because we always emit global data first and then process each function. 
> > I can add a separate CsectGroup to make the contract more explicit. 
> thanks Jason for clarification.
If we intend to map the toc entries into `TOCCsects` along side the toc base csect then its probably best to follow suit and have a separate csect group for the descriptors and make this ordering explicit. If not then we need to have lit test that explicitly check that descriptor data is mapped to after read/write data. It might be worthwhile to check said tests with `LLVM_REVERSE_ITERATION` enabled as well to see if that has any affect.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70271/new/

https://reviews.llvm.org/D70271