[PATCH] D70461: [AIX] Emit TOC entries for ASM printing

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/include/llvm/MC/MCSymbolXCOFF.h:56
+    if (name.back() == ']') {
+      assert(name[name.size() - 4] == '[' &&
+             "Invalid SMC format in XCOFF symbol.");
----------------
I think we have an exception here  ->  TOC[TC0]


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+  for (auto &I : TOC) {
+    OutStreamer->EmitLabel(I.second);
+    TS.emitTCEntry(*I.first);
----------------
TC entry itself is a csect. Do we need to create that csect(MCSectionXCOFF) and do a SwitchSection for every TC entry?
Otherwise, I will need some help on understanding how to implement object file generation for TC entry. 


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1858
+    if (GV) {
+      ValidateGV(GV);
+      if (GV->hasInitializer()) {
----------------
If we are going to call EmitGlobalVariable for every GV, do we need to ValidateGV twice here? Or are there situations that we would call getMCSymbolForTOCPseudoMO for a particular GV that is not going to be checked in EmitGlobalVariable?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1888
+  }
+  return PPCAsmPrinter::getMCSymbolForTOCPseudoMO(MO);
+}
----------------
Nit: Could we further reduce the nestness of this function by using early returns?


```
if (MO.getType() != MachineOperand::MO_GlobalAddress)
  return PPCAsmPrinter::getMCSymbolForTOCPseudoMO(MO);
```



================
Comment at: llvm/lib/Target/PowerPC/PPCMCInstLower.cpp:67
 
+  const MachineFunction *MF = MO.getParent()->getParent()->getParent();
+  const PPCSubtarget *Subtarget = &(MF->getSubtarget<PPCSubtarget>());
----------------
A thought here:
We are going to see this again in getMCSymbolForTOCPseudoMO anyway. Would it be better if we move the logic here to the getMCSymbolForTOCPseudoMO?
We would be able to combine the logic with undefined symbol there, and the only difference is that if we should put XCOFF::XMC_DS or XCOFF::XMC_UA into the MCSection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70461