[llvm-commits] [llvm] r160454 - in /llvm/trunk: lib/Target/X86/X86InstrArithmetic.td lib/Target/X86/X86InstrInfo.cpp test/CodeGen/X86/jump_sign.ll

Chris Lattner clattner at apple.com
Sat Jul 21 21:22:51 PDT 2012


On Jul 18, 2012, at 2:40 PM, Manman Ren wrote:

> Author: mren
> Date: Wed Jul 18 16:40:01 2012
> New Revision: 160454
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=160454&view=rev
> Log:
> X86: remove redundant cmp against zero.
> 
> Updated OptimizeCompare in peephole to remove redundant cmp against zero.
> We only remove Compare if CF and OF are not used.
> 
> rdar://11855129

Hi Manman,

> +/// isDefConvertible - check whether the definition can be converted
> +/// to remove a comparison against zero.
> +inline static bool isDefConvertible(MachineInstr *MI) {

These big hand-written table of predicates are kind of sad, because it entails behavior that isn't surfaced in the .td file.  Would this be reasonable to represent with a bit in the "TSFlags" field?

-Chris


> +  switch (MI->getOpcode()) {
> +  default: return false;
> +  case X86::SUB64ri32: case X86::SUB64ri8: case X86::SUB32ri:
> +  case X86::SUB32ri8:  case X86::SUB16ri:  case X86::SUB16ri8:
> +  case X86::SUB8ri:    case X86::SUB64rr:  case X86::SUB32rr:
> +  case X86::SUB16rr:   case X86::SUB8rr:   case X86::SUB64rm:
> +  case X86::SUB32rm:   case X86::SUB16rm:  case X86::SUB8rm:
> +  case X86::ADD64ri32: case X86::ADD64ri8: case X86::ADD32ri:
> +  case X86::ADD32ri8:  case X86::ADD16ri:  case X86::ADD16ri8:
> +  case X86::ADD8ri:    case X86::ADD64rr:  case X86::ADD32rr:
> +  case X86::ADD16rr:   case X86::ADD8rr:   case X86::ADD64rm:
> +  case X86::ADD32rm:   case X86::ADD16rm:  case X86::ADD8rm:
> +  case X86::AND64ri32: case X86::AND64ri8: case X86::AND32ri:
> +  case X86::AND32ri8:  case X86::AND16ri:  case X86::AND16ri8:
> +  case X86::AND8ri:    case X86::AND64rr:  case X86::AND32rr:
> +  case X86::AND16rr:   case X86::AND8rr:   case X86::AND64rm:
> +  case X86::AND32rm:   case X86::AND16rm:  case X86::AND8rm:
> +  case X86::XOR64ri32: case X86::XOR64ri8: case X86::XOR32ri:
> +  case X86::XOR32ri8:  case X86::XOR16ri:  case X86::XOR16ri8:
> +  case X86::XOR8ri:    case X86::XOR64rr:  case X86::XOR32rr:
> +  case X86::XOR16rr:   case X86::XOR8rr:   case X86::XOR64rm:
> +  case X86::XOR32rm:   case X86::XOR16rm:  case X86::XOR8rm:
> +  case X86::OR64ri32:  case X86::OR64ri8:  case X86::OR32ri:
> +  case X86::OR32ri8:   case X86::OR16ri:   case X86::OR16ri8:
> +  case X86::OR8ri:     case X86::OR64rr:   case X86::OR32rr:
> +  case X86::OR16rr:    case X86::OR8rr:    case X86::OR64rm:
> +  case X86::OR32rm:    case X86::OR16rm:   case X86::OR8rm:
> +    return true;
> +  }
> +}
> +
> /// optimizeCompareInstr - Check if there exists an earlier instruction that
> /// operates on the same source operands and sets flags in the same way as
> /// Compare; remove Compare if possible.
> @@ -3107,6 +3152,13 @@
>   // CmpInstr is the first instruction of the BB.
>   MachineBasicBlock::iterator I = CmpInstr, Def = MI;
> 
> +  // If we are comparing against zero, check whether we can use MI to update
> +  // EFLAGS. If MI is not in the same BB as CmpInstr, do not optimize.
> +  bool IsCmpZero = (SrcReg2 == 0 && CmpValue == 0);
> +  if (IsCmpZero && (MI->getParent() != CmpInstr->getParent() ||
> +      !isDefConvertible(MI)))
> +    return false;
> +
>   // We are searching for an earlier instruction that can make CmpInstr
>   // redundant and that instruction will be saved in Sub.
>   MachineInstr *Sub = NULL;
> @@ -3126,7 +3178,8 @@
>   for (; RI != RE; ++RI) {
>     MachineInstr *Instr = &*RI;
>     // Check whether CmpInstr can be made redundant by the current instruction.
> -    if (isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpValue, Instr)) {
> +    if (!IsCmpZero &&
> +        isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpValue, Instr)) {
>       Sub = Instr;
>       break;
>     }
> @@ -3153,7 +3206,7 @@
>   }
> 
>   // Return false if no candidates exist.
> -  if (!Sub)
> +  if (!IsCmpZero && !Sub)
>     return false;
> 
>   bool IsSwapped = (SrcReg2 != 0 && Sub->getOperand(1).getReg() == SrcReg2 &&
> @@ -3177,13 +3230,10 @@
>       continue;
> 
>     // EFLAGS is used by this instruction.
> -    if (IsSwapped) {
> -      // If we have SUB(r1, r2) and CMP(r2, r1), the condition code needs
> -      // to be changed from r2 > r1 to r1 < r2, from r2 < r1 to r1 > r2, etc.
> -      // We decode the condition code from opcode, swap the condition code,
> -      // and synthesize the new opcode.
> -      bool OpcIsSET = false;
> -      X86::CondCode OldCC;
> +    X86::CondCode OldCC;
> +    bool OpcIsSET = false;
> +    if (IsCmpZero || IsSwapped) {
> +      // We decode the condition code from opcode.
>       if (Instr.isBranch())
>         OldCC = getCondFromBranchOpc(Instr.getOpcode());
>       else {
> @@ -3194,6 +3244,22 @@
>           OldCC = getCondFromCMovOpc(Instr.getOpcode());
>       }
>       if (OldCC == X86::COND_INVALID) return false;
> +    }
> +    if (IsCmpZero) {
> +      switch (OldCC) {
> +      default: break;
> +      case X86::COND_A: case X86::COND_AE:
> +      case X86::COND_B: case X86::COND_BE:
> +      case X86::COND_G: case X86::COND_GE:
> +      case X86::COND_L: case X86::COND_LE:
> +      case X86::COND_O: case X86::COND_NO:
> +        // CF and OF are used, we can't perform this optimization.
> +        return false;
> +      }
> +    } else if (IsSwapped) {
> +      // If we have SUB(r1, r2) and CMP(r2, r1), the condition code needs
> +      // to be changed from r2 > r1 to r1 < r2, from r2 < r1 to r1 > r2, etc.
> +      // We swap the condition code and synthesize the new opcode.
>       X86::CondCode NewCC = getSwappedCondition(OldCC);
>       if (NewCC == X86::COND_INVALID) return false;
> 
> @@ -3223,7 +3289,7 @@
> 
>   // If EFLAGS is not killed nor re-defined, we should check whether it is
>   // live-out. If it is live-out, do not optimize.
> -  if (IsSwapped && !IsSafe) {
> +  if ((IsCmpZero || IsSwapped) && !IsSafe) {
>     MachineBasicBlock *MBB = CmpInstr->getParent();
>     for (MachineBasicBlock::succ_iterator SI = MBB->succ_begin(),
>              SE = MBB->succ_end(); SI != SE; ++SI)
> @@ -3231,6 +3297,8 @@
>         return false;
>   }
> 
> +  // The instruction to be updated is either Sub or MI.
> +  Sub = IsCmpZero ? MI : Sub;
>   // Move Movr0Inst to the place right before Sub.
>   if (Movr0Inst) {
>     Sub->getParent()->remove(Movr0Inst);
> @@ -3238,10 +3306,11 @@
>   }
> 
>   // Make sure Sub instruction defines EFLAGS.
> -  assert(Sub->getNumOperands() >= 4 && Sub->getOperand(3).isReg() &&
> -         Sub->getOperand(3).getReg() == X86::EFLAGS &&
> -         "EFLAGS should be the 4th operand of SUBrr or SUBri.");
> -  Sub->getOperand(3).setIsDef(true);
> +  assert(Sub->getNumOperands() >= 2 &&
> +         Sub->getOperand(Sub->getNumOperands()-1).isReg() &&
> +         Sub->getOperand(Sub->getNumOperands()-1).getReg() == X86::EFLAGS &&
> +         "EFLAGS should be the last operand of SUB, ADD, OR, XOR, AND");
> +  Sub->getOperand(Sub->getNumOperands()-1).setIsDef(true);
>   CmpInstr->eraseFromParent();
> 
>   // Modify the condition code of instructions in OpsToUpdate.
> 
> Modified: llvm/trunk/test/CodeGen/X86/jump_sign.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/jump_sign.ll?rev=160454&r1=160453&r2=160454&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/jump_sign.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/jump_sign.ll Wed Jul 18 16:40:01 2012
> @@ -202,3 +202,14 @@
> if.else.i104:                                     ; preds = %if.then44
>   ret void
> }
> +; rdar://11855129
> +define i32 @p(i32 %a, i32 %b) nounwind {
> +entry:
> +; CHECK: p:
> +; CHECK-NOT: test
> +; CHECK: cmovs
> +  %add = add nsw i32 %b, %a
> +  %cmp = icmp sgt i32 %add, 0
> +  %add. = select i1 %cmp, i32 %add, i32 0
> +  ret i32 %add.
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list