[PowerPC] Use helper functions to check sign-/zero-extended value

[inouehrs]

Summary

Helper functions to identify sign- and zero-extending machine instruction is introduced in rL315888.
This patch makes PPCInstrInfo::optimizeCompareInstr use the helper functions. It simplifies the code and also makes possible more optimizations since the helper can do more analysis than the original check code; I observed about 5000 more compare instructions are eliminated while building LLVM.

Also, this patch fixes a bug in helpers on ANDIo instruction handling due to the order of checks. This bug causes a failure in an existing test case for optimizeCompareInstr.

[nemanjai]

This seems like a perfectly valid thing to do. Please add a test case before this can proceed though.
I would imagine that one of the extra 5000 eliminated compare instructions would make for a good test case.

And FWIW, I still don't like the names of these helper functions. There isn't really much indication in the name what it is sign/zero extending from/to. Presume I want to do add/remove an instruction depending on whether the input is known to be sign-extended from a halfword to a doubleword, but not if it is only known to be sign-extended from a word to a doubleword, it isn't clear how I would use these functions to determine that.

[inouehrs]

• a test case added

[inouehrs]

In D38988#899532, @nemanjai wrote:

And FWIW, I still don't like the names of these helper functions. There isn't really much indication in the name what it is sign/zero extending from/to. Presume I want to do add/remove an instruction depending on whether the input is known to be sign-extended from a halfword to a doubleword, but not if it is only known to be sign-extended from a word to a doubleword, it isn't clear how I would use these functions to determine that.

Is isSignExtendedFromWord better? Other candidates I have are isSignExtendedWord or isSignExtended_32_64. Any suggestions?

[nemanjai]

In D38988#900526, @inouehrs wrote:

In D38988#899532, @nemanjai wrote:

And FWIW, I still don't like the names of these helper functions. There isn't really much indication in the name what it is sign/zero extending from/to. Presume I want to do add/remove an instruction depending on whether the input is known to be sign-extended from a halfword to a doubleword, but not if it is only known to be sign-extended from a word to a doubleword, it isn't clear how I would use these functions to determine that.

Is isSignExtendedFromWord better? Other candidates I have are isSignExtendedWord or isSignExtended_32_64. Any suggestions?

Well, I think it is important to also communicate that the zero/sign extendedness is a property of the instruction rather than a computation of known bits. So I'm thinking a name like instrZeroExtends32To64() and instrSignExtends32To64(). Or perhaps I'm being pedantic here, maybe @echristo or @hfinkel want to chime in. Of course, the rename can be done in a separate patch.

But this patch LGTM now.

lib/Target/PowerPC/PPCInstrInfo.cpp

bool PPCInstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
   bool isPPC64 = Subtarget.isPPC64();
   bool is32BitSignedCompare   = OpC ==  PPC::CMPWI || OpC == PPC::CMPW;
   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) {
+      if (isSignExtended(*MI))
         noSub = true;
-      } else
+      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.
-      // FIXME: Other possible target instructions include ANDISo and
-      //        RLWINM aliases, such as ROTRWI, EXTLWI, SLWI and SRWI.
-      if (MIOpC == PPC::CNTLZW || MIOpC == PPC::CNTLZWo ||
-          MIOpC == PPC::SLW    || MIOpC == PPC::SLWo ||
-          MIOpC == PPC::SRW    || MIOpC == PPC::SRWo ||
-          MIOpC == PPC::ANDIo  ||
-          isZeroExtendingRotate) {
+      if (isZeroExtended(*MI)) {
         noSub = true;
         equalityOnly = true;
       } else
         return false;
     } else
       equalityOnly = is64BitUnsignedCompare;
   } else
     equalityOnly = is32BitUnsignedCompare;
bool PPCInstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
   // Return false if no candidates exist.
   if (!MI && !Sub)
     return false;
 
   // The single candidate is called MI.
   if (!MI) MI = Sub;
 
   int NewOpC = -1;
-  MIOpC = MI->getOpcode();
+  int MIOpC = MI->getOpcode();
   if (MIOpC == PPC::ANDIo || MIOpC == PPC::ANDIo8)
     NewOpC = MIOpC;
   else {
     NewOpC = PPC::getRecordFormOpcode(MIOpC);
     if (NewOpC == -1 && PPC::getNonRecordFormOpcode(MIOpC) != -1)
       NewOpC = MIOpC;
   }
 

 const unsigned MAX_DEPTH = 1;
 
 bool
 PPCInstrInfo::isSignOrZeroExtended(const MachineInstr &MI, bool SignExt,
                                    const unsigned Depth) const {
   const MachineFunction *MF = MI.getParent()->getParent();
   const MachineRegisterInfo *MRI = &MF->getRegInfo();
 
+  // If we know this instruction returns sign- or zero-extended result,
+  // return true.
+  if (SignExt ? isSignExtendingOp(MI):
+                isZeroExtendingOp(MI))
+    return true;
+
   switch (MI.getOpcode()) {
   case PPC::COPY: {
     unsigned SrcReg = MI.getOperand(1).getReg();
 
     // In both ELFv1 and v2 ABI, method parameters and the return value
     // are sign- or zero-extended.
     if (MF->getSubtarget<PPCSubtarget>().isSVR4ABI()) {
       const PPCFunctionInfo *FuncInfo = MF->getInfo<PPCFunctionInfo>();
for (unsigned I = 1; I != E; I += D) {
       }
       else
         return false;
     }
     return true;
   }
 
   default:
-    return SignExt?isSignExtendingOp(MI):
-                   isZeroExtendingOp(MI);
+    break;
   }
   return false;
 }

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

if.then:
   tail call void @exit(i32 signext 0) #3
   unreachable
 
 if.end:
   ret i64 %add
 }
 
 declare void @exit(i32 signext)
+
+; Since %v1 and %v2 are zero-extended 32-bit values, %1 is also zero-extended.
+; In this case, we want to use ORo instead of OR + CMPLWI.
+
+; CHECK-LABEL: fn5
+define zeroext i32 @fn5(i32* %p1, i32* %p2) {
+; CHECK: ORo
+; CHECK-NOT: CMP
+; CHECK: BCC
+  %v1 = load i32, i32* %p1
+  %v2 = load i32, i32* %p2
+  %1 = or i32 %v1, %v2
+  %2 = icmp eq i32 %1, 0
+  br i1 %2, label %foo, label %bar
+
+foo:
+  ret i32 1
+
+bar:
+  ret i32 0
+}