Codegen: [PPC] Word Rotates are Zero Extending

[iteratee]

Summary

Add Word rotates to the list of instructions that are zero extending.
This allows them to be used in dot form to compare with zero.

[echristo]

Testcase?

OK with one of those. :)

-eric

[hfinkel]

I don't think this is right. RLWINM starts by taking two copies of the 32-bit amount, one copy in the lower word and one copy in the upper word and rotates the result, Then it composes the mask. The mask is non-zero in the upper bits if MB >= ME.

[echristo]

You're right. We came up with the usual case, but not the normal case. Kyle
is going to add a check for the arguments as well :)

-eric

[echristo]

Also why testcases are awesome :)

-eric

[iteratee]

Added an operand check to make sure that we only perform this optimization in the case where the mask causes the upper 32 bits to be zeroed out.

Also add a test case to verify that it fires in the correct case, and doesn't fire in the incorrect case.

[echristo]

One nit from me, but given I missed one thing previously I'm going to ask Hal to ack this one :)

-eric

lib/Target/PowerPC/PPCInstrInfo.cpp

1557–1561

Stylistic request: Can you move this closer to the use please?

[kbarton]

lib/Target/PowerPC/PPCInstrInfo.cpp

1559

I think you mean to use MIOpC == PPC::RLWINMo as the second part of the or statement

[iteratee]

Fix mistake caught by kbarton, and add test case to cover that mistake.

[iteratee]

lib/Target/PowerPC/PPCInstrInfo.cpp

1559

I actually meant to use PPC::RLWNM.
I added a test case to cover this.

[kbarton]

lib/Target/PowerPC/PPCInstrInfo.cpp

1559

OK, this might be a dumb question then but why not include the dot version of the instructions (RLWINMo and RLWNMo) as well?

[iteratee]

lib/Target/PowerPC/PPCInstrInfo.cpp

1559

I've added them, but maybe someone else can chime in. Is the goal of checking for those forms to allow for the collapse of multiple compares? Is this something that shows up in real code, or a belt/suspenders check?

[iteratee]

Check for rotates already in dot form.

[iteratee]

Move definition closer to use.

[iteratee]

lib/Target/PowerPC/PPCInstrInfo.cpp

1557–1561

Sorry, I closed this without actually moving it closer. Really done now.

[echristo]

I'm fine here now, Kit or Hal want to comment further?

-eric

[hfinkel]

LGTM.

lib/Target/PowerPC/PPCInstrInfo.cpp

bool PPCInstrInfo::optimizeCompareInstr(MachineInstr *CmpInstr,
   bool is32BitUnsignedCompare = OpC == PPC::CMPLWI || OpC == PPC::CMPLW;
   bool is64BitUnsignedCompare = OpC == PPC::CMPLDI || OpC == PPC::CMPLD;
 
   // Get the unique definition of SrcReg.
   MachineInstr *MI = MRI->getUniqueVRegDef(SrcReg);
   if (!MI) return false;
   int MIOpC = MI->getOpcode();
 
   bool equalityOnly = false;
   bool noSub = false;
   if (isPPC64) {
     if (is32BitSignedCompare) {
       // We can perform this optimization only if MI is sign-extending.
       if (MIOpC == PPC::SRAW  || MIOpC == PPC::SRAWo ||
           MIOpC == PPC::SRAWI || MIOpC == PPC::SRAWIo ||
           MIOpC == PPC::EXTSB || MIOpC == PPC::EXTSBo ||
           MIOpC == PPC::EXTSH || MIOpC == PPC::EXTSHo ||
           MIOpC == PPC::EXTSW || MIOpC == PPC::EXTSWo) {
         noSub = true;
       } else
         return false;
     } else if (is32BitUnsignedCompare) {
+      // 32-bit rotate and mask instructions are zero extending only if MB <= ME
+      bool isZeroExtendingRotate  =
+          (MIOpC == PPC::RLWINM || MIOpC == PPC::RLWINMo ||
+           MIOpC == PPC::RLWNM || MIOpC == PPC::RLWNMo)
+          && MI->getOperand(3).getImm() <= MI->getOperand(4).getImm();
+
       // We can perform this optimization, equality only, if MI is
       // zero-extending.
       if (MIOpC == PPC::CNTLZW || MIOpC == PPC::CNTLZWo ||
           MIOpC == PPC::SLW    || MIOpC == PPC::SLWo ||
-          MIOpC == PPC::SRW    || MIOpC == PPC::SRWo) {
+          MIOpC == PPC::SRW    || MIOpC == PPC::SRWo ||
+          isZeroExtendingRotate) {
         noSub = true;
         equalityOnly = true;
       } else
         return false;
     } else
       equalityOnly = is64BitUnsignedCompare;
   } else
     equalityOnly = is32BitUnsignedCompare;

test/CodeGen/PowerPC/rlwinm-zero-ext.ll

+; RUN: llc -O2 < %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-n32:64"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+; CHECK-LABEL: test1
+define i8 @test1(i32 %a) {
+entry:
+; CHECK-NOT: rlwinm {{{[0-9]+}}}, {{[0-9]+}}, 0, 24, 27
+; CHECK: rlwinm. [[REG:[0-9]+]], {{[0-9]+}}, 0, 24, 27
+; CHECK-NOT: cmplwi [[REG]], 0
+; CHECK: beq 0
+  %0 = and i32 %a, 240
+  %1 = icmp eq i32 %0, 0
+  br i1 %1, label %eq0, label %neq0
+eq0:
+  ret i8 102
+neq0:
+  ret i8 116
+}
+
+; CHECK-LABEL: test2
+define i8 @test2(i32 %a) {
+entry:
+; CHECK: rlwinm [[REG:[0-9]+]], {{[0-9]+}}, 0, 28, 23
+; CHECK: cmplwi [[REG]], 0
+; CHECK: beq 0
+  %0 = and i32 %a, -241
+  %1 = icmp eq i32 %0, 0
+  br i1 %1, label %eq0, label %neq0
+eq0:
+  ret i8 102
+neq0:
+  ret i8 116
+}
+
+declare {i32, i1} @llvm.ssub.with.overflow.i32(i32 %a, i32 %b)
+
+; CHECK-LABEL: test3
+define i8 @test3(i32 %a, i32 %b) {
+entry:
+; CHECK-NOT: rlwnm {{{[0-9]+}}}, {{[0-9]+}}, {{{[0-9]+}}}, 28, 31
+; CHECK: rlwnm. [[REG:[0-9]+]], {{[0-9]+}}, 4, 28, 31
+; CHECK-NOT: cmplwi [[REG]], 0
+; CHECK: beq 0
+  %left = shl i32 %a, %b
+  %res = call {i32, i1} @llvm.ssub.with.overflow.i32(i32 32, i32 %b)
+  %right_amount = extractvalue {i32, i1} %res, 0
+  %right = lshr i32 %a, %right_amount
+  %0 = or i32 %left, %right
+  %1 = and i32 %0, 15
+  %2 = icmp eq i32 %1, 0
+  br i1 %2, label %eq0, label %neq0
+eq0:
+  ret i8 102
+neq0:
+  ret i8 116
+}