From a3db45981cb7219619802cbfd263f9518f4d5b69 Mon Sep 17 00:00:00 2001 From: Diana Picus Date: Wed, 12 Jul 2017 10:31:16 +0000 Subject: [PATCH] [ARM] GlobalISel: Simplify inst selector code. NFC Refactor CmpHelper into something simpler. It was overkill to use templates for this - instead, use a simple CmpConstants structure to hold the opcodes and other constants that are different when selecting int / float / double comparisons. Also, extract some of the helpers that were in CmpHelper into ARMInstructionSelector and make use of some of them when selecting other things than just compares. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@307766 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMInstructionSelector.cpp | 349 +++++++++------------- 1 file changed, 147 insertions(+), 202 deletions(-) diff --git a/lib/Target/ARM/ARMInstructionSelector.cpp b/lib/Target/ARM/ARMInstructionSelector.cpp index c318c9c60a5..29ef69ad001 100644 --- a/lib/Target/ARM/ARMInstructionSelector.cpp +++ b/lib/Target/ARM/ARMInstructionSelector.cpp @@ -44,16 +44,32 @@ public: private: bool selectImpl(MachineInstr &I) const; - template struct CmpHelper; + struct CmpConstants; + struct InsertInfo; - template - bool selectCmp(MachineInstrBuilder &MIB, const ARMBaseInstrInfo &TII, - MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI, - const RegisterBankInfo &RBI) const; + bool selectCmp(CmpConstants Helper, MachineInstrBuilder &MIB, + MachineRegisterInfo &MRI) const; - bool selectSelect(MachineInstrBuilder &MIB, const ARMBaseInstrInfo &TII, - MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI, - const RegisterBankInfo &RBI) const; + // Helper for inserting a comparison sequence that sets \p ResReg to either 1 + // if \p LHSReg and \p RHSReg are in the relationship defined by \p Cond, or + // \p PrevRes otherwise. In essence, it computes PrevRes OR (LHS Cond RHS). + bool insertComparison(CmpConstants Helper, InsertInfo I, unsigned ResReg, + ARMCC::CondCodes Cond, unsigned LHSReg, unsigned RHSReg, + unsigned PrevRes) const; + + // Set \p DestReg to \p Constant. + void putConstant(InsertInfo I, unsigned DestReg, unsigned Constant) const; + + bool selectSelect(MachineInstrBuilder &MIB, MachineRegisterInfo &MRI) const; + + // Check if the types match and both operands have the expected size and + // register bank. + bool validOpRegPair(MachineRegisterInfo &MRI, unsigned LHS, unsigned RHS, + unsigned ExpectedSize, unsigned ExpectedRegBankID) const; + + // Check if the register has the expected size and register bank. + bool validReg(MachineRegisterInfo &MRI, unsigned Reg, unsigned ExpectedSize, + unsigned ExpectedRegBankID) const; const ARMBaseInstrInfo &TII; const ARMBaseRegisterInfo &TRI; @@ -326,212 +342,110 @@ getComparePreds(CmpInst::Predicate Pred) { return Preds; } -template struct ARMInstructionSelector::CmpHelper { - CmpHelper(const ARMInstructionSelector &Selector, MachineInstrBuilder &MIB, - const ARMBaseInstrInfo &TII, MachineRegisterInfo &MRI, - const TargetRegisterInfo &TRI, const RegisterBankInfo &RBI) - : MBB(*MIB->getParent()), InsertBefore(std::next(MIB->getIterator())), - DbgLoc(MIB->getDebugLoc()), TII(TII), MRI(MRI), TRI(TRI), RBI(RBI), - Selector(Selector) {} +struct ARMInstructionSelector::CmpConstants { + CmpConstants(unsigned CmpOpcode, unsigned FlagsOpcode, unsigned OpRegBank, + unsigned OpSize) + : ComparisonOpcode(CmpOpcode), ReadFlagsOpcode(FlagsOpcode), + OperandRegBankID(OpRegBank), OperandSize(OpSize) {} // The opcode used for performing the comparison. - static const unsigned ComparisonOpcode; + const unsigned ComparisonOpcode; // The opcode used for reading the flags set by the comparison. May be // ARM::INSTRUCTION_LIST_END if we don't need to read the flags. - static const unsigned ReadFlagsOpcode; - - // The opcode used for selecting the result register, based on the value - // of the flags. - static const unsigned SetResultOpcode = ARM::MOVCCi; + const unsigned ReadFlagsOpcode; // The assumed register bank ID for the operands. - static const unsigned OperandRegBankID; + const unsigned OperandRegBankID; // The assumed size in bits for the operands. - static const unsigned OperandSize; - - // The assumed register bank ID for the result. - static const unsigned ResultRegBankID = ARM::GPRRegBankID; - - unsigned getZeroRegister() { - unsigned Reg = MRI.createVirtualRegister(&ARM::GPRRegClass); - putConstant(Reg, 0); - return Reg; - } - - void putConstant(unsigned DestReg, unsigned Constant) { - (void)BuildMI(MBB, InsertBefore, DbgLoc, TII.get(ARM::MOVi)) - .addDef(DestReg) - .addImm(Constant) - .add(predOps(ARMCC::AL)) - .add(condCodeOp()); - } - - bool insertComparison(unsigned ResReg, ARMCC::CondCodes Cond, unsigned LHSReg, - unsigned RHSReg, unsigned PrevRes) { - - // Perform the comparison. - auto CmpI = BuildMI(MBB, InsertBefore, DbgLoc, TII.get(ComparisonOpcode)) - .addUse(LHSReg) - .addUse(RHSReg) - .add(predOps(ARMCC::AL)); - if (!Selector.constrainSelectedInstRegOperands(*CmpI, TII, TRI, RBI)) - return false; - - // Read the comparison flags (if necessary). - if (ReadFlagsOpcode != ARM::INSTRUCTION_LIST_END) { - auto ReadI = BuildMI(MBB, InsertBefore, DbgLoc, TII.get(ReadFlagsOpcode)) - .add(predOps(ARMCC::AL)); - if (!Selector.constrainSelectedInstRegOperands(*ReadI, TII, TRI, RBI)) - return false; - } - - // Select either 1 or the previous result based on the value of the flags. - auto Mov1I = BuildMI(MBB, InsertBefore, DbgLoc, TII.get(SetResultOpcode)) - .addDef(ResReg) - .addUse(PrevRes) - .addImm(1) - .add(predOps(Cond, ARM::CPSR)); - if (!Selector.constrainSelectedInstRegOperands(*Mov1I, TII, TRI, RBI)) - return false; - - return true; - } - - bool validateOpRegs(unsigned LHSReg, unsigned RHSReg) { - return MRI.getType(LHSReg) == MRI.getType(RHSReg) && - validateOpReg(LHSReg, MRI, TRI, RBI) && - validateOpReg(RHSReg, MRI, TRI, RBI); - } - - bool validateResReg(unsigned ResReg) { - if (MRI.getType(ResReg).getSizeInBits() != 1) { - DEBUG(dbgs() << "Unsupported size for comparison operand"); - return false; - } - - if (RBI.getRegBank(ResReg, MRI, TRI)->getID() != ResultRegBankID) { - DEBUG(dbgs() << "Unsupported register bank for comparison operand"); - return false; - } - - return true; - } - -private: - bool validateOpReg(unsigned OpReg, MachineRegisterInfo &MRI, - const TargetRegisterInfo &TRI, - const RegisterBankInfo &RBI) { - if (MRI.getType(OpReg).getSizeInBits() != OperandSize) { - DEBUG(dbgs() << "Unsupported size for comparison operand"); - return false; - } - - if (RBI.getRegBank(OpReg, MRI, TRI)->getID() != OperandRegBankID) { - DEBUG(dbgs() << "Unsupported register bank for comparison operand"); - return false; - } - - return true; - } - - MachineBasicBlock &MBB; - MachineBasicBlock::instr_iterator InsertBefore; - const DebugLoc &DbgLoc; - - const ARMBaseInstrInfo &TII; - MachineRegisterInfo &MRI; - const TargetRegisterInfo &TRI; - const RegisterBankInfo &RBI; - - const ARMInstructionSelector &Selector; + const unsigned OperandSize; }; -// Specialize the opcode to be used for comparing different types of operands. -template <> -const unsigned ARMInstructionSelector::CmpHelper::ComparisonOpcode = - ARM::CMPrr; -template <> -const unsigned ARMInstructionSelector::CmpHelper::ComparisonOpcode = - ARM::VCMPS; -template <> -const unsigned ARMInstructionSelector::CmpHelper::ComparisonOpcode = - ARM::VCMPD; +struct ARMInstructionSelector::InsertInfo { + InsertInfo(MachineInstrBuilder &MIB) + : MBB(*MIB->getParent()), InsertBefore(std::next(MIB->getIterator())), + DbgLoc(MIB->getDebugLoc()) {} -// Specialize the opcode to be used for reading the comparison flags for -// different types of operands. -template <> -const unsigned ARMInstructionSelector::CmpHelper::ReadFlagsOpcode = - ARM::INSTRUCTION_LIST_END; -template <> -const unsigned ARMInstructionSelector::CmpHelper::ReadFlagsOpcode = - ARM::FMSTAT; -template <> -const unsigned ARMInstructionSelector::CmpHelper::ReadFlagsOpcode = - ARM::FMSTAT; + MachineBasicBlock &MBB; + const MachineBasicBlock::instr_iterator InsertBefore; + const DebugLoc &DbgLoc; +}; -// Specialize the register bank where the operands of the comparison are assumed -// to live. -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandRegBankID = - ARM::GPRRegBankID; -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandRegBankID = - ARM::FPRRegBankID; -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandRegBankID = - ARM::FPRRegBankID; +void ARMInstructionSelector::putConstant(InsertInfo I, unsigned DestReg, + unsigned Constant) const { + (void)BuildMI(I.MBB, I.InsertBefore, I.DbgLoc, TII.get(ARM::MOVi)) + .addDef(DestReg) + .addImm(Constant) + .add(predOps(ARMCC::AL)) + .add(condCodeOp()); +} -// Specialize the size that the operands of the comparison are assumed to have. -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandSize = 32; -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandSize = 32; -template <> -const unsigned ARMInstructionSelector::CmpHelper::OperandSize = 64; +bool ARMInstructionSelector::validOpRegPair(MachineRegisterInfo &MRI, + unsigned LHSReg, unsigned RHSReg, + unsigned ExpectedSize, + unsigned ExpectedRegBankID) const { + return MRI.getType(LHSReg) == MRI.getType(RHSReg) && + validReg(MRI, LHSReg, ExpectedSize, ExpectedRegBankID) && + validReg(MRI, RHSReg, ExpectedSize, ExpectedRegBankID); +} -template -bool ARMInstructionSelector::selectCmp(MachineInstrBuilder &MIB, - const ARMBaseInstrInfo &TII, - MachineRegisterInfo &MRI, - const TargetRegisterInfo &TRI, - const RegisterBankInfo &RBI) const { - auto Helper = CmpHelper(*this, MIB, TII, MRI, TRI, RBI); +bool ARMInstructionSelector::validReg(MachineRegisterInfo &MRI, unsigned Reg, + unsigned ExpectedSize, + unsigned ExpectedRegBankID) const { + if (MRI.getType(Reg).getSizeInBits() != ExpectedSize) { + DEBUG(dbgs() << "Unexpected size for register"); + return false; + } + + if (RBI.getRegBank(Reg, MRI, TRI)->getID() != ExpectedRegBankID) { + DEBUG(dbgs() << "Unexpected register bank for register"); + return false; + } + + return true; +} + +bool ARMInstructionSelector::selectCmp(CmpConstants Helper, + MachineInstrBuilder &MIB, + MachineRegisterInfo &MRI) const { + const InsertInfo I(MIB); auto ResReg = MIB->getOperand(0).getReg(); - if (!Helper.validateResReg(ResReg)) + if (!validReg(MRI, ResReg, 1, ARM::GPRRegBankID)) return false; auto Cond = static_cast(MIB->getOperand(1).getPredicate()); if (Cond == CmpInst::FCMP_TRUE || Cond == CmpInst::FCMP_FALSE) { - Helper.putConstant(ResReg, Cond == CmpInst::FCMP_TRUE ? 1 : 0); + putConstant(I, ResReg, Cond == CmpInst::FCMP_TRUE ? 1 : 0); MIB->eraseFromParent(); return true; } auto LHSReg = MIB->getOperand(2).getReg(); auto RHSReg = MIB->getOperand(3).getReg(); - if (!Helper.validateOpRegs(LHSReg, RHSReg)) + if (!validOpRegPair(MRI, LHSReg, RHSReg, Helper.OperandSize, + Helper.OperandRegBankID)) return false; auto ARMConds = getComparePreds(Cond); - auto ZeroReg = Helper.getZeroRegister(); + auto ZeroReg = MRI.createVirtualRegister(&ARM::GPRRegClass); + putConstant(I, ZeroReg, 0); if (ARMConds.second == ARMCC::AL) { // Simple case, we only need one comparison and we're done. - if (!Helper.insertComparison(ResReg, ARMConds.first, LHSReg, RHSReg, - ZeroReg)) + if (!insertComparison(Helper, I, ResReg, ARMConds.first, LHSReg, RHSReg, + ZeroReg)) return false; } else { // Not so simple, we need two successive comparisons. auto IntermediateRes = MRI.createVirtualRegister(&ARM::GPRRegClass); - if (!Helper.insertComparison(IntermediateRes, ARMConds.first, LHSReg, - RHSReg, ZeroReg)) + if (!insertComparison(Helper, I, IntermediateRes, ARMConds.first, LHSReg, + RHSReg, ZeroReg)) return false; - if (!Helper.insertComparison(ResReg, ARMConds.second, LHSReg, RHSReg, - IntermediateRes)) + if (!insertComparison(Helper, I, ResReg, ARMConds.second, LHSReg, RHSReg, + IntermediateRes)) return false; } @@ -539,19 +453,50 @@ bool ARMInstructionSelector::selectCmp(MachineInstrBuilder &MIB, return true; } +bool ARMInstructionSelector::insertComparison(CmpConstants Helper, InsertInfo I, + unsigned ResReg, + ARMCC::CondCodes Cond, + unsigned LHSReg, unsigned RHSReg, + unsigned PrevRes) const { + // Perform the comparison. + auto CmpI = + BuildMI(I.MBB, I.InsertBefore, I.DbgLoc, TII.get(Helper.ComparisonOpcode)) + .addUse(LHSReg) + .addUse(RHSReg) + .add(predOps(ARMCC::AL)); + if (!constrainSelectedInstRegOperands(*CmpI, TII, TRI, RBI)) + return false; + + // Read the comparison flags (if necessary). + if (Helper.ReadFlagsOpcode != ARM::INSTRUCTION_LIST_END) { + auto ReadI = BuildMI(I.MBB, I.InsertBefore, I.DbgLoc, + TII.get(Helper.ReadFlagsOpcode)) + .add(predOps(ARMCC::AL)); + if (!constrainSelectedInstRegOperands(*ReadI, TII, TRI, RBI)) + return false; + } + + // Select either 1 or the previous result based on the value of the flags. + auto Mov1I = BuildMI(I.MBB, I.InsertBefore, I.DbgLoc, TII.get(ARM::MOVCCi)) + .addDef(ResReg) + .addUse(PrevRes) + .addImm(1) + .add(predOps(Cond, ARM::CPSR)); + if (!constrainSelectedInstRegOperands(*Mov1I, TII, TRI, RBI)) + return false; + + return true; +} + bool ARMInstructionSelector::selectSelect(MachineInstrBuilder &MIB, - const ARMBaseInstrInfo &TII, - MachineRegisterInfo &MRI, - const TargetRegisterInfo &TRI, - const RegisterBankInfo &RBI) const { + MachineRegisterInfo &MRI) const { auto &MBB = *MIB->getParent(); auto InsertBefore = std::next(MIB->getIterator()); auto &DbgLoc = MIB->getDebugLoc(); // Compare the condition to 0. auto CondReg = MIB->getOperand(1).getReg(); - assert(MRI.getType(CondReg).getSizeInBits() == 1 && - RBI.getRegBank(CondReg, MRI, TRI)->getID() == ARM::GPRRegBankID && + assert(validReg(MRI, CondReg, 1, ARM::GPRRegBankID) && "Unsupported types for select operation"); auto CmpI = BuildMI(MBB, InsertBefore, DbgLoc, TII.get(ARM::CMPri)) .addUse(CondReg) @@ -565,11 +510,8 @@ bool ARMInstructionSelector::selectSelect(MachineInstrBuilder &MIB, auto ResReg = MIB->getOperand(0).getReg(); auto TrueReg = MIB->getOperand(2).getReg(); auto FalseReg = MIB->getOperand(3).getReg(); - assert(MRI.getType(ResReg) == MRI.getType(TrueReg) && - MRI.getType(TrueReg) == MRI.getType(FalseReg) && - MRI.getType(FalseReg).getSizeInBits() == 32 && - RBI.getRegBank(TrueReg, MRI, TRI)->getID() == ARM::GPRRegBankID && - RBI.getRegBank(FalseReg, MRI, TRI)->getID() == ARM::GPRRegBankID && + assert(validOpRegPair(MRI, ResReg, TrueReg, 32, ARM::GPRRegBankID) && + validOpRegPair(MRI, TrueReg, FalseReg, 32, ARM::GPRRegBankID) && "Unsupported types for select operation"); auto Mov1I = BuildMI(MBB, InsertBefore, DbgLoc, TII.get(ARM::MOVCCr)) .addDef(ResReg) @@ -684,26 +626,30 @@ bool ARMInstructionSelector::select(MachineInstr &I) const { return selectCopy(I, TII, MRI, TRI, RBI); } case G_SELECT: - return selectSelect(MIB, TII, MRI, TRI, RBI); - case G_ICMP: - return selectCmp(MIB, TII, MRI, TRI, RBI); + return selectSelect(MIB, MRI); + case G_ICMP: { + CmpConstants Helper(ARM::CMPrr, ARM::INSTRUCTION_LIST_END, + ARM::GPRRegBankID, 32); + return selectCmp(Helper, MIB, MRI); + } case G_FCMP: { assert(TII.getSubtarget().hasVFP2() && "Can't select fcmp without VFP"); unsigned OpReg = I.getOperand(2).getReg(); unsigned Size = MRI.getType(OpReg).getSizeInBits(); - if (Size == 32) - return selectCmp(MIB, TII, MRI, TRI, RBI); - if (Size == 64) { - if (TII.getSubtarget().isFPOnlySP()) { - DEBUG(dbgs() << "Subtarget only supports single precision"); - return false; - } - return selectCmp(MIB, TII, MRI, TRI, RBI); + + if (Size == 64 && TII.getSubtarget().isFPOnlySP()) { + DEBUG(dbgs() << "Subtarget only supports single precision"); + return false; + } + if (Size != 32 && Size != 64) { + DEBUG(dbgs() << "Unsupported size for G_FCMP operand"); + return false; } - DEBUG(dbgs() << "Unsupported size for G_FCMP operand"); - return false; + CmpConstants Helper(Size == 32 ? ARM::VCMPS : ARM::VCMPD, ARM::FMSTAT, + ARM::FPRRegBankID, Size); + return selectCmp(Helper, MIB, MRI); } case G_GEP: I.setDesc(TII.get(ARM::ADDrr)); @@ -717,11 +663,10 @@ bool ARMInstructionSelector::select(MachineInstr &I) const { break; case G_CONSTANT: { unsigned Reg = I.getOperand(0).getReg(); - if (MRI.getType(Reg).getSizeInBits() != 32) + + if (!validReg(MRI, Reg, 32, ARM::GPRRegBankID)) return false; - assert(RBI.getRegBank(Reg, MRI, TRI)->getID() == ARM::GPRRegBankID && - "Expected constant to live in a GPR"); I.setDesc(TII.get(ARM::MOVi)); MIB.add(predOps(ARMCC::AL)).add(condCodeOp());