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

Manman Ren mren at apple.com
Wed Jun 27 16:30:00 PDT 2012


I updated the patch, please review and provide feedback.

Update peephole optimization for X86 and ARM:
1> Interface: added SrcReg2 to analyzeCompare and optimizeCompare.
                       AnalyzeCompare to analyzeCompare ...

2> ARM: Clean up, no functional change.
Replaced a loop with existing interfaces: modifiesRegister and readsRegister.
Factored out code into inline functions and simplified the code.

3> X86: added peephole optimization to remove cmp instruction
It will optimize the following:
  sub r1, r3
  cmp r3, r1 or cmp r1, r3
  bge L1
TO
  sub r1, r3
  bge L1 or ble L1
If the branch instruction can use flag from "sub", then we can eliminate
the "cmp" instruction.

For this optimization, we need to first check the condition code of
SET|CMOV|Jcc and then update the condition code.

This patch implemented 4 helper functions:
getSwappedConditionForCMov
getSwappedConditionForBranch
getSwappedConditionForSET
isRedundantFlagInstr

I can't think of a way to easily update the condition code of an instruction
by adding flags in td file or data in MCInstrDesc, since the condition code
is hard-coded in the opcode.

>From what I know, there are 3 pairs of equivalent falg instructions for ARM:
CMP vs SUB, CMN vs ADD, TST vs AND
2 pairs for X86:
CMP vs SUB, TST vs AND
If there are more pairs, or there is a better way to implement isRedundantFlagInstr, please let me know.

Comments are welcome.

Thanks,
Manman


On Jun 2, 2012, at 3:55 PM, Chandler Carruth wrote:

