[PowerPC] Fold redundant load immediates of zero and delete if possible

[kamaub]

Summary

This patch folds redundant load immediates into a zero for instructions
which recognise this as the value zero and not the register. If the load
immediate is no longer in use it is then deleted.

This is already done in earlier passes but the ppc-mi-peephole allows for
a more general implementation.

[NeHuang]

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1403

nit: typo - "delete the caller"

1415

Is this function defined for load immediate 0 case only? Better to add it into function comments.

llvm/test/CodeGen/PowerPC/fold-remove-li.ll

4

Any reason not adding a big endian test here? : )

[amyk]

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1402

End with period.

1407

Is MRI needed in this function/is it being used?

1428

Can we add a comment here describing the following?

1463

nit: Add a space here maybe for better readability.

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

321

if it is not needed

323

Correct me if I am wrong, but isn't this being checked inside FoldImmediateWithoutDelete?

if (!DefMI.getOperand(1).isImm())
   return false;
 if (DefMI.getOperand(1).getImm() != 0)
   return false;

Do we need to check this in both places?

[nemanjai]

Please refactor the common logic out and address the comments.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1405

We shouldn't do this. We now have two nearly identical implementations of this function. One deletes the def the other one does not. That is the only difference as far as I can tell.

Please refactor the common logic into a single function to form something like this:

bool PPCInstrInfo::onlyFoldImm(...) { 
  // All the folding logic.
}
bool PPCInstrInfo::FoldImmediate(...) {
  bool Changed = onlyFoldImm(...);
  if (Changed && MRI->use_nodbg_empty(Reg))
    // Delete the def
  return Changed;
}

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

319

This run-on sentence is hard to follow. Break it up a bit please. Perhaps:

// If we are materializing a zero, look for any use operands for which zero
// means immediate zero. All such operands can be replaced with PPC::ZERO.

323

We do want to keep it here as well because there may be many users. It is pointless to go through all of them if the materialized value is non-zero. But please do combine these into a single condition:

if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != 0)
  break;

329

Let's not check this at each iteration. We can just check for no remaining uses after the loop.
Then the body of the loop becomes a single line:

Simplified |=
  TII->FoldImmediateWithoutDelete(UseMI, MI, MIDestReg, MRI);

337

Here, we can check for no remaining uses of MIDestReg and mark MI for deletion if so.

[kamaub]

• Refractoring this patch to removed duplicated function

[stefanp]

Mostly minor comments and a couple of questions.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1398

I know that this is how it was done in the past but how about just checking:

MRI->use_nodbg_empty(Reg)

Or maybe there was a good reason to have done it this way in the past and I can't see it. :)

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

329

Why do you need to check && Folded here? The output register of this LI instruction has no non-debug uses so you can probably just schedule it to be deleted anyway. If you do that you also don't need to keep Folded around and just do what Nemanja mentioned:

Simplified |=
  TII->FoldImmediateWithoutDelete(UseMI, MI, MIDestReg, MRI);

llvm/test/CodeGen/PowerPC/fold-remove-li.ll

6

nit:
Add -mcpu to these tests. If you don't I think that the default for LE is pwr8 and the default for BE is pwr4 (?). Anyway, I think you will get a more reliable test if you specify the cpu.

llvm/test/CodeGen/PowerPC/save-crbp-ppc32svr4.ll

17

Question:
Why do we lose a store here? Is it because we remove an LI instruction and then that reduces register pressure?

[nemanjai]

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

329

In fact, as implemented, Folded will be set to whatever TII->onlyFoldImmediate() returns for the last use which may be false even if you have folded it in some previous uses. This needs to be fixed.

[kamaub]

Thank you, comments were addressing in most recent commit.

[kamaub]

• Changing some condition and fixing test cases.

This change edits some conditions to be more robust in their logic.
Of note is the change to clean up load immediates even if they weren't
folded, a side-effect of this is that one test case needs to fixed up.

[kamaub]

All comments have been address, thank you.

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

329

That right, I shouldn't used Folded |= instead, I've gone with your (earlier suggestion however, thank you.

llvm/test/CodeGen/PowerPC/save-crbp-ppc32svr4.ll

17

Yes, an Li is removed which does reduce register pressure but that isn't why this test cases changes. r28 is spilled because zero is loaded into it after the prologue but, with this patch, it is folded into and addi and removed so the stack is reduced from 28 -> 24

[stefanp]

LGTM

[nemanjai]

LGTM except for formatting nits. Thanks for addressing the comments.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

1328

Similar issue with alignment.

llvm/lib/Target/PowerPC/PPCInstrInfo.h

311

Minor nit: alignment of the last parameter is off, please fix on the commit.

[amyk]

llvm/test/CodeGen/PowerPC/fold-remove-li.ll

4

Minor nit: indentation here and on line 6.

llvm/lib/Target/PowerPC/PPCInstrInfo.h

unsigned getLoadOpcodeForSpill(unsigned Reg,
                                  const TargetRegisterClass *RC = nullptr) const;
 
   bool
   reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;
 
   bool FoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI, unsigned Reg,
                      MachineRegisterInfo *MRI) const override;
 
