[llvm] r307766 - [ARM] GlobalISel: Simplify inst selector code. NFC
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 12 03:31:16 PDT 2017
Author: rovka
Date: Wed Jul 12 03:31:16 2017
New Revision: 307766
URL: http://llvm.org/viewvc/llvm-project?rev=307766&view=rev
Log:
[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.
Modified:
llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp
Modified: llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp?rev=307766&r1=307765&r2=307766&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMInstructionSelector.cpp Wed Jul 12 03:31:16 2017
@@ -44,16 +44,32 @@ public:
private:
bool selectImpl(MachineInstr &I) const;
- template <typename T> struct CmpHelper;
+ struct CmpConstants;
+ struct InsertInfo;
- template <typename T>
- bool selectCmp(MachineInstrBuilder &MIB, const ARMBaseInstrInfo &TII,
- MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
- const RegisterBankInfo &RBI) const;
-
- bool selectSelect(MachineInstrBuilder &MIB, const ARMBaseInstrInfo &TII,
- MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
- const RegisterBankInfo &RBI) const;
+ bool selectCmp(CmpConstants Helper, MachineInstrBuilder &MIB,
+ MachineRegisterInfo &MRI) 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 <typename T> 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);
- }
+ const unsigned OperandSize;
+};
- bool validateResReg(unsigned ResReg) {
- if (MRI.getType(ResReg).getSizeInBits() != 1) {
- DEBUG(dbgs() << "Unsupported size for comparison operand");
- return false;
- }
+struct ARMInstructionSelector::InsertInfo {
+ InsertInfo(MachineInstrBuilder &MIB)
+ : MBB(*MIB->getParent()), InsertBefore(std::next(MIB->getIterator())),
+ DbgLoc(MIB->getDebugLoc()) {}
- if (RBI.getRegBank(ResReg, MRI, TRI)->getID() != ResultRegBankID) {
- DEBUG(dbgs() << "Unsupported register bank for comparison operand");
- return false;
- }
+ MachineBasicBlock &MBB;
+ const MachineBasicBlock::instr_iterator InsertBefore;
+ const DebugLoc &DbgLoc;
+};
- return true;
+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());
+}
+
+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);
+}
+
+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;
}
-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;
+ if (RBI.getRegBank(Reg, MRI, TRI)->getID() != ExpectedRegBankID) {
+ DEBUG(dbgs() << "Unexpected register bank for register");
+ return false;
}
- MachineBasicBlock &MBB;
- MachineBasicBlock::instr_iterator InsertBefore;
- const DebugLoc &DbgLoc;
-
- const ARMBaseInstrInfo &TII;
- MachineRegisterInfo &MRI;
- const TargetRegisterInfo &TRI;
- const RegisterBankInfo &RBI;
-
- const ARMInstructionSelector &Selector;
-};
+ return true;
+}
-// Specialize the opcode to be used for comparing different types of operands.
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<int>::ComparisonOpcode =
- ARM::CMPrr;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<float>::ComparisonOpcode =
- ARM::VCMPS;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<double>::ComparisonOpcode =
- ARM::VCMPD;
-
-// Specialize the opcode to be used for reading the comparison flags for
-// different types of operands.
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<int>::ReadFlagsOpcode =
- ARM::INSTRUCTION_LIST_END;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<float>::ReadFlagsOpcode =
- ARM::FMSTAT;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<double>::ReadFlagsOpcode =
- ARM::FMSTAT;
-
-// Specialize the register bank where the operands of the comparison are assumed
-// to live.
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<int>::OperandRegBankID =
- ARM::GPRRegBankID;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<float>::OperandRegBankID =
- ARM::FPRRegBankID;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<double>::OperandRegBankID =
- ARM::FPRRegBankID;
-
-// Specialize the size that the operands of the comparison are assumed to have.
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<int>::OperandSize = 32;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<float>::OperandSize = 32;
-template <>
-const unsigned ARMInstructionSelector::CmpHelper<double>::OperandSize = 64;
-
-template <typename T>
-bool ARMInstructionSelector::selectCmp(MachineInstrBuilder &MIB,
- const ARMBaseInstrInfo &TII,
- MachineRegisterInfo &MRI,
- const TargetRegisterInfo &TRI,
- const RegisterBankInfo &RBI) const {
- auto Helper = CmpHelper<T>(*this, MIB, TII, MRI, TRI, RBI);
+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<CmpInst::Predicate>(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(M
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::selectSelec
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(Mach
return selectCopy(I, TII, MRI, TRI, RBI);
}
case G_SELECT:
- return selectSelect(MIB, TII, MRI, TRI, RBI);
- case G_ICMP:
- return selectCmp<int>(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<float>(MIB, TII, MRI, TRI, RBI);
- if (Size == 64) {
- if (TII.getSubtarget().isFPOnlySP()) {
- DEBUG(dbgs() << "Subtarget only supports single precision");
- return false;
- }
- return selectCmp<double>(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(Mach
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());
More information about the llvm-commits
mailing list