[NFC][PowerPC]Clean up PPCAsmPrinter for TOC related pseudo opcode

[Xiangling_L]

Summary

Add a helper function getMCSymbolForTOCPseudoMO to clean up PPCAsmPrinter a little bit.

[jsji]

LGTM. Some nit comments.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

741

MOSymbol used only once here, maybe inline the call directly?

761

large code model only for CPI?

774

Are these clang formatted? Formatting looks weird to me.

797

large code model only for CPI?

835

MOSymbol used only once here, maybe inline the call directly?

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

774

Yes, they are.

[Xiangling_L]

Address commments & add const for lookUpOrCreateTOCEntry & TOC MapVector<const MCSymbol *, MCSymbol *>

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

517

Has a switch on getType been considered?

685

Is it also possible to use * const?

737

There's an extra space in the indentation here.

760

Please don't remove the Oxford comma before the "or".

771

Can this be const?

774

This does not seem to be an NFC change.

797

Comma before "then".

798

Comma after "Otherwise".

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

836

Missing the invocation of assert here?

1378

Has a range-based for loop been considered?

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

517

Sure.

774

Since Machine Operand for this TOC pseudo opcode can only be MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress(), so the line`(MO.isCPI() && TM.getCodeModel() == CodeModel::Large) will have the same logic with or without MO.isCPI(). But since people @jsji have some slight preference to make it clear, I add MO.isCPI()`. Please let me know if you still have any concerns about this change.

836

Thank you for mentioning this mistake.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

774

I do not believe that the statement is sufficiently supported in this context except if there is:

assert(GlobalToc || !MO.isGlobal());

[Xiangling_L]

Address 2nd round comments

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

774

I think I kinda know your concern, do you mean without my changes, for query if (GlobalToc || MO.isJTI() || MO.isBlockAddress() || TM.getCodeModel() == CodeModel::Large), there is the possibility that GlobalToc is false, but the code model is large, under which we still generate TOCEntry for global MO? If my understanding is correct, then I think we don't need to worry about this situation, because according to the definition of isGVIndirectSymbol:

bool PPCSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
  // Large code model always uses the TOC even for local symbols.
  if (TM.getCodeModel() == CodeModel::Large)
    return true;
  if (TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
    return false;
  return true;
}

When it's large code model, and MO.isGlobal() == true, GlobalToc is definitely true, so there is no situation that when MO.isGlobal() &&GlobalToc == false, but the code model is not large.

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

774

typo: not large --> large

[hubert.reinterpretcast]

LGTM with some straightforward changes (can be fixed on the commit).

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

685

My fault on the previous comment; please adjust the space: const MCSymbol *const MOSymbol.

774

Okay, thanks.

1381

I prefer to use meaningful names:

for (const auto &TOCMapPair : TOC) {
  const MCSymbol *const TOCEntryTarget = TOCMapPair.first;
  MCSymbol *const TOCEntryLabel = TOCMapPair.second;

  OutStreamer->EmitLabel(TOCEntryLabel);
  if (isPPC64) {
    TS.emitTCEntry(*TOCEntryTarget);
  } else {
    OutStreamer->EmitValueToAlignment(4);
    OutStreamer->EmitSymbolValue(TOCEntryTarget, 4)
  }
}

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

515

s/the machine operand/a machine operand for a TOC pseudo-machine instruction/;

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCAsmPrinter::EmitTlsCall(const MachineInstr *MI,
   const MCExpr *SymVar = MCSymbolRefExpr::create(MOSymbol, VK, OutContext);
   EmitToStreamer(*OutStreamer,
                  MCInstBuilder(Subtarget->isPPC64() ?
                                PPC::BL8_NOP_TLS : PPC::BL_TLS)
                  .addExpr(TlsRef)
                  .addExpr(SymVar));
 }
 