+  bool onlyFoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI,
+                                     unsigned Reg) const;
+
   // If conversion by predication (only supported by some branch instructions).
   // All of the profitability checks always return true; it is always
   // profitable to use the predicated branches.
   bool isProfitableToIfCvt(MachineBasicBlock &MBB,
                           unsigned NumCycles, unsigned ExtraPredCycles,
                           BranchProbability Probability) const override {
     return true;
   }

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const {
   if (Cond[1].getReg() == PPC::CTR8 || Cond[1].getReg() == PPC::CTR)
     Cond[0].setImm(Cond[0].getImm() == 0 ? 1 : 0);
   else
     // Leave the CR# the same, but invert the condition.
     Cond[0].setImm(PPC::InvertPredicate((PPC::Predicate)Cond[0].getImm()));
   return false;
 }
 
-bool PPCInstrInfo::FoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI,
-                                 unsigned Reg, MachineRegisterInfo *MRI) const {
-  // For some instructions, it is legal to fold ZERO into the RA register field.
+// This function performs that fold by replacing the operand with PPC::ZERO,
+// it does not consider whether the load immediate zero is no longer in use.
+bool PPCInstrInfo::onlyFoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI,
+                               unsigned Reg) const {
   // A zero immediate should always be loaded with a single li.
   unsigned DefOpc = DefMI.getOpcode();
   if (DefOpc != PPC::LI && DefOpc != PPC::LI8)
     return false;
   if (!DefMI.getOperand(1).isImm())
     return false;
   if (DefMI.getOperand(1).getImm() != 0)
     return false;
 
   // Note that we cannot here invert the arguments of an isel in order to fold
   // a ZERO into what is presented as the second argument. All we have here
   // is the condition bit, and that might come from a CR-logical bit operation.
 
   const MCInstrDesc &UseMCID = UseMI.getDesc();
 
   // Only fold into real machine instructions.
   if (UseMCID.isPseudo())
     return false;
 
+  // We need to find which of the User's operands is to be folded, that will be
+  // the operand that matches the given register ID.
   unsigned UseIdx;
   for (UseIdx = 0; UseIdx < UseMI.getNumOperands(); ++UseIdx)
     if (UseMI.getOperand(UseIdx).isReg() &&
         UseMI.getOperand(UseIdx).getReg() == Reg)
       break;
 
   assert(UseIdx < UseMI.getNumOperands() && "Cannot find Reg in UseMI");
   assert(UseIdx < UseMCID.getNumOperands() && "No operand description for Reg");
bool PPCInstrInfo::onlyFoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI,
   unsigned ZeroReg;
   if (UseInfo->isLookupPtrRegClass()) {
     bool isPPC64 = Subtarget.isPPC64();
     ZeroReg = isPPC64 ? PPC::ZERO8 : PPC::ZERO;
   } else {
     ZeroReg = UseInfo->RegClass == PPC::G8RC_NOX0RegClassID ?
               PPC::ZERO8 : PPC::ZERO;
   }
   
-  bool DeleteDef = MRI->hasOneNonDBGUse(Reg);
   UseMI.getOperand(UseIdx).setReg(ZeroReg);
+  return true;
+}
 
-  if (DeleteDef)
+// Folds zero into instructions which have a load immediate zero as an operand
+// but also recognize zero as immediate zero. If the definition of the load
+// has no more users it is deleted.
+bool PPCInstrInfo::FoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI,
+                                 unsigned Reg, MachineRegisterInfo *MRI) const {
+  bool Changed = onlyFoldImmediate(UseMI, DefMI, Reg);
+  if (MRI->use_nodbg_empty(Reg))
     DefMI.eraseFromParent();
-
-  return true;
+  return Changed;
 }
 
 static bool MBBDefinesCTR(MachineBasicBlock &MBB) {
   for (MachineBasicBlock::iterator I = MBB.begin(), IE = MBB.end();
        I != IE; ++I)
     if (I->definesRegister(PPC::CTR) || I->definesRegister(PPC::CTR8))
       return true;
   return false;
 }
 
 // We should make sure that, if we're going to predicate both sides of a
 // condition (a diamond), that both sides don't define the counter register. We
 // can predicate counter-decrement-based branches, but while that predicates
 // the branching, it does not predicate the counter decrement. If we tried to
 // merge the triangle into one predicated block, we'd decrement the counter
 // twice.
 bool PPCInstrInfo::isProfitableToIfCvt(MachineBasicBlock &TMBB,
                      unsigned NumT, unsigned ExtraT,
                      MachineBasicBlock &FMBB,
                      unsigned NumF, unsigned ExtraF,
                      BranchProbability Probability) const {
   return !(MBBDefinesCTR(TMBB) && MBBDefinesCTR(FMBB));
 }
 
 
 bool PPCInstrInfo::isPredicated(const MachineInstr &MI) const {
   // The predicated branches are identified by their type, not really by the
   // explicit presence of a predicate. Furthermore, some of them can be
   // predicated more than once. Because if conversion won't try to predicate
   // any instruction which already claims to be predicated (by returning true
   // here), always return false. In doing so, we let isPredicable() be the
   // final word on whether not the instruction can be (further) predicated.
 
   return false;
 }
 
 bool PPCInstrInfo::isUnpredicatedTerminator(const MachineInstr &MI) const {
if (OpC == PPC::BLR || OpC == PPC::BLR8) {
     } else if (Pred[0].getImm() == PPC::PRED_BIT_SET) {
       MI.setDesc(get(PPC::BCLR));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
     } else if (Pred[0].getImm() == PPC::PRED_BIT_UNSET) {
       MI.setDesc(get(PPC::BCLRn));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI).add(Pred[1]);
     } else {
       MI.setDesc(get(PPC::BCCLR));
       MachineInstrBuilder(*MI.getParent()->getParent(), MI)
           .addImm(Pred[0].getImm())
           .add(Pred[1]);
     }
 
     return true;
   } else if (OpC == PPC::B) {
     if (Pred[1].getReg() == PPC::CTR8 || Pred[1].getReg() == PPC::CTR) {
       bool isPPC64 = Subtarget.isPPC64();

llvm/lib/Target/PowerPC/PPCMIPeephole.cpp

STATISTIC(NumFunctionsEnteredInMIPeephole,
           "Number of functions entered in PPC MI Peepholes");
 STATISTIC(NumFixedPointIterations,
           "Number of fixed-point iterations converting reg-reg instructions "
           "to reg-imm ones");
 STATISTIC(NumRotatesCollapsed,
           "Number of pairs of rotate left, clear left/right collapsed");
 STATISTIC(NumEXTSWAndSLDICombined,
           "Number of pairs of EXTSW and SLDI combined as EXTSWSLI");
+STATISTIC(NumLoadImmZeroFoldedAndRemoved,
+          "Number of LI(8) reg, 0 that are folded to r0 and removed");
 
 static cl::opt<bool>
 FixedPointRegToImm("ppc-reg-to-imm-fixed-point", cl::Hidden, cl::init(true),
                    cl::desc("Iterate to a fixed point when attempting to "
                             "convert reg-reg instructions to reg-imm"));
 
 static cl::opt<bool>
 ConvertRegReg("ppc-convert-rr-to-ri", cl::Hidden, cl::init(true),
for (MachineInstr &MI : MBB) {
       if (MI.isDebugInstr())
         continue;
 
       // Per-opcode peepholes.
       switch (MI.getOpcode()) {
 
       default:
         break;
-
+      case PPC::LI:
+      case PPC::LI8: {
+        // If we are materializing a zero, look for any use operands for which
+        // zero means immediate zero. All such operands can be replaced with
+        // PPC::ZERO.
+        if (!MI.getOperand(1).isImm() || MI.getOperand(1).getImm() != 0)
+          break;
+        unsigned MIDestReg = MI.getOperand(0).getReg();
+        for (MachineInstr& UseMI : MRI->use_instructions(MIDestReg))
+          Simplified |= TII->onlyFoldImmediate(UseMI, MI, MIDestReg);
+        if (MRI->use_nodbg_empty(MIDestReg)) {
+          ++NumLoadImmZeroFoldedAndRemoved;
+          ToErase = &MI;
+        }
+        break;
+      }
       case PPC::STD: {
         MachineFrameInfo &MFI = MF->getFrameInfo();
         if (MFI.hasVarSizedObjects() ||
             !MF->getSubtarget<PPCSubtarget>().isELFv2ABI())
           break;
         // When encountering a TOC save instruction, call UpdateTOCSaves
         // to add it to the TOCSaves map and mark any existing TOC saves
         // it dominates as redundant.
         if (TII->isTOCSaveMI(MI))
           UpdateTOCSaves(TOCSaves, &MI);
         break;
       }
       case PPC::XXPERMDI: {

llvm/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir

 body:             |
   bb.0.entry:
     liveins: $x3, $x4
   
     %1 = COPY $x4
     %0 = LI8 89
     %2 = CMPDI %0, 87
     %4 = ISEL8 $zero8, %0, %2.sub_gt
-    ; CHECK: LI8 0
+    ; CHECK: ADDI8 %1, 0
     %5 = ADD8 killed %4, %1
     $x3 = COPY %5
     BLR8 implicit $lr8, implicit $rm, implicit $x3
 
 ...
 ---
 name:            testCMPDI_F
 # CHECK-ALL: name: testCMPDI_F
bb.0.entry:
     liveins: $x3, $x4
   
     %1 = COPY $x4
     %0 = COPY $x3
     %2 = COPY %1.sub_32
     %3 = LI -3
     %4 = CMPLWI %3, 87
     %6 = ISEL $zero, %3, %4.sub_gt
-    ; CHECK: LI 0
+    ; CHECK: ADDI killed %2, 0
     %7 = ADD4 killed %6, killed %2
     %9 = IMPLICIT_DEF
     %8 = INSERT_SUBREG %9, killed %7, 1
     %10 = RLDICL killed %8, 0, 32
     $x3 = COPY %10
     BLR8 implicit $lr8, implicit $rm, implicit $x3
 
 ...

llvm/test/CodeGen/PowerPC/fold-remove-li.ll

+; NOTE: This test verifies that a redundant load immediate of zero is folded
+; NOTE: from its use in an isel and deleted as it is no longer in use.
+; RUN:  llc -mcpu=pwr9 -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:  -ppc-asm-full-reg-names -verify-machineinstrs < %s | FileCheck %s
+; RUN:  llc -mcpu=pwr9 -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:  -ppc-asm-full-reg-names -verify-machineinstrs < %s | FileCheck %s
+
+%0 = type { i32, i16 }
+
+@val = common dso_local local_unnamed_addr global %0* null, align 8
+
+define dso_local signext i32 @redunLoadImm(%0* %arg) {
+; CHECK-LABEL: redunLoadImm:
+; verify that the load immediate has been folded into the isel and deleted
+; CHECK-NOT:   li r[[REG1:[0-9]+]], 0
+; CHECK:       isel r[[REG2:[0-9]+]], 0, r[[REG3:[0-9]+]], eq
+
+bb:
+  %tmp = icmp eq %0* %arg, null
+  br i1 %tmp, label %bb9, label %bb1
+
+bb1:                                              ; preds = %bb
+  %tmp2 = getelementptr inbounds %0, %0* %arg, i64 0, i32 1
+  br label %bb3
+
+bb3:                                              ; preds = %bb3, %bb1
+  %tmp4 = load i16, i16* %tmp2, align 4
+  %tmp5 = sext i16 %tmp4 to i64
+  %tmp6 = getelementptr inbounds %0, %0* %arg, i64 %tmp5
+  %tmp7 = icmp eq i16 %tmp4, 0
+  %tmp8 = select i1 %tmp7, %0* null, %0* %tmp6
+  store %0* %tmp8, %0** @val, align 8
+  br label %bb3
+
+bb9:                                              ; preds = %bb
+  %tmp10 = load %0*, %0** @val, align 8
+  %tmp11 = getelementptr inbounds %0, %0* %tmp10, i64 0, i32 0
+  %tmp12 = load i32, i32* %tmp11, align 4
+  ret i32 %tmp12
+}

llvm/test/CodeGen/PowerPC/save-crbp-ppc32svr4.ll

 ; RUN: llc -relocation-model pic < %s | FileCheck %s
 
 ; CHECK-LABEL: fred
 ; CHECK: stwux 1, 1, 0
 ; Save R31..R29 via R0:
 ; CHECK: addic 0, 0, -12
 ; CHECK: stwx 31, 0, 0
 ; CHECK: addic 0, 0, -4
 ; CHECK: stwx 30, 0, 0
 ; CHECK: addic 0, 0, -4
 ; CHECK: stwx 29, 0, 0
 ; Set R29 back to the value of R0 from before the updates:
 ; CHECK: addic 29, 0, 20
 ; Save CR through R12 using R29 as the stack pointer (aligned base pointer).
 ; CHECK: mfcr 12
-; CHECK: stw 28, -24(29)
-; CHECK: stw 12, -28(29)
+; CHECK: stw 12, -24(29)
 
 target datalayout = "E-m:e-p:32:32-i64:64-n32"
 target triple = "powerpc-unknown-freebsd"
 
 ; Function Attrs: norecurse readnone sspstrong
 define i64 @fred(double %a0) local_unnamed_addr #0 {
 b1:
   %v2 = alloca i64, align 128