[PATCH] D68341: [AIX] TOC pseudo expansion for 64bit large + 64bit small + 32bit large modes

Tue Oct 8 14:15:08 PDT 2019

Xiangling_L marked 14 inline comments as done.
Xiangling_L added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:909
+
+    MCSymbol *MOSymbol = getMCSymbolForMO(MO, *this);

----------------
jasonliu wrote:
> nit: MOSymbol could move down a bit to be closer to where it get used. 
> 
> Also I would want to add const to this MOSymbol. But it seems that a lot of similar MOSymbols in this file do not have const with them, only add const for this one here would create inconsistency. Not sure if it's worth to have a NFC patch to add const for all the MOSymbol in this file first. 
As Sean mentioned to me, Stefan is doing this NFC patch about MOSymbol, so I think I can put this up there: https://github.ibm.com/compiler/llvm-project/pull/672.

================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:926
     // Into:      %xd = ADDIS8 %x2, sym at got@tlsgd at ha
-    assert(Subtarget->isPPC64() && "Not supported for 32-bit PowerPC");
+    assert(IsPPC64 && "Not supported for 32-bit PowerPC");
     const MachineOperand &MO = MI->getOperand(2);
----------------
hubert.reinterpretcast wrote:
> There is quite a bit of noise in this patch from these NFC changes. Please split them out.
Thanks for mentioning this, will do.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68341