+// Map the machine operand to its corresponding MCSymbol.
+static MCSymbol *getMCSymbolForTOCPseudoMO(const MachineOperand &MO, AsmPrinter &AP) {
+  if (MO.isGlobal())
+    return AP.getSymbol(MO.getGlobal());
+  if (MO.isCPI())
+    return AP.GetCPISymbol(MO.getIndex());
+  if (MO.isJTI())
+    return AP.GetJTISymbol(MO.getIndex());
+  if (MO.isBlockAddress())
+    return AP.GetBlockAddressSymbol(MO.getBlockAddress());
+
+  llvm_unreachable("Unsupported machine operand type.");
+}
+
 /// EmitInstruction -- Print out a single PowerPC MI in Darwin syntax to
 /// the current output stream.
 ///
 void PPCAsmPrinter::EmitInstruction(const MachineInstr *MI) {
   MCInst TmpInst;
   const bool IsDarwin = TM.getTargetTriple().isOSDarwin();
   const Module *M = MF->getFunction().getParent();
   PICLevel::Level PL = M->getPICLevel();
case PPC::LWZtoc: {
     // Change the opcode to LWZ.
     TmpInst.setOpcode(PPC::LWZ);
 
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Unexpected operand type for LWZtoc pseudo.");
 
     // Map the operand to its corresponding MCSymbol.
-    MCSymbol *MOSymbol = nullptr;
-    if (MO.isGlobal())
-      MOSymbol = getSymbol(MO.getGlobal());
-    else if (MO.isCPI())
-      MOSymbol = GetCPISymbol(MO.getIndex());
-    else if (MO.isJTI())
-      MOSymbol = GetJTISymbol(MO.getIndex());
-    else if (MO.isBlockAddress())
-      MOSymbol = GetBlockAddressSymbol(MO.getBlockAddress());
-
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
     const bool IsAIX = TM.getTargetTriple().isOSAIX();
 
     // Create a reference to the GOT entry for the symbol. The GOT entry will be
     // synthesized later.
     if (PL == PICLevel::SmallPIC && !IsAIX) {
       const MCExpr *Exp =
         MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_GOT,
                                 OutContext);
#endif
   }
   case PPC::LDtocJTI:
   case PPC::LDtocCPT:
   case PPC::LDtocBA:
   case PPC::LDtoc: {
     // Transform %x3 = LDtoc @min1, %x2
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, IsDarwin);
 
-    // Change the opcode to LD, and the global address operand to be a
-    // reference to the TOC entry we will synthesize later.
+    // Change the opcode to LD.
     TmpInst.setOpcode(PPC::LD);
-    const MachineOperand &MO = MI->getOperand(1);
 
-    // Map symbol -> label of TOC entry
+    const MachineOperand &MO = MI->getOperand(1);
     assert(MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress());
-    MCSymbol *MOSymbol = nullptr;
-    if (MO.isGlobal())
-      MOSymbol = getSymbol(MO.getGlobal());
-    else if (MO.isCPI())
-      MOSymbol = GetCPISymbol(MO.getIndex());
-    else if (MO.isJTI())
-      MOSymbol = GetJTISymbol(MO.getIndex());
-    else if (MO.isBlockAddress())
-      MOSymbol = GetBlockAddressSymbol(MO.getBlockAddress());
 
+    // Map the machine operand to its corresponding MCSymbol.
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
+    // Map the global address operand to be a reference to the TOC entry we
+    // will synthesize later.
     MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
 
     const MCExpr *Exp =
       MCSymbolRefExpr::create(TOCEntry, MCSymbolRefExpr::VK_PPC_TOC,
                               OutContext);
     TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
 
   case PPC::ADDIStocHA8: {
     // Transform %xd = ADDIStocHA8 %x2, @sym
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, IsDarwin);
 
-    // Change the opcode to ADDIS8.  If the global address is external, has
-    // common linkage, is a non-local function address, or is a jump table
-    // address, then generate a TOC entry and reference that.  Otherwise
-    // reference the symbol directly.
+    // Change the opcode to ADDIS8. If the global address is the address of
+    // an external symbol, is a jump table address, or is a block address; or if
+    // the large code model is enabled then generate a TOC entry and reference
+    // that. Otherwise, reference the symbol directly.
     TmpInst.setOpcode(PPC::ADDIS8);
+
     const MachineOperand &MO = MI->getOperand(2);
-    assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() ||
-            MO.isBlockAddress()) &&
+    assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand for ADDIStocHA8!");
-    MCSymbol *MOSymbol = nullptr;
-    bool GlobalToc = false;
 
-    if (MO.isGlobal()) {
-      const GlobalValue *GV = MO.getGlobal();
-      MOSymbol = getSymbol(GV);
-      GlobalToc = Subtarget->isGVIndirectSymbol(GV);
-    } else if (MO.isCPI()) {
-      MOSymbol = GetCPISymbol(MO.getIndex());
-    } else if (MO.isJTI()) {
-      MOSymbol = GetJTISymbol(MO.getIndex());
-    } else if (MO.isBlockAddress()) {
-      MOSymbol = GetBlockAddressSymbol(MO.getBlockAddress());
-    }
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
+    bool GlobalToc = 
+  MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal());
     if (GlobalToc || MO.isJTI() || MO.isBlockAddress() ||
-        TM.getCodeModel() == CodeModel::Large)
+  (MO.isCPI() && TM.getCodeModel() == CodeModel::Large))
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
 
     const MCExpr *Exp =
       MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_PPC_TOC_HA,
                               OutContext);
 
     if (!MO.isJTI() && MO.getOffset())
       Exp = MCBinaryExpr::createAdd(Exp,
                                     MCConstantExpr::create(MO.getOffset(),
                                                            OutContext),
                                     OutContext);
 
     TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
   case PPC::LDtocL: {
     // Transform %xd = LDtocL @sym, %xs
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, IsDarwin);
 