> 
> On Fri, Jun 1, 2012 at 12:49 PM, Manman Ren <mren at apple.com> wrote:
> >
> > Author: mren
> > Date: Fri Jun  1 14:49:33 2012
> > New Revision: 157831
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=157831&view=rev
> > Log:
> > X86: peephole optimization to remove cmp instruction
> 
> I'm frustrated that you waited a total of 3 hours for review on the final version of this patch before committing it, w/o getting an LGTM. Eric even made comments on that thread, which he has had to repeat here. Please try to give the code review process a chance.
> This patch doesn't feel ready for commit:
> 
> 1) From my original comments: I still think the properties of instructions you're currently hard coding in C++ need to be modeled in a more principled manner. Maybe this isn't possible, I'm not a backend expert, but in code review all you said was that it didn't exist today. The point of my comment was that if these properties are not currently available as a fundamental part of the instruction, you should add them rather than writing one-off functions like this.
> 
> 2) You haven't followed the coding conventions or addressed any style issues with the patch. Nor have you added doxygen comments for the APIs you've adding. I'll try to mention a few of the style issues below, but I'm just pointing out a few. It would be good to go over this and try to be consistent.
> 
> 3) I still think that this is a bit of a hack to handle a single case of a more general problem. There are numerous instructions in x86 that set some subset of EFLAGS. We should have a general pass that can both re-use existing instructions' flags, and even switch instructions which have equivalent instructions (functionally) that additionally set flags. Certainly, 'cmp x, y' sets flags *differently* from binary operations other than 'sub x, y', but you're already handling cases like x and y being reversed, so it doesn't seem like a huge leap here.
> 
> 4) Also, what about removing 'test' as well as 'cmp'?
> 
> >
> > This patch will optimize the following:
> >  sub r1, r3
> >  cmp r3, r1 or cmp r1, r3
> >  bge L1
> > TO
> >  sub r1, r3
> >  bge L1 or ble L1
> >
> > If the branch instruction can use flag from "sub", then we can eliminate
> > the "cmp" instruction.
> >
> > Modified:
> >    llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
> >    llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
> >    llvm/trunk/lib/Target/X86/X86InstrInfo.h
> >    llvm/trunk/test/CodeGen/X86/jump_sign.ll
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrArithmetic.td
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrArithmetic.td?rev=157831&r1=157830&r2=157831&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrArithmetic.td (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrArithmetic.td Fri Jun  1 14:49:33 2012
> > @@ -1143,7 +1143,9 @@
> >                             0, 0>;
> >  }
> >
> > +let isCompare = 1 in {
> >  defm CMP : ArithBinOp_F<0x38, 0x3A, 0x3C, "cmp", MRM7r, MRM7m, X86cmp, 0, 0>;
> > +}
> >
> >
> >  //===----------------------------------------------------------------------===//
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=157831&r1=157830&r2=157831&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Fri Jun  1 14:49:33 2012
> > @@ -33,6 +33,7 @@
> >  #include "llvm/Support/ErrorHandling.h"
> >  #include "llvm/Support/raw_ostream.h"
> >  #include "llvm/Target/TargetOptions.h"
> > +#include "llvm/ADT/Statistic.h"
> >  #include <limits>
> >
> >  #define GET_INSTRINFO_CTOR
> > @@ -40,6 +41,8 @@
> >
> >  using namespace llvm;
> >
> > +STATISTIC(NumCmpsRemoved, "Number of Cmps removed due to an earlier Sub");
> > +
> >  static cl::opt<bool>
> >  NoFusing("disable-spill-fusing",
> >          cl::desc("Disable fusing of spill code into instructions"));
> > @@ -2728,6 +2731,326 @@
> >  }
> >
> >  bool X86InstrInfo::
> > +AnalyzeCompare(const MachineInstr *MI, unsigned &SrcReg, int &CmpMask,
> > +               int &CmpValue) const {
> 
> 'analyzeCompare'
>  
> >
> > +  switch (MI->getOpcode()) {
> > +  default: break;
> > +  case X86::CMP64ri32:
> > +  case X86::CMP64ri8:
> > +  case X86::CMP32ri:
> > +  case X86::CMP32ri8:
> > +  case X86::CMP16ri:
> > +  case X86::CMP16ri8:
> > +  case X86::CMP8ri:
> > +    SrcReg = MI->getOperand(0).getReg();
> > +    CmpMask = ~0;
> > +    CmpValue = MI->getOperand(1).getImm();
> > +    return true;
> > +  case X86::CMP64rr:
> > +  case X86::CMP32rr:
> > +  case X86::CMP16rr:
> > +  case X86::CMP8rr:
> > +    SrcReg = MI->getOperand(0).getReg();
> > +    CmpMask = ~0;
> > +    CmpValue = 0;
> > +    return true;
> > +  }
> > +
> > +  return false;
> > +}
> > +
> > +// This function updates condition code for SET Opcodes.
> > +// The input condition code can be E,NE,L,LE,G,GE,B,BE,A,AE and
> > +// the updated condition code will be E,NE,G,GE,L,LE,A,AE,B,BE.
> > +// This is to convert from a > b to b < a, a >= b to b <= a etc.
> > +static unsigned UpdateSETCondToOptimizeCmp(unsigned SETOpc) {
> 
> Update... -> update...
>  
> >
> > +  switch (SETOpc) {
> > +  default: return 0;
> > +  case X86::SETEr:  return X86::SETEr;
> > +  case X86::SETEm:  return X86::SETEm;
> > +  case X86::SETNEr: return X86::SETNEr;
> > +  case X86::SETNEm: return X86::SETNEm;
> > +  case X86::SETLr:  return X86::SETGr;
> > +  case X86::SETLm:  return X86::SETGm;
> > +  case X86::SETLEr: return X86::SETGEr;
> > +  case X86::SETLEm: return X86::SETGEm;
> > +  case X86::SETGr:  return X86::SETLr;
> > +  case X86::SETGm:  return X86::SETLm;
> > +  case X86::SETGEr: return X86::SETLEr;
> > +  case X86::SETGEm: return X86::SETLEm;
> > +  case X86::SETBr:  return X86::SETAr;
> > +  case X86::SETBm:  return X86::SETAm;
> > +  case X86::SETBEr: return X86::SETAEr;
> > +  case X86::SETBEm: return X86::SETAEm;
> > +  case X86::SETAr:  return X86::SETBr;
> > +  case X86::SETAm:  return X86::SETBm;
> > +  case X86::SETAEr: return X86::SETBEr;
> > +  case X86::SETAEm: return X86::SETBEm;
> > +  }
> > +}
> > +
> > +// This function updates condition code for Branch Opcodes.
> > +// The input condition code can be E,NE,L,LE,G,GE,B,BE,A,AE and
> > +// the updated condition code will be E,NE,G,GE,L,LE,A,AE,B,BE.
> > +// This is to convert from a > b to b < a, a >= b to b <= a etc.
> > +static unsigned UpdateBranchCondToOptimizeCmp(unsigned BranchOpc) {
> 
> Update -> update
>  
> >
> > +  switch (BranchOpc) {
> > +  default: return 0;
> > +  case X86::JE_4:  return X86::JE_4;
> > +  case X86::JNE_4: return X86::JNE_4;
> > +  case X86::JL_4:  return X86::JG_4;
> > +  case X86::JLE_4: return X86::JGE_4;
> > +  case X86::JG_4:  return X86::JL_4;
> > +  case X86::JGE_4: return X86::JLE_4;
> > +  case X86::JB_4:  return X86::JA_4;
> > +  case X86::JBE_4: return X86::JAE_4;
> > +  case X86::JA_4:  return X86::JB_4;
> > +  case X86::JAE_4: return X86::JBE_4;
> > +  }
> > +}
> > +
> > +// This function updates condition code for CMOV Opcodes.
> > +// The input condition code can be E,NE,L,LE,G,GE,B,BE,A,AE and
> > +// the updated condition code will be E,NE,G,GE,L,LE,A,AE,B,BE.
> > +// This is to convert from a > b to b < a, a >= b to b <= a etc.
> > +static unsigned UpdateCMovCondToOptimizeCmp(unsigned CMovOpc) {
> > +  switch (CMovOpc) {
> > +  default: return 0;
> > +  case X86::CMOVE16rm:  return X86::CMOVE16rm;
> > +  case X86::CMOVE16rr:  return X86::CMOVE16rr;
> > +  case X86::CMOVE32rm:  return X86::CMOVE32rm;
> > +  case X86::CMOVE32rr:  return X86::CMOVE32rr;
> > +  case X86::CMOVE64rm:  return X86::CMOVE64rm;
> > +  case X86::CMOVE64rr:  return X86::CMOVE64rr;
> > +  case X86::CMOVNE16rm: return X86::CMOVNE16rm;
> > +  case X86::CMOVNE16rr: return X86::CMOVNE16rr;
> > +  case X86::CMOVNE32rm: return X86::CMOVNE32rm;
> > +  case X86::CMOVNE32rr: return X86::CMOVNE32rr;
> > +  case X86::CMOVNE64rm: return X86::CMOVNE64rm;
> > +  case X86::CMOVNE64rr: return X86::CMOVNE64rr;
> > +
> > +  case X86::CMOVL16rm:  return X86::CMOVG16rm;
> > +  case X86::CMOVL16rr:  return X86::CMOVG16rr;
> > +  case X86::CMOVL32rm:  return X86::CMOVG32rm;
> > +  case X86::CMOVL32rr:  return X86::CMOVG32rr;
> > +  case X86::CMOVL64rm:  return X86::CMOVG64rm;
> > +  case X86::CMOVL64rr:  return X86::CMOVG64rr;
> > +  case X86::CMOVLE16rm: return X86::CMOVGE16rm;
> > +  case X86::CMOVLE16rr: return X86::CMOVGE16rr;
> > +  case X86::CMOVLE32rm: return X86::CMOVGE32rm;
> > +  case X86::CMOVLE32rr: return X86::CMOVGE32rr;
> > +  case X86::CMOVLE64rm: return X86::CMOVGE64rm;
> > +  case X86::CMOVLE64rr: return X86::CMOVGE64rr;
> > +
> > +  case X86::CMOVG16rm:  return X86::CMOVL16rm;
> > +  case X86::CMOVG16rr:  return X86::CMOVL16rr;
> > +  case X86::CMOVG32rm:  return X86::CMOVL32rm;
> > +  case X86::CMOVG32rr:  return X86::CMOVL32rr;
> > +  case X86::CMOVG64rm:  return X86::CMOVL64rm;
> > +  case X86::CMOVG64rr:  return X86::CMOVL64rr;
> > +  case X86::CMOVGE16rm: return X86::CMOVLE16rm;
> > +  case X86::CMOVGE16rr: return X86::CMOVLE16rr;
> > +  case X86::CMOVGE32rm: return X86::CMOVLE32rm;
> > +  case X86::CMOVGE32rr: return X86::CMOVLE32rr;
> > +  case X86::CMOVGE64rm: return X86::CMOVLE64rm;
> > +  case X86::CMOVGE64rr: return X86::CMOVLE64rr;
> > +
> > +  case X86::CMOVB16rm:  return X86::CMOVA16rm;
> > +  case X86::CMOVB16rr:  return X86::CMOVA16rr;
> > +  case X86::CMOVB32rm:  return X86::CMOVA32rm;
> > +  case X86::CMOVB32rr:  return X86::CMOVA32rr;
> > +  case X86::CMOVB64rm:  return X86::CMOVA64rm;
> > +  case X86::CMOVB64rr:  return X86::CMOVA64rr;
> > +  case X86::CMOVBE16rm: return X86::CMOVAE16rm;
> > +  case X86::CMOVBE16rr: return X86::CMOVAE16rr;
> > +  case X86::CMOVBE32rm: return X86::CMOVAE32rm;
> > +  case X86::CMOVBE32rr: return X86::CMOVAE32rr;
> > +  case X86::CMOVBE64rm: return X86::CMOVAE64rm;
> > +  case X86::CMOVBE64rr: return X86::CMOVAE64rr;
> > +
> > +  case X86::CMOVA16rm:  return X86::CMOVB16rm;
> > +  case X86::CMOVA16rr:  return X86::CMOVB16rr;
> > +  case X86::CMOVA32rm:  return X86::CMOVB32rm;
> > +  case X86::CMOVA32rr:  return X86::CMOVB32rr;
> > +  case X86::CMOVA64rm:  return X86::CMOVB64rm;
> > +  case X86::CMOVA64rr:  return X86::CMOVB64rr;
> > +  case X86::CMOVAE16rm: return X86::CMOVBE16rm;
> > +  case X86::CMOVAE16rr: return X86::CMOVBE16rr;
> > +  case X86::CMOVAE32rm: return X86::CMOVBE32rm;
> > +  case X86::CMOVAE32rr: return X86::CMOVBE32rr;
> > +  case X86::CMOVAE64rm: return X86::CMOVBE64rm;
> > +  case X86::CMOVAE64rr: return X86::CMOVBE64rr;
> > +  }
> > +}
> > +
> > +bool X86InstrInfo::
> > +OptimizeCompareInstr(MachineInstr *CmpInstr, unsigned SrcReg, int CmpMask,
> 
> Optimize -> optimize
>  
> >
> > +                     int CmpValue, const MachineRegisterInfo *MRI) const {
> > +  MachineRegisterInfo::def_iterator DI = MRI->def_begin(SrcReg);
> > +  if (llvm::next(DI) != MRI->def_end())
> > +    // Only support one definition.
> > +    return false;
> > +
> > +  MachineInstr *MI = &*DI;
> > +  // Get ready to iterate backward from CmpInstr.
> > +  MachineBasicBlock::iterator I = CmpInstr, E = MI,
> > +                              B = CmpInstr->getParent()->begin();
> 
> Reverse iterators please. Iterating backwards is too hard to follow otherwise. Add the iterators if they're missing.
>  
> >
> > +
> > +  // Early exit if CmpInstr is at the beginning of the BB.
> > +  if (I == B) return false;
> 
> Set up your loop variables below, where you're looping. You shouldn't need to do early exits etc, the loop simply won't iterate...
>  
> >
> > +
> > +  // For CMPrr(r1,r2), we are looking for SUB(r1,r2) or SUB(r2,r1).
> > +  // For CMPri(r1, CmpValue), we are looking for SUBri(r1, CmpValue).
> > +  MachineInstr *Sub = NULL;
> > +  unsigned SrcReg2 = 0;
> > +  if (CmpInstr->getOpcode() == X86::CMP64rr ||
> > +      CmpInstr->getOpcode() == X86::CMP32rr ||
> > +      CmpInstr->getOpcode() == X86::CMP16rr ||
> > +      CmpInstr->getOpcode() == X86::CMP8rr) {
> > +    SrcReg2 = CmpInstr->getOperand(1).getReg();
> > +  }
> > +
> > +  // Search backwards for a SUB instruction.
> > +  // If EFLAGS is updated in the middle, we can not remove the CMP instruction.
> > +  --I;
> > +  for (; I != E; --I) {
> > +    const MachineInstr &Instr = *I;
> > +
> > +    // Check whether the current instruction is SUB(r1, r2) or SUB(r2, r1).
> > +    if (((CmpInstr->getOpcode() == X86::CMP64rr &&
> > +          Instr.getOpcode() == X86::SUB64rr) ||
> > +         (CmpInstr->getOpcode() == X86::CMP32rr &&
> > +          Instr.getOpcode() == X86::SUB32rr)||
> > +         (CmpInstr->getOpcode() == X86::CMP16rr &&
> > +          Instr.getOpcode() == X86::SUB16rr)||
> > +         (CmpInstr->getOpcode() == X86::CMP8rr &&
> > +          Instr.getOpcode() == X86::SUB8rr)) &&
> > +        ((Instr.getOperand(1).getReg() == SrcReg &&
> > +          Instr.getOperand(2).getReg() == SrcReg2) ||
> > +         (Instr.getOperand(1).getReg() == SrcReg2 &&
> > +          Instr.getOperand(2).getReg() == SrcReg))) {
> > +      Sub = &*I;
> > +      break;
> > +    }
> > +
> > +    // Check whether the current instruction is SUBri(r1, CmpValue).
> > +    if (((CmpInstr->getOpcode() == X86::CMP64ri32 &&
> > +          Instr.getOpcode() == X86::SUB64ri32) ||
> > +         (CmpInstr->getOpcode() == X86::CMP64ri8 &&
> > +          Instr.getOpcode() == X86::SUB64ri8) ||
> > +         (CmpInstr->getOpcode() == X86::CMP32ri &&
> > +          Instr.getOpcode() == X86::SUB32ri) ||
> > +         (CmpInstr->getOpcode() == X86::CMP32ri8 &&
> > +          Instr.getOpcode() == X86::SUB32ri8) ||
> > +         (CmpInstr->getOpcode() == X86::CMP16ri &&
> > +          Instr.getOpcode() == X86::SUB16ri) ||
> > +         (CmpInstr->getOpcode() == X86::CMP16ri8 &&
> > +          Instr.getOpcode() == X86::SUB16ri8) ||
> > +         (CmpInstr->getOpcode() == X86::CMP8ri &&
> > +          Instr.getOpcode() == X86::SUB8ri)) &&
> > +        CmpValue != 0 &&
> > +        Instr.getOperand(1).getReg() == SrcReg &&
> > +        Instr.getOperand(2).getImm() == CmpValue) {
> > +      Sub = &*I;
> > +      break;
> > +    }
> > +
> > +    for (unsigned IO = 0, EO = Instr.getNumOperands(); IO != EO; ++IO) {
> > +      const MachineOperand &MO = Instr.getOperand(IO);
> > +      if (MO.isRegMask() && MO.clobbersPhysReg(X86::EFLAGS))
> > +        return false;
> > +      if (!MO.isReg()) continue;
> > +
> > +      // This instruction modifies or uses EFLAGS before we find a SUB.
> > +      // We can't do this transformation.
> > +      if (MO.getReg() == X86::EFLAGS)
> > +        return false;
> > +    }
> 
> Please hoist this loop into a helper function. Ideally on MachineInstr so others can use it. It seems generically useful.
>  
> >
> > +
> > +    if (I == B)
> > +      // Reaching beginning of the block.
> > +      return false;
> > +  }
> > +
> > +  // Return false if no candidates exist.
> > +  if (!Sub)
> > +    return false;
> > +  MI = Sub;
> > +
> > +  switch (MI->getOpcode()) {
> > +  default: break;
> > +  case X86::SUB64rr:
> > +  case X86::SUB32rr:
> > +  case X86::SUB16rr:
> > +  case X86::SUB8rr:
> > +  case X86::SUB64ri32:
> > +  case X86::SUB64ri8:
> > +  case X86::SUB32ri:
> > +  case X86::SUB32ri8:
> > +  case X86::SUB16ri:
> > +  case X86::SUB16ri8:
> > +  case X86::SUB8ri: {
> > +    // Scan forward from CmpInstr for the use of EFLAGS.
> > +    // Handle the condition codes GE, L, G, LE, B, L, BE, LE.
> > +    SmallVector<std::pair<MachineInstr*, unsigned /*NewOpc*/>, 4> OpsToUpdate;
> > +    bool isSafe = false;
> 
> isSafe -> IsSafe.
> 
> > +    I = CmpInstr;
> > +    E = CmpInstr->getParent()->end();
> > +    while (!isSafe && ++I != E) {
> 
> Don't re-use loop variables this way. It's much better to declare them in the for (...) loop they are used for, and declare new ones in each loop.
> 
> > +      const MachineInstr &Instr = *I;
> > +      for (unsigned IO = 0, EO = Instr.getNumOperands();
> > +           !isSafe && IO != EO; ++IO) {
> > +        const MachineOperand &MO = Instr.getOperand(IO);
> > +        if (MO.isRegMask() && MO.clobbersPhysReg(X86::EFLAGS)) {
> > +          isSafe = true;
> > +          break;
> 
> This whole thing would be a lot more clear if you hoisted this into a helper function and used early exit.
> 
> > +        }
> > +        if (!MO.isReg() || MO.getReg() != X86::EFLAGS)
> > +          continue;
> > +        // EFLAGS is redefined by this instruction.
> > +        if (MO.isDef()) {
> > +          isSafe = true;
> > +          break;
> > +        }
> > +        // EFLAGS is used by this instruction.
> > +
> > +        // 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.
> > +        unsigned NewOpc = UpdateBranchCondToOptimizeCmp(Instr.getOpcode());
> > +        if (!NewOpc) NewOpc = UpdateSETCondToOptimizeCmp(Instr.getOpcode());
> > +        if (!NewOpc) NewOpc = UpdateCMovCondToOptimizeCmp(Instr.getOpcode());
> > +        if (!NewOpc) return false;
> > +
> > +        // Push the MachineInstr to OpsToUpdate.
> > +        // If it is safe to remove CmpInstr, the condition code of these
> > +        // instructions will be modified.
> > +        if (SrcReg2 != 0 && Sub->getOperand(1).getReg() == SrcReg2 &&
> > +            Sub->getOperand(2).getReg() == SrcReg)
> > +          OpsToUpdate.push_back(std::make_pair(&*I, NewOpc));
> > +      }
> > +    }
> > +
> > +    // We may exit the loop at end of the basic block.
> > +    // In that case, it is still safe to remove CmpInstr.
> > +
> > +    // Make sure Sub instruction defines EFLAGS.
> > +    assert(MI->getOperand(3).getReg() == X86::EFLAGS &&
> > +           "Expected EFLAGS is the 4th operand of SUBrr or SUBri.");
> > +    MI->getOperand(3).setIsDef(true);
> > +    CmpInstr->eraseFromParent();
> > +
> > +    // Modify the condition code of instructions in OpsToUpdate.
> > +    for (unsigned i = 0; i < OpsToUpdate.size(); i++)
> 
> Always cache the end value in a loop variable.
> 
> 
> > +      OpsToUpdate[i].first->setDesc(get(OpsToUpdate[i].second));
> > +    NumCmpsRemoved++;
> > +    return true;
> > +  }
> > +  }
> > +
> > +  return false;
> > +}
> > +
> > +bool X86InstrInfo::
> >  OptimizeSubInstr(MachineInstr *SubInstr, const MachineRegisterInfo *MRI) const {
> >   // If destination is a memory operand, do not perform this optimization.
> >   if ((SubInstr->getOpcode() != X86::SUB64rr) &&
> >
> > Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.h
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.h?rev=157831&r1=157830&r2=157831&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Target/X86/X86InstrInfo.h (original)
> > +++ llvm/trunk/lib/Target/X86/X86InstrInfo.h Fri Jun  1 14:49:33 2012
> > @@ -367,6 +367,12 @@
> >   virtual bool OptimizeSubInstr(MachineInstr *SubInstr,
> >                                 const MachineRegisterInfo *MRI) const;
> >
> > +  virtual bool AnalyzeCompare(const MachineInstr *MI, unsigned &SrcReg,
> > +                              int &CmpMask, int &CmpValue) const;
> > +  virtual bool OptimizeCompareInstr(MachineInstr *CmpInstr, unsigned SrcReg,
> > +                                    int CmpMask, int CmpValue,
> > +                                    const MachineRegisterInfo *MRI) const;
> > +
> >  private:
> >   MachineInstr * convertToThreeAddressWithLEA(unsigned MIOpc,
> >                                               MachineFunction::iterator &MFI,
> >
> > 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=157831&r1=157830&r2=157831&view=diff
> > ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/jump_sign.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/jump_sign.ll Fri Jun  1 14:49:33 2012
> > @@ -83,6 +83,25 @@
> >   %cond = select i1 %cmp, i32 %sub, i32 0
> >   ret i32 %cond
> >  }
> > +; redundant cmp instruction
> > +define i32 @l(i32 %a, i32 %b) nounwind {
> > +entry:
> > +; CHECK: l:
> > +; CHECK-NOT: cmp
> > +  %cmp = icmp slt i32 %b, %a
> > +  %sub = sub nsw i32 %a, %b
> > +  %cond = select i1 %cmp, i32 %sub, i32 %a
> > +  ret i32 %cond
> > +}
> > +define i32 @m(i32 %a, i32 %b) nounwind {
> > +entry:
> > +; CHECK: m:
> > +; CHECK-NOT: cmp
> > +  %cmp = icmp sgt i32 %a, %b
> > +  %sub = sub nsw i32 %a, %b
> > +  %cond = select i1 %cmp, i32 %b, i32 %sub
> > +  ret i32 %cond
> > +}
> >  ; rdar://11540023
> >  define i64 @n(i64 %x, i64 %y) nounwind {
> >  entry:
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120627/a880ce9d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: peephole.patch
Type: application/octet-stream
Size: 26395 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120627/a880ce9d/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120627/a880ce9d/attachment-0001.html>


More information about the llvm-commits mailing list