diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -4288,75 +4288,84 @@ bool ShouldUpdateCC = false; X86::CondCode NewCC = X86::COND_INVALID; - // Search backward from CmpInstr for the next instruction defining EFLAGS. + // Search backward from CmpInstr for the next instruction producing EFLAGS. const TargetRegisterInfo *TRI = &getRegisterInfo(); MachineBasicBlock &CmpMBB = *CmpInstr.getParent(); MachineBasicBlock::reverse_iterator From = std::next(MachineBasicBlock::reverse_iterator(CmpInstr)); MachineInstr *SrcRegDef = MRI->getVRegDef(SrcReg); assert(SrcRegDef && "Must have a definition (SSA)"); - for (MachineInstr &Inst : make_range(From, CmpMBB.rend())) { - // Can we use EFLAGS from instr defining %SrcReg? Example: - // %eax = addl ... - // ... // EFLAGS not changed - // testl %eax, %eax // <-- can be removed - if (&Inst == SrcRegDef) { - if (IsCmpZero && isDefConvertible(Inst, NoSignFlag, ClearsOverflowFlag)) { - MI = &Inst; - break; - } - // Cannot find other candidates before definition of SrcReg. - return false; - } - - if (Inst.modifiesRegister(X86::EFLAGS, TRI)) { - // Can we reuse EFLAGS produces by insn reading %SrcReg. Example: - // %eax = ... - // ... - // popcntl %eax - // ... // EFLAGS not changed - // testl %eax, %eax // <-- can be removed - if (IsCmpZero) { - NewCC = isUseDefConvertible(Inst); - if (NewCC != X86::COND_INVALID && Inst.getOperand(1).isReg() && - Inst.getOperand(1).getReg() == SrcReg) { - ShouldUpdateCC = true; + for (MachineBasicBlock *MBB = &CmpMBB;;) { + for (MachineInstr &Inst : make_range(From, MBB->rend())) { + // Can we use EFLAGS from instr defining %SrcReg? Example: + // %eax = addl ... + // ... // EFLAGS not changed + // testl %eax, %eax // <-- can be removed + if (&Inst == SrcRegDef) { + if (IsCmpZero && + isDefConvertible(Inst, NoSignFlag, ClearsOverflowFlag)) { MI = &Inst; break; } + // Cannot find other candidates before definition of SrcReg. + return false; } - // Can we reuse results of equivalent cmp or sub? Example: - // sub x, y or cmp x, y - // ... // EFLAGS not changed - // cmp x, y // <-- can be removed - if (!IsCmpZero && isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpMask, - CmpValue, Inst)) { - Sub = &Inst; - break; - } + if (Inst.modifiesRegister(X86::EFLAGS, TRI)) { + // Can we reuse EFLAGS produces by insn reading %SrcReg. Example: + // %eax = ... + // ... + // popcntl %eax + // ... // EFLAGS not changed + // testl %eax, %eax // <-- can be removed + if (IsCmpZero) { + NewCC = isUseDefConvertible(Inst); + if (NewCC != X86::COND_INVALID && Inst.getOperand(1).isReg() && + Inst.getOperand(1).getReg() == SrcReg) { + ShouldUpdateCC = true; + MI = &Inst; + break; + } + } - // MOV32r0 is implemented with xor which clobbers condition code. It is - // safe to move up, if the definition to EFLAGS is dead and earlier - // instructions do not read or write EFLAGS. - if (!Movr0Inst && Inst.getOpcode() == X86::MOV32r0 && - Inst.registerDefIsDead(X86::EFLAGS, TRI)) { - Movr0Inst = &Inst; - continue; + // Can we reuse results of equivalent cmp or sub? Example: + // sub x, y or cmp x, y + // ... // EFLAGS not changed + // cmp x, y // <-- can be removed + if (!IsCmpZero && isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, + CmpMask, CmpValue, Inst)) { + Sub = &Inst; + break; + } + + // MOV32r0 is implemented with xor which clobbers condition code. It is + // safe to move up, if the definition to EFLAGS is dead and earlier + // instructions do not read or write EFLAGS. + if (!Movr0Inst && Inst.getOpcode() == X86::MOV32r0 && + Inst.registerDefIsDead(X86::EFLAGS, TRI)) { + Movr0Inst = &Inst; + continue; + } + + // Cannot do anything for any other EFLAG changes. + return false; } - // Cannot do anything for any other EFLAG changes. - return false; + if (Inst.readsRegister(X86::EFLAGS, TRI)) + return false; } - if (Inst.readsRegister(X86::EFLAGS, TRI)) + if (MI || Sub) + break; + + // Reached begin of basic block. Continue in predecessor if there is + // exactly one. + if (MBB->pred_size() != 1) return false; + MBB = *MBB->pred_begin(); + From = MBB->rbegin(); } - // Return false if no candidates exist. - if (!MI && !Sub) - return false; - bool IsSwapped = (SrcReg2 != 0 && Sub && Sub->getOperand(1).getReg() == SrcReg2 && Sub->getOperand(2).getReg() == SrcReg); @@ -4461,8 +4470,13 @@ // The instruction to be updated is either Sub or MI. Sub = IsCmpZero ? MI : Sub; + MachineBasicBlock *SubBB = Sub->getParent(); // Move Movr0Inst to the appropriate place before Sub. if (Movr0Inst) { + // Only move within the same block so we don't accidentally move to a + // block with higher execution frequency. + if (&CmpMBB != SubBB) + return false; // Look backwards until we find a def that doesn't use the current EFLAGS. MachineBasicBlock::reverse_iterator InsertI = Sub, InsertE = Sub->getParent()->rend(); @@ -4470,7 +4484,7 @@ MachineInstr *Instr = &*InsertI; if (!Instr->readsRegister(X86::EFLAGS, TRI) && Instr->modifiesRegister(X86::EFLAGS, TRI)) { - Sub->getParent()->remove(Movr0Inst); + Movr0Inst->getParent()->remove(Movr0Inst); Instr->getParent()->insert(MachineBasicBlock::iterator(Instr), Movr0Inst); break; @@ -4492,6 +4506,13 @@ Op.first->getOperand(Op.first->getDesc().getNumOperands() - 1) .setImm(Op.second); } + // Add EFLAGS to block live-ins between CmpBB and block of flags producer. + for (MachineBasicBlock *MBB = &CmpMBB; MBB != SubBB; + MBB = *MBB->pred_begin()) { + assert(MBB->pred_size() == 1 && "Expected exactly one predecessor"); + if (!MBB->isLiveIn(X86::EFLAGS)) + MBB->addLiveIn(X86::EFLAGS); + } return true; } diff --git a/llvm/test/CodeGen/X86/optimize-compare.mir b/llvm/test/CodeGen/X86/optimize-compare.mir --- a/llvm/test/CodeGen/X86/optimize-compare.mir +++ b/llvm/test/CodeGen/X86/optimize-compare.mir @@ -32,6 +32,76 @@ $al = SETCCr 4, implicit $eflags ... --- +name: opt_zerocmp_2 +tracksRegLiveness: true +body: | + ; CHECK-LABEL: name: opt_zerocmp_2 + ; CHECK: bb.0: + ; CHECK-NEXT: successors: %bb.1(0x80000000) + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY undef $rsi + ; CHECK-NEXT: [[INC64r:%[0-9]+]]:gr64 = INC64r [[COPY]], implicit-def $eflags + ; CHECK-NEXT: PUSH64r undef $rcx, implicit-def $rsp, implicit $rsp + ; CHECK-NEXT: $rcx = POP64r implicit-def $rsp, implicit $rsp + ; CHECK-NEXT: JMP_1 %bb.1 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.1: + ; CHECK-NEXT: liveins: $eflags + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[LEA64r:%[0-9]+]]:gr64 = LEA64r [[INC64r]], 5, $noreg, 12, $noreg + ; CHECK-NEXT: $al = SETCCr 4, implicit $eflags + bb.0: + %0:gr64 = COPY undef $rsi + %1:gr64 = INC64r %0, implicit-def $eflags + PUSH64r undef $rcx, implicit-def $rsp, implicit $rsp + $rcx = POP64r implicit-def $rsp, implicit $rsp + JMP_1 %bb.1 + + bb.1: + ; TEST should be removed. + TEST64rr %1, %1, implicit-def $eflags + %2:gr64 = LEA64r %1, 5, $noreg, 12, $noreg + $al = SETCCr 4, implicit $eflags +... +--- +name: opt_zerocmp_noopt +body: | + ; CHECK-LABEL: name: opt_zerocmp_noopt + ; CHECK: bb.0: + ; CHECK-NEXT: successors: %bb.1(0x80000000) + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY undef $rsi + ; CHECK-NEXT: [[INC64r:%[0-9]+]]:gr64 = INC64r [[COPY]], implicit-def $eflags + ; CHECK-NEXT: PUSH64r undef $rcx, implicit-def $rsp, implicit $rsp + ; CHECK-NEXT: $rcx = POP64r implicit-def $rsp, implicit $rsp + ; CHECK-NEXT: JMP_1 %bb.1 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.1: + ; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000) + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: TEST64rr [[INC64r]], [[INC64r]], implicit-def $eflags + ; CHECK-NEXT: [[LEA64r:%[0-9]+]]:gr64 = LEA64r [[INC64r]], 5, $noreg, 12, $noreg + ; CHECK-NEXT: $al = SETCCr 4, implicit $eflags + ; CHECK-NEXT: JCC_1 %bb.1, 2, implicit $eflags + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.2: + bb.0: + %0:gr64 = COPY undef $rsi + %1:gr64 = INC64r %0, implicit-def $eflags + PUSH64r undef $rcx, implicit-def $rsp, implicit $rsp + $rcx = POP64r implicit-def $rsp, implicit $rsp + JMP_1 %bb.1 + + bb.1: + ; The TEST64rr should not be removed, since there are multiple preds. + TEST64rr %1, %1, implicit-def $eflags + %2:gr64 = LEA64r %1, 5, $noreg, 12, $noreg + $al = SETCCr 4, implicit $eflags + JCC_1 %bb.1, 2, implicit $eflags + + bb.2: +... +--- name: opt_zerocmp_user_0 body: | bb.0: