Skip to content

Commit

Permalink
X86: Teach X86InstrInfo::analyzeCompare to recognize compares of symb…
Browse files Browse the repository at this point in the history
…ols.

This requires that we communicate to X86InstrInfo::optimizeCompareInstr
that the second operand is neither a register nor an immediate. The way we
do that is by setting CmpMask to zero.

Note that there were already instructions where the second operand was not a
register nor an immediate, namely X86::SUB*rm, so also set CmpMask to zero
for those instructions. This seems like a latent bug, but I was unable to
trigger it.

Differential Revision: https://reviews.llvm.org/D28621

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294634 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
pcc committed Feb 9, 2017
1 parent bf8cee8 commit ce00de8
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
39 changes: 22 additions & 17 deletions lib/Target/X86/X86InstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6486,12 +6486,14 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
case X86::CMP16ri:
case X86::CMP16ri8:
case X86::CMP8ri:
if (!MI.getOperand(1).isImm())
return false;
SrcReg = MI.getOperand(0).getReg();
SrcReg2 = 0;
CmpMask = ~0;
CmpValue = MI.getOperand(1).getImm();
if (MI.getOperand(1).isImm()) {
CmpMask = ~0;
CmpValue = MI.getOperand(1).getImm();
} else {
CmpMask = CmpValue = 0;
}
return true;
// A SUB can be used to perform comparison.
case X86::SUB64rm:
Expand All @@ -6500,7 +6502,7 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
case X86::SUB8rm:
SrcReg = MI.getOperand(1).getReg();
SrcReg2 = 0;
CmpMask = ~0;
CmpMask = 0;
CmpValue = 0;
return true;
case X86::SUB64rr:
Expand All @@ -6509,7 +6511,7 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
case X86::SUB8rr:
SrcReg = MI.getOperand(1).getReg();
SrcReg2 = MI.getOperand(2).getReg();
CmpMask = ~0;
CmpMask = 0;
CmpValue = 0;
return true;
case X86::SUB64ri32:
Expand All @@ -6519,20 +6521,22 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
case X86::SUB16ri:
case X86::SUB16ri8:
case X86::SUB8ri:
if (!MI.getOperand(2).isImm())
return false;
SrcReg = MI.getOperand(1).getReg();
SrcReg2 = 0;
CmpMask = ~0;
CmpValue = MI.getOperand(2).getImm();
if (MI.getOperand(2).isImm()) {
CmpMask = ~0;
CmpValue = MI.getOperand(2).getImm();
} else {
CmpMask = CmpValue = 0;
}
return true;
case X86::CMP64rr:
case X86::CMP32rr:
case X86::CMP16rr:
case X86::CMP8rr:
SrcReg = MI.getOperand(0).getReg();
SrcReg2 = MI.getOperand(1).getReg();
CmpMask = ~0;
CmpMask = 0;
CmpValue = 0;
return true;
case X86::TEST8rr:
Expand All @@ -6558,8 +6562,8 @@ bool X86InstrInfo::analyzeCompare(const MachineInstr &MI, unsigned &SrcReg,
/// SrcReg, SrcRegs: register operands for FlagI.
/// ImmValue: immediate for FlagI if it takes an immediate.
inline static bool isRedundantFlagInstr(MachineInstr &FlagI, unsigned SrcReg,
unsigned SrcReg2, int ImmValue,
MachineInstr &OI) {
unsigned SrcReg2, int ImmMask,
int ImmValue, MachineInstr &OI) {
if (((FlagI.getOpcode() == X86::CMP64rr && OI.getOpcode() == X86::SUB64rr) ||
(FlagI.getOpcode() == X86::CMP32rr && OI.getOpcode() == X86::SUB32rr) ||
(FlagI.getOpcode() == X86::CMP16rr && OI.getOpcode() == X86::SUB16rr) ||
Expand All @@ -6570,7 +6574,8 @@ inline static bool isRedundantFlagInstr(MachineInstr &FlagI, unsigned SrcReg,
OI.getOperand(2).getReg() == SrcReg)))
return true;

if (((FlagI.getOpcode() == X86::CMP64ri32 &&
if (ImmMask != 0 &&
((FlagI.getOpcode() == X86::CMP64ri32 &&
OI.getOpcode() == X86::SUB64ri32) ||
(FlagI.getOpcode() == X86::CMP64ri8 &&
OI.getOpcode() == X86::SUB64ri8) ||
Expand Down Expand Up @@ -6757,7 +6762,7 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,

// 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);
bool IsCmpZero = (CmpMask != 0 && CmpValue == 0);
if (IsCmpZero && MI->getParent() != CmpInstr.getParent())
return false;

Expand Down Expand Up @@ -6807,8 +6812,8 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, unsigned SrcReg,
for (; RI != RE; ++RI) {
MachineInstr &Instr = *RI;
// Check whether CmpInstr can be made redundant by the current instruction.
if (!IsCmpZero &&
isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpValue, Instr)) {
if (!IsCmpZero && isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpMask,
CmpValue, Instr)) {
Sub = &Instr;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion test/CodeGen/X86/compare-global.ll
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ target triple = "i686-pc-windows-msvc18.0.0"

define void @f(i8* %c) {
entry:
; CHECK: subl $_foo, %eax
; CHECK: cmpl $_foo, 4(%esp)
%cmp = icmp eq i8* %c, @foo
br i1 %cmp, label %if.then, label %if.end

Expand Down

0 comments on commit ce00de8

Please sign in to comment.