[PowerPC] fix a bug in redundant compare elimination

[inouehrs]

Summary

This patch fixes a bug in the redundant compare elimination reported in https://reviews.llvm.org/rL320786 and re-enables the optimization.

The redundant compare elimination assumes that we can replace signed comparison with unsigned comparison for the equality check. But due to the difference in the sign extension behavior we cannot change the opcode if the comparison is against an immediate and the most significant bit of the immediate is one.
The reported problem happen in the following code sequence:

if (a == -1) ...
if (a == (uint16_t)-1) ...

[hfinkel]

LGTM

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

 //   bgt    0, .LBB0_4
 // So, before merging two compares, we need to modify these instructions as
 //   cmpwi  r3, 0       ; cmplwi and cmpwi yield same result for beq
 //   beq    0, .LBB0_3
 //   cmpwi  r3, 0       ; greather than -1 means greater or equal to 0
 //   bge    0, .LBB0_4
 
 bool PPCMIPeephole::eliminateRedundantCompare(void) {
-  // FIXME: this transformation is causing miscompiles. Disabling it for now
-  // until we can resolve the issue.
-  return false;
   bool Simplified = false;
 
   for (MachineBasicBlock &MBB2 : *MF) {
     MachineBasicBlock *MBB1 = nullptr, *MBBtoMoveCmp = nullptr;
 
     // For fully redundant case, we select two basic blocks MBB1 and MBB2
     // as an optimization target if
     // - both MBBs end with a conditional branch,
for (MachineBasicBlock &MBB2 : *MF) {
     int16_t Imm1 = 0, NewImm1 = 0, Imm2 = 0, NewImm2 = 0;
     bool SwapOperands = false;
 
     if (CMPI1->getOpcode() != CMPI2->getOpcode()) {
       // Typically, unsigned comparison is used for equality check, but
       // we replace it with a signed comparison if the comparison
       // to be merged is a signed comparison.
       // In other cases of opcode mismatch, we cannot optimize this.
-      if (isEqOrNe(BI2) &&
+
+      // We cannot change opcode when comparing against an immediate
+      // if the most significant bit of the immediate is one
+      // due to the difference in sign extension.
+      auto CmpAgainstImmWithSignBit = [](MachineInstr *I) {
+        if (!I->getOperand(2).isImm())
+          return false;
+        int16_t Imm = (int16_t)I->getOperand(2).getImm();
+        return Imm < 0;
+      };
+
+      if (isEqOrNe(BI2) && !CmpAgainstImmWithSignBit(CMPI2) &&
           CMPI1->getOpcode() == getSignedCmpOpCode(CMPI2->getOpcode()))
         NewOpCode = CMPI1->getOpcode();
-      else if (isEqOrNe(BI1) &&
+      else if (isEqOrNe(BI1) && !CmpAgainstImmWithSignBit(CMPI1) &&
                getSignedCmpOpCode(CMPI1->getOpcode()) == CMPI2->getOpcode())
         NewOpCode = CMPI2->getOpcode();
       else continue;
     }
 
     if (CMPI1->getOperand(2).isReg() && CMPI2->getOperand(2).isReg()) {
       // In case of comparisons between two registers, these two registers
       // must be same to merge two comparisons.

llvm/trunk/test/CodeGen/PowerPC/cmp_elimination.ll

-; XFAIL: *
 ; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64-unknown-linux-gnu | FileCheck %s
 ; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64le-unknown-linux-gnu | FileCheck %s
 
 ; Test cases for compare elimination in PPCMIPeephole pass
 
 define void @func1(i32 signext %a) {
 ; We should have only one compare instruction
 ; CHECK-LABEL: @func1
if.end:
   %call = tail call signext i32 @func(i32 signext %a.addr.0) #2
   %cmp1 = icmp sgt i32 %call, 0
   br i1 %cmp1, label %do.body, label %do.end
 
 do.end:
   ret void
 }
 
+define void @func29(i32 signext %a) {
+; We cannot merge two compares due to difference in sign extension behaviors.
+; equivalent C code example:
+;   int a = .. ;
+;   if (a == -1) dummy1();
+;   if (a == (uint16_t)-1) dummy2();
+
+; CHECK-LABEL: @func29
+; CHECK: cmp
+; CHECK: cmp
+; CHECK: blr
+entry:
+  %cmp = icmp eq i32 %a, -1
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  tail call void @dummy1()
+  br label %if.end3
+
+if.else:
+  %cmp1 = icmp eq i32 %a, 65535
+  br i1 %cmp1, label %if.then2, label %if.end3
+
+if.then2:
+  tail call void @dummy2()
+  br label %if.end3
+
+if.end3:
+  ret void
+}
+
 declare void @dummy1()
 declare void @dummy2()
 declare void @dummy3()
 declare signext i32 @func(i32 signext)
 
 !1 = !{!"branch_weights", i32 2000, i32 1}
 !2 = !{!"branch_weights", i32 1, i32 2000}