Address review comments

Author: Zaara Syeda

llvm/lib/Target/PowerPC/P10InstrResources.td

@@ -881,7 +881,7 @@ def : InstRW<[P10W_FX_3C, P10W_DISP_ANY],
 // 3 Cycles ALU operations, 1 input operands
 def : InstRW<[P10W_FX_3C, P10W_DISP_ANY, P10FX_Read],
       (instrs
-    ADDI, ADDI8, ADDIdtprelL32, ADDItlsldLADDR32, ADDItocL8, LI, LI8,
+    ADDI, ADDI8, ADDIdtprelL32, ADDItlsldLADDR32, ADDItocL, ADDItocL8, LI, LI8,
     ADDIC, ADDIC8,
     ADDIS, ADDIS8, ADDISdtprelHA32, ADDIStocHA, ADDIStocHA8, LIS, LIS8,
     ADDME, ADDME8,

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

@@ -1153,19 +1153,18 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
     // storage allocated in the TOC which contains the address of 'MOSymbol'.
     // If the toc-data attribute is used, the TOC entry contains the data
     // rather than the address of the MOSymbol.
-    auto isSymbolTD = [](const MachineOperand &MO) {
-      if (!MO.isGlobal())
-        return false;
+    if (![](const MachineOperand &MO) {
+          if (!MO.isGlobal())
+            return false;
 
-      const GlobalVariable *GV = dyn_cast<GlobalVariable>(MO.getGlobal());
-      if (!GV)
-        return false;
+          const GlobalVariable *GV = dyn_cast<GlobalVariable>(MO.getGlobal());
+          if (!GV)
+            return false;
 
-      return GV->hasAttribute("toc-data");
-    };
-
-    if (!isSymbolTD(MO))
+          return GV->hasAttribute("toc-data");
+        }(MO)) {
       MOSymbol = lookUpOrCreateTOCEntry(MOSymbol, getTOCEntryTypeForMO(MO), VK);
+    }
 
     const MCExpr *Exp = MCSymbolRefExpr::create(
         MOSymbol, MCSymbolRefExpr::VK_PPC_U, OutContext);
@@ -1285,49 +1284,33 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
-  case PPC::ADDItocL: {
+  case PPC::ADDItocL:
+  case PPC::ADDItocL8: {
     // Transform %xd = ADDItocL %xs, @sym
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this);
 
-    // Change the opcode to load address.
-    TmpInst.setOpcode(PPC::LA);
+    unsigned Op = MI->getOpcode();
 
-    const MachineOperand &MO = MI->getOperand(2);
-    assert(MO.isGlobal() && "Invalid operand for ADDItocL.");
+    // Change the opcode to load address for tocdata
+    TmpInst.setOpcode(Op == PPC::ADDItocL8 ? PPC::ADDI8 : PPC::LA);
 
-    LLVM_DEBUG(assert(
-        !(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
-        "Interposable definitions must use indirect accesses."));
+    const MachineOperand &MO = MI->getOperand(2);
+    if (Op == PPC::ADDItocL8)
+      assert((MO.isGlobal() || MO.isCPI()) && "Invalid operand for ADDItocL8.");
+    else
+      assert(MO.isGlobal() && "Invalid operand for ADDItocL.");
+    assert(!(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
+           "Interposable definitions must use indirect accesses.");
 
     // Map the operand to its corresponding MCSymbol.
     const MCSymbol *const MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
 
     const MCExpr *Exp = MCSymbolRefExpr::create(
-        MOSymbol, MCSymbolRefExpr::VK_PPC_L, OutContext);
-
-    TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
-    EmitToStreamer(*OutStreamer, TmpInst);
-    return;
-  }
-  case PPC::ADDItocL8: {
-    // Transform %xd = ADDItocL8 %xs, @sym
-    LowerPPCMachineInstrToMCInst(MI, TmpInst, *this);
-
-    // 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 ADDItocL8.");
-
-    LLVM_DEBUG(assert(
-        !(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
-        "Interposable definitions must use indirect accesses."));
+        MOSymbol,
+        Op == PPC::ADDItocL8 ? MCSymbolRefExpr::VK_PPC_TOC_LO
+                             : MCSymbolRefExpr::VK_PPC_L,
+        OutContext);
 
-    const MCExpr *Exp =
-        MCSymbolRefExpr::create(getMCSymbolForTOCPseudoMO(MO, *this),
-                                MCSymbolRefExpr::VK_PPC_TOC_LO, OutContext);
     TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;

llvm/lib/Target/PowerPC/PPCMacroFusion.def

@@ -32,7 +32,7 @@
 // {addi} followed by one of these {lxvd2x, lxvw4x, lxvdsx, lvebx, lvehx,
 // lvewx, lvx, lxsdx}
 FUSION_FEATURE(AddiLoad, hasAddiLoadFusion, 2, \
-               FUSION_OP_SET(ADDI, ADDI8, ADDItocL8), \
+               FUSION_OP_SET(ADDI, ADDI8, ADDItocL, ADDItocL8), \
                FUSION_OP_SET(LXVD2X, LXVW4X, LXVDSX, LVEBX, LVEHX, LVEWX, \
                              LVX, LXSDX))
 
@@ -134,13 +134,13 @@ FUSION_FEATURE(XorisXori, hasWideImmFusion, 1, FUSION_OP_SET(XORIS, XORIS8),
 
 // addis rx,ra,si - addi rt,rx,SI, SI >= 0
 FUSION_FEATURE(AddisAddi, hasWideImmFusion, 1,
-               FUSION_OP_SET(ADDIS, ADDIS8, ADDIStocHA8),
-               FUSION_OP_SET(ADDI, ADDI8, ADDItocL8))
+               FUSION_OP_SET(ADDIS, ADDIS8, ADDIStocHA8, ADDIStocHA),
+               FUSION_OP_SET(ADDI, ADDI8, ADDItocL8, ADDItocL))
 
 // addi rx,ra,si - addis rt,rx,SI, ra > 0, SI >= 2
 FUSION_FEATURE(AddiAddis, hasWideImmFusion, 1,
-               FUSION_OP_SET(ADDI, ADDI8, ADDItocL8),
-               FUSION_OP_SET(ADDIS, ADDIS8, ADDIStocHA8))
+               FUSION_OP_SET(ADDI, ADDI8, ADDItocL8, ADDItocL),
+               FUSION_OP_SET(ADDIS, ADDIS8, ADDIStocHA8, ADDIStocHA))
 
 // mtctr - { bcctr,bcctrl }
 FUSION_FEATURE(ZeroMoveCTR, hasZeroMoveFusion, -1,

llvm/test/CodeGen/PowerPC/toc-data.ll

@@ -57,6 +57,7 @@ define dso_local void @write_int(i32 signext %in) {
 ; CHECK32LARGE-NEXT: %[[SCRATCH2:[0-9]+]]:gprc_and_gprc_nor0 = ADDItocL killed %[[SCRATCH1]], @i
 ; CHECK32LARGE-NEXT: STW %{{[0-9]+}}, 0, killed %[[SCRATCH2]] :: (store (s32) into @i)
 
+; FIXME: peephole optimization opportunity for lower part relocation @l to the consuming stw
 ; TEST32LARGE:         .write_int:
 ; TEST32LARGE:          addis 4, i[TD]@u(2)
 ; TEST32LARGE-NEXT:	la 4, i[TD]@l(4)
@@ -127,6 +128,7 @@ define dso_local float @read_float() {
 ; CHECK32LARGE-NEXT: %[[SCRATCH2:[0-9]+]]:gprc_and_gprc_nor0 = ADDItocL killed %[[SCRATCH1]], @f
 ; CHECK32LARGE-NEXT: LFS 0, killed %[[SCRATCH2]] :: (dereferenceable load (s32) from @f)
 
+; FIXME: peephole optimization opportunity for lower part relocation @l to the consuming lfs
 ; TEST32LARGE:         .read_float:
 ; TEST32LARGE:          addis 3, f[TD]@u(2)
 ; TEST32LARGE-NEXT:	la 3, f[TD]@l(3)