-    // Change the opcode to LD.  If the global address is external, has
-    // common linkage, or is a jump table address, then reference the
-    // associated TOC entry.  Otherwise reference the symbol directly.
+    // Change the opcode to LD.  If the global address is the address of
+    // an external symbol, is a jump table address, is a block address; or if
+    // large code model is enabled then generate a TOC entry and reference that.
+    // Otherwise reference the symbol directly.
     TmpInst.setOpcode(PPC::LD);
+
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() ||
             MO.isBlockAddress()) &&
            "Invalid operand for LDtocL!");
-    MCSymbol *MOSymbol = nullptr;
 
-    if (MO.isJTI())
-      MOSymbol = lookUpOrCreateTOCEntry(GetJTISymbol(MO.getIndex()));
-    else if (MO.isBlockAddress()) {
-      MOSymbol = GetBlockAddressSymbol(MO.getBlockAddress());
-      MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
-    }
-    else if (MO.isCPI()) {
-      MOSymbol = GetCPISymbol(MO.getIndex());
-      if (TM.getCodeModel() == CodeModel::Large)
-        MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
-    }
-    else if (MO.isGlobal()) {
-      const GlobalValue *GV = MO.getGlobal();
-      MOSymbol = getSymbol(GV);
-      LLVM_DEBUG(
-          assert((Subtarget->isGVIndirectSymbol(GV)) &&
-                 "LDtocL used on symbol that could be accessed directly is "
-                 "invalid. Must match ADDIStocHA8."));
+    LLVM_DEBUG(assert(
+        (!MO.isGlobal() || Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
+
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
+    if (!MO.isCPI() || TM.getCodeModel() == CodeModel::Large)
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol);
-    }
 
     const MCExpr *Exp =
       MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_PPC_TOC_LO,
                               OutContext);
     TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
   case PPC::ADDItocL: {
     // Transform %xd = ADDItocL %xs, @sym
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, IsDarwin);
 
-    // Change the opcode to ADDI8.  If the global address is external, then
-    // generate a TOC entry and reference that.  Otherwise reference the
+    // Change the opcode to ADDI8. If the global address is external, then
+    // generate a TOC entry and reference that. Otherwise, reference the
     // symbol directly.
     TmpInst.setOpcode(PPC::ADDI8);
+
     const MachineOperand &MO = MI->getOperand(2);
-    assert((MO.isGlobal() || MO.isCPI()) && "Invalid operand for ADDItocL");
-    MCSymbol *MOSymbol = nullptr;
+    assert((MO.isGlobal() || MO.isCPI()) && "Invalid operand for ADDItocL.");
 
-    if (MO.isGlobal()) {
-      const GlobalValue *GV = MO.getGlobal();
-      LLVM_DEBUG(assert(!(Subtarget->isGVIndirectSymbol(GV)) &&
-                        "Interposable definitions must use indirect access."));
-      MOSymbol = getSymbol(GV);
-    } else if (MO.isCPI()) {
-      MOSymbol = GetCPISymbol(MO.getIndex());
-    }
+    const MCSymbol *MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
+    LLVM_DEBUG(!(MO.isGlobal() && Subtarget->isGVIndirectSymbol(GV)) &&
+               "Interposable definitions must use indirect access.");
 
     const MCExpr *Exp =
       MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_PPC_TOC_LO,
                               OutContext);
     TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
if (!TOC.empty()) {
     if (isPPC64)
       Section = OutStreamer->getContext().getELFSection(
           ".toc", ELF::SHT_PROGBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
         else
           Section = OutStreamer->getContext().getELFSection(
               ".got2", ELF::SHT_PROGBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
     OutStreamer->SwitchSection(Section);
 
     for (MapVector<MCSymbol*, MCSymbol*>::iterator I = TOC.begin(),
          E = TOC.end(); I != E; ++I) {
       OutStreamer->EmitLabel(I->second);
       MCSymbol *S = I->first;
       if (isPPC64) {
         TS.emitTCEntry(*S);
       } else {
         OutStreamer->EmitValueToAlignment(4);
         OutStreamer->EmitSymbolValue(S, 4);
       }
     }
   }