[PATCH] D70798: [XCOFF][AIX] Emit TOC entries for object file generation

[Jason Liu via Phabricator via llvm-commits]

jasonliu marked 4 inline comments as done.
jasonliu added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:281
+           "Only an initialized csect can contain TC entry.");
+    assert(!TOCCsects.empty() &&
+           "We should at least have a TOC-base in this CsectGroup.");
----------------
daltenty wrote:
> A bit of a question: This assumes an ordering of CSects in the MCAssembler. This works because we always create the TOC base first. Is it OK to impose ordering requirements on the MCAssembler?
If we take AIX system assembler as the reference assembler, TOC-base has to come first even in assembly parsing. Otherwise, assembly code got rejected.
For example, 
I got this when .toc is not before .tc:

```
...
LC..0:
        .tc a[TC],a
        .toc
...

Assembler:
toc.s: line 10: 1252-018 Use a .tc inside a .toc scope only.
        Precede the .tc statements with a .toc statement.
```


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:247
+private:
+  bool Is64Bit;
+
----------------
DiggerLin wrote:
> I am prefer not to added a  new Is64Bit here.
> PPCTargetStreamer has a data member (MCStreamer). MCStreamer->getContext()->getAsmInfo()->getCodePointerSize(). 
Thanks. Good idea. 


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:254
   void emitTCEntry(const MCSymbol &S) override {
-    // Object writing TOC entries not supported yet.
+    unsigned PointerSize = Is64Bit ? 8 : 4;
+    Streamer.EmitValueToAlignment(PointerSize);
----------------
DiggerLin wrote:
> daltenty wrote:
> > Wouldn't a use of `MAI->getCodePointerSize()` be more appropriate here
> PPCTargetStreamer has a data member (MCStreamer). MCStreamer->getContext()->getAsmInfo()->getCodePointerSize(). 
> 
> Can we use getCodePointerSize() instead of Is64Bit？
Got it. 


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1842
       SectionKind::getData());
+  // TOC-base always have 0 size, but 4 byte alignment.
+  TOCBaseSection->setAlignment(Align(4));
----------------
daltenty wrote:
> nit: s/TOC-base/The TOC-base will/
I will add "The", but I don't think we need "will" here (we are stating something that's always the case, not something that's going to happen in the future). 


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

https://reviews.llvm.org/D70798