Index: lib/Target/AArch64/AArch64InstrInfo.cpp =================================================================== --- lib/Target/AArch64/AArch64InstrInfo.cpp +++ lib/Target/AArch64/AArch64InstrInfo.cpp @@ -929,11 +929,162 @@ return false; } +/// Find the first modification of AArch64::NZCV after the specified instruction. +static MachineBasicBlock::iterator +firstModificationOfCondFlagsAfterInstr(MachineInstr *Instr, + const TargetRegisterInfo *TRI) { + assert(Instr); + return std::find_if(std::next(Instr->getIterator()), Instr->getParent()->instr_end(), + [TRI](MachineInstr &MI) { + return MI.modifiesRegister(AArch64::NZCV, TRI); + }); +} + +/// Find a condition code used by the instruction. +/// Returns AArch64CC::Invalid if either the instruction does not use condition +/// codes or we don't optimize CmpInstr in the presence of such instructions. +static AArch64CC::CondCode findCondCodeUsedByInstr(MachineInstr &Instr) { + switch (Instr.getOpcode()) { + default: + return AArch64CC::Invalid; + + case AArch64::Bcc: { + int Idx = Instr.findRegisterUseOperandIdx(AArch64::NZCV); + assert(Idx >= 2); + return static_cast(Instr.getOperand(Idx - 2).getImm()); + } + + case AArch64::CSINVWr: + case AArch64::CSINVXr: + case AArch64::CSINCWr: + case AArch64::CSINCXr: + case AArch64::CSELWr: + case AArch64::CSELXr: + case AArch64::CSNEGWr: + case AArch64::CSNEGXr: + case AArch64::FCSELSrrr: + case AArch64::FCSELDrrr: { + int Idx = Instr.findRegisterUseOperandIdx(AArch64::NZCV); + assert(Idx >= 1); + return static_cast(Instr.getOperand(Idx - 1).getImm()); + } + } +} + +/// Check if Instr can produce a needed condition code. +static bool canInstrProduceCondCode(MachineInstr &Instr, + AArch64CC::CondCode NeededCondCode) { + switch (NeededCondCode) { + default: + return false; + + case AArch64CC::NE: { + switch (Instr.getOpcode()) { + default: + break; + case AArch64::ADDWri: + case AArch64::ADDXri: + case AArch64::ADDSWri: + case AArch64::ADDSXri: + return Instr.getOperand(1).isFI() + && (Instr.getOperand(2).getImm() == 0); + } + break; + } + } + return false; +} + +/// Find what a condition code is used after CmpInstr. +/// Returns AArch64CC::Invalid: +/// 1. if no instruction uses condition codes +/// 2. or a condition code is used but the instruction is not recognized +/// 3. or different condition codes are used after CmpInstr +static AArch64CC::CondCode findUsedCondCodeAfterCmp(MachineInstr *CmpInstr, + const TargetRegisterInfo *TRI) { + assert(CmpInstr); + assert(TRI); + auto EndLoc = firstModificationOfCondFlagsAfterInstr(CmpInstr, TRI).getInstrIterator(); + // There are instructions which can read and write flags at the same time. + if (EndLoc != CmpInstr->getParent()->instr_end() + && EndLoc->readsRegister(AArch64::NZCV, TRI)) { + EndLoc = std::next(EndLoc); + } + auto CFlagsFirstReadLoc + = std::find_if(std::next(CmpInstr->getIterator()), + EndLoc, + [TRI](MachineInstr &MI) { + return MI.readsRegister(AArch64::NZCV, TRI); + }); + if (CFlagsFirstReadLoc == EndLoc) + return AArch64CC::Invalid; + + AArch64CC::CondCode UsedCondCode = findCondCodeUsedByInstr(*CFlagsFirstReadLoc); + if (UsedCondCode == AArch64CC::Invalid) + return AArch64CC::Invalid; + + // Check that all other used condition codes are the same. + // If they are not it might not be safe to remove CmpInstr. + if (!std::all_of(std::next(CFlagsFirstReadLoc), EndLoc, + [UsedCondCode, TRI](MachineInstr &MI) { + return !MI.readsRegister(AArch64::NZCV, TRI) + || (UsedCondCode == findCondCodeUsedByInstr(MI)); + })) + return AArch64CC::Invalid; + + return UsedCondCode; +} + +static bool isADDS(unsigned Opcode) { + return AArch64::ADDSWri <= Opcode && Opcode <= AArch64::ADDSXrx64; +} + +static bool isSUBS(unsigned Opcode) { + return AArch64::SUBSWri <= Opcode && Opcode <= AArch64::SUBSXrx64; +} + +template bool sameSForm(unsigned CmpOpcode, + unsigned CandidateOpcode, IsOpSFuncT IsOpS) { + return IsOpS(CmpOpcode) && IsOpS(CandidateOpcode); +} + +// Check if CmpInstr can be substituted by MI. +static bool canInstrSubstituteCmpInstr(MachineInstr *MI, MachineInstr *CmpInstr, + const TargetRegisterInfo *TRI) { + assert(MI); + assert(CmpInstr); + + AArch64CC::CondCode UsedCondCode = findUsedCondCodeAfterCmp(CmpInstr, TRI); + switch (UsedCondCode) { + default: + return false; + + case AArch64CC::EQ: + case AArch64CC::NE: + case AArch64CC::MI: + case AArch64CC::PL: { + unsigned MiSOpcode = sForm(*MI); + assert(MiSOpcode != AArch64::INSTRUCTION_LIST_END); + if (sameSForm(CmpInstr->getOpcode(), MiSOpcode, isADDS)) + return true; + if (sameSForm(CmpInstr->getOpcode(), MiSOpcode, isSUBS)) + return true; + if (canInstrProduceCondCode(*MI, UsedCondCode)) + return true; + break; + } + } + + return false; +} + /// Substitute CmpInstr with another instruction which produces a needed /// condition code. /// Return true on success. bool AArch64InstrInfo::substituteCmpInstr(MachineInstr *CmpInstr, unsigned SrcReg, const MachineRegisterInfo *MRI) const { + assert(CmpInstr); + assert(MRI); // Get the unique definition of SrcReg. MachineInstr *MI = MRI->getUniqueVRegDef(SrcReg); if (!MI) @@ -947,73 +1098,10 @@ if (NewOpc == AArch64::INSTRUCTION_LIST_END) return false; - // Scan forward for the use of NZCV. - // When checking against MI: if it's a conditional code requires - // checking of V bit, then this is not safe to do. - // It is safe to remove CmpInstr if NZCV is redefined or killed. - // If we are done with the basic block, we need to check whether NZCV is - // live-out. - bool IsSafe = false; - for (MachineBasicBlock::iterator I = CmpInstr, - E = CmpInstr->getParent()->end(); - !IsSafe && ++I != E;) { - 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(AArch64::NZCV)) { - IsSafe = true; - break; - } - if (!MO.isReg() || MO.getReg() != AArch64::NZCV) - continue; - if (MO.isDef()) { - IsSafe = true; - break; - } - - // Decode the condition code. - unsigned Opc = Instr.getOpcode(); - AArch64CC::CondCode CC; - switch (Opc) { - default: - return false; - case AArch64::Bcc: - CC = (AArch64CC::CondCode)Instr.getOperand(IO - 2).getImm(); - break; - case AArch64::CSINVWr: - case AArch64::CSINVXr: - case AArch64::CSINCWr: - case AArch64::CSINCXr: - case AArch64::CSELWr: - case AArch64::CSELXr: - case AArch64::CSNEGWr: - case AArch64::CSNEGXr: - case AArch64::FCSELSrrr: - case AArch64::FCSELDrrr: - CC = (AArch64CC::CondCode)Instr.getOperand(IO - 1).getImm(); - break; - } - - // It is not safe to remove Compare instruction if Overflow(V) is used. - switch (CC) { - default: - // NZCV can be used multiple times, we should continue. - break; - case AArch64CC::VS: - case AArch64CC::VC: - case AArch64CC::GE: - case AArch64CC::LT: - case AArch64CC::GT: - case AArch64CC::LE: - return false; - } - } - } + if (areCFlagsAliveInSuccessors(CmpInstr->getParent())) + return false; - // If NZCV is not killed nor re-defined, we should check whether it is - // live-out. If it is live-out, do not optimize. - if (!IsSafe && areCFlagsAliveInSuccessors(CmpInstr->getParent())) + if (!canInstrSubstituteCmpInstr(MI, CmpInstr, TRI)) return false; // Update the instruction to set NZCV. Index: test/CodeGen/AArch64/arm64-regress-opt-cmp.ll =================================================================== --- /dev/null +++ test/CodeGen/AArch64/arm64-regress-opt-cmp.ll @@ -0,0 +1,33 @@ +; RUN: llc -mtriple=aarch64-linux-gnu -O3 -o - %s | FileCheck %s + +@d = internal global [4 x i8] c"\01\00\00\00", align 1 +@c = internal global i8 2, align 1 + +declare void @a(i32) + +; A regression tests for PR27158: AArch64InstrInfo::optimizeCompareInstr incorrectly removes compare instructions +; 'and+cmp' should be replaced be 'ands'. +define i32 @test01() nounwind { +; CHECK: lsl {{.*}} +; CHECK-NEXT: and {{.*}} +; CHECK-NEXT: cmp {{.*}} +entry: + %0 = load i8, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @d, i64 0, i64 3), align 1 + store i8 %0, i8* @c, align 1 + %1 = load i8, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @d, i64 0, i64 1), align 1 + %conv = zext i8 %1 to i32 + %2 = load i8, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @d, i64 0, i64 2), align 1 + %conv1 = zext i8 %2 to i32 + %shl = shl i32 %conv, %conv1 + %conv3 = and i32 %shl, 65535 + %cmp = icmp ult i32 %conv3, zext (i1 icmp eq (i8* getelementptr inbounds ([4 x i8], [4 x i8]* @d, i64 0, i64 3), i8* @c) to i32) + br i1 %cmp, label %if.end, label %if.then + +if.then: + call void @a(i32 0) + br label %if.end + +if.end: + ret i32 0 +} + Index: test/CodeGen/AArch64/subs-to-sub-opt.ll =================================================================== --- /dev/null +++ test/CodeGen/AArch64/subs-to-sub-opt.ll @@ -0,0 +1,23 @@ +; RUN: llc -mtriple=aarch64-linux-gnu -O3 -o - %s | FileCheck %s + +@a = external global i8, align 1 +@b = external global i8, align 1 + +; Test that SUBS is replaced by SUB if condition flags are not used. +define i32 @test01() nounwind { +; CHECK: ldrb {{.*}} +; CHECK-NEXT: ldrb {{.*}} +; CHECK-NEXT: sub {{.*}} +; CHECK-NEXT: cmn {{.*}} +entry: + %0 = load i8, i8* @a, align 1 + %conv = zext i8 %0 to i32 + %1 = load i8, i8* @b, align 1 + %conv1 = zext i8 %1 to i32 + %s = sub nsw i32 %conv1, %conv + %cmp0 = icmp eq i32 %s, -1 + %cmp1 = sext i1 %cmp0 to i8 + store i8 %cmp1, i8* @a + ret i32 0 +} +