[PowerPC] Use record-form instruction for Less-or-Equal -1 and Greater-or-Equal 1

[inouehrs]

Summary

Currently a record-form instruction is used for comparison of "greater than -1" and "less than 1" by modifying the predicate (e.g. LT 1 into LE 0) in addition to the naive case of comparison against 0.
This patch also enables emitting a record-form instruction for "less than or equal to -1" (i.e. "less than 0") and "greater than or equal to 1" (i.e. "greater than 0") to increase the optimization opportunities.

[echristo]

Please update the comments in both the code and the testcase to express what we're looking for and why we're looking for it. Right now if something breaks no one will know why.

[inouehrs]

Comments updated/added in code and test case.

[sfertile]

lib/Target/PowerPC/PPCInstrInfo.cpp

1740–1743 ↗

(On Diff #119259)

It seems to me this would be cleaner if we get rid of the 'Success' variable and convert the 2 ifs to check the opposite conditions and early return on failure.

test/CodeGen/PowerPC/opt-cmp-inst-cr0-live.ll

83 ↗

(On Diff #119259)

cconfirms --> confirms

[inouehrs]

• updated based on suggestions from @sfertile
• rebased to the latest code and fixed a conflict

[inouehrs]

lib/Target/PowerPC/PPCInstrInfo.cpp

1740–1743 ↗

(On Diff #119259)

is this better?

test/CodeGen/PowerPC/opt-cmp-inst-cr0-live.ll

83 ↗

(On Diff #119259)

Good catch. Fixed.

[echristo]

Looks much better to me. Let sfertile give a final ACK since he's been in here too, but I'm happy. One inline comment :)

-eric

lib/Target/PowerPC/PPCInstrInfo.cpp

1724 ↗

(On Diff #120013)

Because... :)

[sfertile]

Looks good to me.

[nemanjai]

As Eric points out, perhaps mention that it would be possible to optimize even if there were multiple uses of the CR register if all the uses fell into one of the four categories, but it would be very unlikely that doing to would be profitable.

But I agree, this looks much better now.

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp

if (SrcReg2 != 0)
     MI = nullptr;
   // FIXME: Conservatively refuse to convert an instruction which isn't in the
   // same BB as the comparison. This is to allow the check below to avoid calls
   // (and other explicit clobbers); instead we should really check for these
   // more explicitly (in at least a few predecessors).
   else if (MI->getParent() != CmpInstr.getParent())
     return false;
   else if (Value != 0) {
-    // The record-form instructions set CR bit based on signed comparison against 0.
-    // We try to convert a compare against 1 or -1 into a compare against 0.
-    bool Success = false;
-    if (!equalityOnly && MRI->hasOneUse(CRReg)) {
-      MachineInstr *UseMI = &*MRI->use_instr_begin(CRReg);
-      if (UseMI->getOpcode() == PPC::BCC) {
-        PPC::Predicate Pred = (PPC::Predicate)UseMI->getOperand(0).getImm();
-        unsigned PredCond = PPC::getPredicateCondition(Pred);
-        unsigned PredHint = PPC::getPredicateHint(Pred);
-        int16_t Immed = (int16_t)Value;
+    // The record-form instructions set CR bit based on signed comparison
+    // against 0. We try to convert a compare against 1 or -1 into a compare
+    // against 0 to exploit record-form instructions. For example, we change
+    // the condition "greater than -1" into "greater than or equal to 0"
+    // and "less than 1" into "less than or equal to 0".
+
+    // Since we optimize comparison based on a specific branch condition,
+    // we don't optimize if condition code is used by more than once.
+    if (equalityOnly || !MRI->hasOneUse(CRReg))
+      return false;
+
+    if (UseMI->getOpcode() != PPC::BCC)
+      return false;
+
+    PPC::Predicate NewPred = Pred;
 
-        // When modyfing the condition in the predicate, we propagate hint bits
-        // from the original predicate to the new one.
-        if (Immed == -1 && PredCond == PPC::PRED_GT) {
-          // We convert "greater than -1" into "greater than or equal to 0",
-          // since we are assuming signed comparison by !equalityOnly
-          PredsToUpdate.push_back(std::make_pair(&(UseMI->getOperand(0)),
-                                  PPC::getPredicate(PPC::PRED_GE, PredHint)));
-          Success = true;
-        }
-        else if (Immed == 1 && PredCond == PPC::PRED_LT) {
-          // We convert "less than 1" into "less than or equal to 0".
-          PredsToUpdate.push_back(std::make_pair(&(UseMI->getOperand(0)),
-                                  PPC::getPredicate(PPC::PRED_LE, PredHint)));
-          Success = true;
-        }
-      }
-    }
-
-    // PPC does not have a record-form SUBri.
-    if (!Success)
+    if (Immed == -1 && PredCond == PPC::PRED_GT)
+      NewPred = PPC::getPredicate(PPC::PRED_GE, PredHint);
+    else if (Immed == -1 && PredCond == PPC::PRED_LE)
+      // We convert "less than or equal to -1" into "less than 0".
+      NewPred = PPC::getPredicate(PPC::PRED_LT, PredHint);
+    else if (Immed == 1 && PredCond == PPC::PRED_LT)
+      NewPred = PPC::getPredicate(PPC::PRED_LE, PredHint);
+    else if (Immed == 1 && PredCond == PPC::PRED_GE)
+      // We convert "greater than or equal to 1" into "greater than 0".
+      NewPred = PPC::getPredicate(PPC::PRED_GT, PredHint);
+    else
       return false;
+
+    PredsToUpdate.push_back(std::make_pair(&(UseMI->getOperand(0)),
+                                            NewPred));
   }
 
   // Search for Sub.
   const TargetRegisterInfo *TRI = &getRegisterInfo();
   --I;
 
   // Get ready to iterate backward from CmpInstr.
   MachineBasicBlock::iterator E = MI, B = CmpInstr.getParent()->begin();

llvm/trunk/test/CodeGen/PowerPC/opt-cmp-inst-cr0-live.ll

 ; RUN: llc -verify-machineinstrs -print-before=peephole-opt -print-after=peephole-opt -mtriple=powerpc64-unknown-linux-gnu -o /dev/null 2>&1 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -print-before=peephole-opt -print-after=peephole-opt -mtriple=powerpc64le-unknown-linux-gnu -o /dev/null 2>&1 < %s | FileCheck %s
 
 ; CHECK-LABEL: fn1
 define signext i32 @fn1(i32 %baz) {
   %1 = mul nsw i32 %baz, 208
   %2 = zext i32 %1 to i64
   %3 = shl i64 %2, 48
   %4 = ashr exact i64 %3, 48
 ; CHECK: ANDIo8 {{[^,]+}}, 65520, %CR0<imp-def,dead>;
; CHECK: BCC
   br i1 %2, label %foo, label %bar
 
 foo:
   ret i32 1
 
 bar:
   ret i32 0
 }
+
+; This test confirms record-form instructions are emitted for comparison
+; against a non-zero value.
+
+; CHECK-LABEL: fn6
+define i8* @fn6(i8* readonly %p) {
+; CHECK: LBZU
+; CHECK: EXTSBo
+; CHECK-NOT: CMP
+; CHECK: BCC
+; CHECK: LBZU
+; CHECK: EXTSBo
+; CHECK-NOT: CMP
+; CHECK: BCC
+
+entry:
+  %incdec.ptr = getelementptr inbounds i8, i8* %p, i64 -1
+  %0 = load i8, i8* %incdec.ptr
+  %cmp = icmp sgt i8 %0, -1
+  br i1 %cmp, label %out, label %if.end
+
+if.end:
+  %incdec.ptr2 = getelementptr inbounds i8, i8* %p, i64 -2
+  %1 = load i8, i8* %incdec.ptr2
+  %cmp4 = icmp sgt i8 %1, -1
+  br i1 %cmp4, label %out, label %cleanup
+
+out:
+  %p.addr.0 = phi i8* [ %incdec.ptr, %entry ], [ %incdec.ptr2, %if.end ]
+  br label %cleanup
+
+cleanup:
+  %retval.0 = phi i8* [ %p.addr.0, %out ], [ null, %if.end ]
+  ret i8* %retval.0
+}