CodeGen: improve MachineInstrBuilder & MachineIRBuilder interface

For MachineInstrBuilder, having to manually use RegState::Define is ugly and
makes register definitions clunkier than they need to be, so this adds two
convenience functions: addDef and addUse.

For MachineIRBuilder, we want to avoid BuildMI's first-reg-is-def rule because
it's hidden away and causes bugs. So this patch switches buildInstr to
returning a MachineInstrBuilder and adding *all* operands via addDef/addUse.

NFC.

llvm-svn: 277176
This commit is contained in:
Tim Northover 2016-07-29 17:43:52 +00:00
parent 48c7cc9bc0
commit a51575ffa2
5 changed files with 96 additions and 120 deletions

View File

@ -52,16 +52,6 @@ class MachineIRBuilder {
return *TII; return *TII;
} }
static void addRegs(MachineInstrBuilder &MIB) {}
template <typename... MoreRegs>
static void addRegs(MachineInstrBuilder &MIB, unsigned Reg,
MoreRegs... Regs) {
MIB.addReg(Reg);
addRegs(MIB, Regs...);
}
public: public:
/// Getter for the function we currently build. /// Getter for the function we currently build.
MachineFunction &getMF() { MachineFunction &getMF() {
@ -107,53 +97,19 @@ public:
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// \pre Ty == LLT{} or isPreISelGenericOpcode(Opcode) /// \pre Ty == LLT{} or isPreISelGenericOpcode(Opcode)
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildInstr(unsigned Opcode, ArrayRef<LLT> Tys); MachineInstrBuilder buildInstr(unsigned Opcode, ArrayRef<LLT> Tys);
/// Build and insert \p Res = \p Opcode [\p Ty] \p Uses....
/// \p Ty is the type of the instruction if \p Opcode describes
/// a generic machine instruction. \p Ty must be LLT{} if \p Opcode
/// does not describe a generic instruction.
/// The insertion point is the one set by the last call of either
/// setBasicBlock or setMI.
///
/// \pre setBasicBlock or setMI must have been called.
/// \pre Ty == LLT{} or isPreISelGenericOpcode(Opcode)
///
/// \return The newly created instruction.
template <typename... MoreRegs>
MachineInstr *buildInstr(unsigned Opcode, ArrayRef<LLT> Tys, unsigned Res,
MoreRegs... Uses) {
MachineInstr *NewMI = buildInstr(Opcode, Tys);
MachineInstrBuilder MIB{getMF(), NewMI};
MIB.addReg(Res, RegState::Define);
addRegs(MIB, Uses...);
return NewMI;
}
/// Build and insert <empty> = \p Opcode <empty>. /// Build and insert <empty> = \p Opcode <empty>.
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// \pre not isPreISelGenericOpcode(\p Opcode) /// \pre not isPreISelGenericOpcode(\p Opcode)
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildInstr(unsigned Opcode) { MachineInstrBuilder buildInstr(unsigned Opcode) {
return buildInstr(Opcode, ArrayRef<LLT>()); return buildInstr(Opcode, ArrayRef<LLT>());
} }
/// Build and insert \p Res = \p Opcode \p Uses....
/// The insertion point is the one set by the last call of either
/// setBasicBlock or setMI.
///
/// \pre setBasicBlock or setMI must have been called.
///
/// \return The newly created instruction.
template <typename... MoreRegs>
MachineInstr *buildInstr(unsigned Opcode, unsigned Res, MoreRegs... Uses) {
return buildInstr(Opcode, ArrayRef<LLT>(), Res, Uses...);
}
/// Build and insert \p Res<def> = G_FRAME_INDEX \p Ty \p Idx /// Build and insert \p Res<def> = G_FRAME_INDEX \p Ty \p Idx
/// ///
/// G_FRAME_INDEX materializes the address of an alloca value or other /// G_FRAME_INDEX materializes the address of an alloca value or other
@ -161,8 +117,8 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildFrameIndex(LLT Ty, unsigned Res, int Idx); MachineInstrBuilder buildFrameIndex(LLT Ty, unsigned Res, int Idx);
/// Build and insert \p Res<def> = G_ADD \p Ty \p Op0, \p Op1 /// Build and insert \p Res<def> = G_ADD \p Ty \p Op0, \p Op1
/// ///
@ -171,8 +127,9 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildAdd(LLT Ty, unsigned Res, unsigned Op0, unsigned Op1); MachineInstrBuilder buildAdd(LLT Ty, unsigned Res, unsigned Op0,
unsigned Op1);
/// Build and insert G_BR unsized \p Dest /// Build and insert G_BR unsized \p Dest
/// ///
@ -180,8 +137,8 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildBr(MachineBasicBlock &BB); MachineInstrBuilder buildBr(MachineBasicBlock &BB);
/// Build and insert \p Res<def> = COPY Op /// Build and insert \p Res<def> = COPY Op
/// ///
@ -189,8 +146,8 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildCopy(unsigned Res, unsigned Op); MachineInstrBuilder buildCopy(unsigned Res, unsigned Op);
/// Build and insert `Res<def> = G_LOAD { VTy, PTy } Addr, MMO`. /// Build and insert `Res<def> = G_LOAD { VTy, PTy } Addr, MMO`.
/// ///
@ -199,9 +156,9 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildLoad(LLT VTy, LLT PTy, unsigned Res, unsigned Addr, MachineInstrBuilder buildLoad(LLT VTy, LLT PTy, unsigned Res, unsigned Addr,
MachineMemOperand &MMO); MachineMemOperand &MMO);
/// Build and insert `G_STORE { VTy, PTy } Val, Addr, MMO`. /// Build and insert `G_STORE { VTy, PTy } Val, Addr, MMO`.
/// ///
@ -210,9 +167,9 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildStore(LLT VTy, LLT PTy, unsigned Val, unsigned Addr, MachineInstrBuilder buildStore(LLT VTy, LLT PTy, unsigned Val, unsigned Addr,
MachineMemOperand &MMO); MachineMemOperand &MMO);
/// Build and insert `Res0<def>, ... = G_EXTRACT Ty Src, Idx0, ...`. /// Build and insert `Res0<def>, ... = G_EXTRACT Ty Src, Idx0, ...`.
/// ///
@ -221,9 +178,9 @@ public:
/// ///
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildExtract(LLT Ty, ArrayRef<unsigned> Results, unsigned Src, MachineInstrBuilder buildExtract(LLT Ty, ArrayRef<unsigned> Results,
ArrayRef<unsigned> Indexes); unsigned Src, ArrayRef<unsigned> Indexes);
/// Build and insert \p Res<def> = G_SEQUENCE \p Ty \p Ops[0], ... /// Build and insert \p Res<def> = G_SEQUENCE \p Ty \p Ops[0], ...
/// ///
@ -233,8 +190,9 @@ public:
/// \pre setBasicBlock or setMI must have been called. /// \pre setBasicBlock or setMI must have been called.
/// \pre The sum of the input sizes must equal the result's size. /// \pre The sum of the input sizes must equal the result's size.
/// ///
/// \return The newly created instruction. /// \return a MachineInstrBuilder for the newly created instruction.
MachineInstr *buildSequence(LLT Ty, unsigned Res, ArrayRef<unsigned> Ops); MachineInstrBuilder buildSequence(LLT Ty, unsigned Res,
ArrayRef<unsigned> Ops);
}; };
} // End namespace llvm. } // End namespace llvm.

View File

@ -83,6 +83,21 @@ public:
return *this; return *this;
} }
/// Add a virtual register definition operand.
const MachineInstrBuilder &addDef(unsigned RegNo, unsigned Flags = 0,
unsigned SubReg = 0) const {
return addReg(RegNo, Flags | RegState::Define, SubReg);
}
/// Add a virtual register use operand. It is an error for Flags to contain
/// `RegState::Define` when calling this function.
const MachineInstrBuilder &addUse(unsigned RegNo, unsigned Flags = 0,
unsigned SubReg = 0) const {
assert(!(Flags & RegState::Define) &&
"Misleading addUse defines register, use addReg instead.");
return addReg(RegNo, Flags, SubReg);
}
/// Add a new immediate operand. /// Add a new immediate operand.
const MachineInstrBuilder &addImm(int64_t Val) const { const MachineInstrBuilder &addImm(int64_t Val) const {
MI->addOperand(*MF, MachineOperand::CreateImm(Val)); MI->addOperand(*MF, MachineOperand::CreateImm(Val));

View File

@ -85,7 +85,10 @@ bool IRTranslator::translateBinaryOp(unsigned Opcode, const Instruction &Inst) {
unsigned Op0 = getOrCreateVReg(*Inst.getOperand(0)); unsigned Op0 = getOrCreateVReg(*Inst.getOperand(0));
unsigned Op1 = getOrCreateVReg(*Inst.getOperand(1)); unsigned Op1 = getOrCreateVReg(*Inst.getOperand(1));
unsigned Res = getOrCreateVReg(Inst); unsigned Res = getOrCreateVReg(Inst);
MIRBuilder.buildInstr(Opcode, LLT{*Inst.getType()}, Res, Op0, Op1); MIRBuilder.buildInstr(Opcode, LLT{*Inst.getType()})
.addDef(Res)
.addUse(Op0)
.addUse(Op1);
return true; return true;
} }
@ -160,8 +163,9 @@ bool IRTranslator::translateBitCast(const CastInst &CI) {
bool IRTranslator::translateCast(unsigned Opcode, const CastInst &CI) { bool IRTranslator::translateCast(unsigned Opcode, const CastInst &CI) {
unsigned Op = getOrCreateVReg(*CI.getOperand(0)); unsigned Op = getOrCreateVReg(*CI.getOperand(0));
unsigned Res = getOrCreateVReg(CI); unsigned Res = getOrCreateVReg(CI);
MIRBuilder.buildInstr(Opcode, {LLT{*CI.getDestTy()}, LLT{*CI.getSrcTy()}}, MIRBuilder.buildInstr(Opcode, {LLT{*CI.getDestTy()}, LLT{*CI.getSrcTy()}})
Res, Op); .addDef(Res)
.addUse(Op);
return true; return true;
} }

View File

@ -57,83 +57,83 @@ MachineBasicBlock::iterator MachineIRBuilder::getInsertPt() {
// Build instruction variants. // Build instruction variants.
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
MachineInstr *MachineIRBuilder::buildInstr(unsigned Opcode, ArrayRef<LLT> Tys) { MachineInstrBuilder MachineIRBuilder::buildInstr(unsigned Opcode,
MachineInstr *NewMI = BuildMI(getMF(), DL, getTII().get(Opcode)); ArrayRef<LLT> Tys) {
MachineInstrBuilder MIB = BuildMI(getMF(), DL, getTII().get(Opcode));
if (Tys.size() > 0) { if (Tys.size() > 0) {
assert(isPreISelGenericOpcode(Opcode) && assert(isPreISelGenericOpcode(Opcode) &&
"Only generic instruction can have a type"); "Only generic instruction can have a type");
for (unsigned i = 0; i < Tys.size(); ++i) for (unsigned i = 0; i < Tys.size(); ++i)
NewMI->setType(Tys[i], i); MIB->setType(Tys[i], i);
} else } else
assert(!isPreISelGenericOpcode(Opcode) && assert(!isPreISelGenericOpcode(Opcode) &&
"Generic instruction must have a type"); "Generic instruction must have a type");
getMBB().insert(getInsertPt(), NewMI); getMBB().insert(getInsertPt(), MIB);
return NewMI; return MIB;
} }
MachineInstr *MachineIRBuilder::buildFrameIndex(LLT Ty, unsigned Res, int Idx) { MachineInstrBuilder MachineIRBuilder::buildFrameIndex(LLT Ty, unsigned Res,
MachineInstr *NewMI = buildInstr(TargetOpcode::G_FRAME_INDEX, Ty); int Idx) {
auto MIB = MachineInstrBuilder(getMF(), NewMI); return buildInstr(TargetOpcode::G_FRAME_INDEX, Ty)
MIB.addReg(Res, RegState::Define); .addDef(Res)
MIB.addFrameIndex(Idx); .addFrameIndex(Idx);
return NewMI;
} }
MachineInstr *MachineIRBuilder::buildAdd(LLT Ty, unsigned Res, unsigned Op0, MachineInstrBuilder MachineIRBuilder::buildAdd(LLT Ty, unsigned Res,
unsigned Op1) { unsigned Op0, unsigned Op1) {
return buildInstr(TargetOpcode::G_ADD, Ty, Res, Op0, Op1); return buildInstr(TargetOpcode::G_ADD, Ty)
.addDef(Res)
.addUse(Op0)
.addUse(Op1);
} }
MachineInstr *MachineIRBuilder::buildBr(MachineBasicBlock &Dest) { MachineInstrBuilder MachineIRBuilder::buildBr(MachineBasicBlock &Dest) {
MachineInstr *NewMI = buildInstr(TargetOpcode::G_BR, LLT::unsized()); return buildInstr(TargetOpcode::G_BR, LLT::unsized()).addMBB(&Dest);
MachineInstrBuilder(getMF(), NewMI).addMBB(&Dest);
return NewMI;
} }
MachineInstr *MachineIRBuilder::buildCopy(unsigned Res, unsigned Op) { MachineInstrBuilder MachineIRBuilder::buildCopy(unsigned Res, unsigned Op) {
return buildInstr(TargetOpcode::COPY, Res, Op); return buildInstr(TargetOpcode::COPY).addDef(Res).addUse(Op);
} }
MachineInstr *MachineIRBuilder::buildLoad(LLT VTy, LLT PTy, unsigned Res, MachineInstrBuilder MachineIRBuilder::buildLoad(LLT VTy, LLT PTy, unsigned Res,
unsigned Addr, unsigned Addr,
MachineMemOperand &MMO) { MachineMemOperand &MMO) {
MachineInstr *NewMI = buildInstr(TargetOpcode::G_LOAD, {VTy, PTy}, Res, Addr); return buildInstr(TargetOpcode::G_LOAD, {VTy, PTy})
NewMI->addMemOperand(getMF(), &MMO); .addDef(Res)
return NewMI; .addUse(Addr)
.addMemOperand(&MMO);
} }
MachineInstr *MachineIRBuilder::buildStore(LLT VTy, LLT PTy, unsigned Val, MachineInstrBuilder MachineIRBuilder::buildStore(LLT VTy, LLT PTy,
unsigned Addr, unsigned Val, unsigned Addr,
MachineMemOperand &MMO) { MachineMemOperand &MMO) {
MachineInstr *NewMI = buildInstr(TargetOpcode::G_STORE, {VTy, PTy}); return buildInstr(TargetOpcode::G_STORE, {VTy, PTy})
NewMI->addMemOperand(getMF(), &MMO); .addUse(Val)
MachineInstrBuilder(getMF(), NewMI).addReg(Val).addReg(Addr); .addUse(Addr)
return NewMI; .addMemOperand(&MMO);
} }
MachineInstr *MachineIRBuilder::buildExtract(LLT Ty, ArrayRef<unsigned> Results, MachineInstrBuilder
unsigned Src, MachineIRBuilder::buildExtract(LLT Ty, ArrayRef<unsigned> Results, unsigned Src,
ArrayRef<unsigned> Indexes) { ArrayRef<unsigned> Indexes) {
assert(Results.size() == Indexes.size() && "inconsistent number of regs"); assert(Results.size() == Indexes.size() && "inconsistent number of regs");
MachineInstr *NewMI = buildInstr(TargetOpcode::G_EXTRACT, Ty); MachineInstrBuilder MIB = buildInstr(TargetOpcode::G_EXTRACT, Ty);
auto MIB = MachineInstrBuilder(getMF(), NewMI);
for (auto Res : Results) for (auto Res : Results)
MIB.addReg(Res, RegState::Define); MIB.addDef(Res);
MIB.addReg(Src); MIB.addUse(Src);
for (auto Idx : Indexes) for (auto Idx : Indexes)
MIB.addImm(Idx); MIB.addImm(Idx);
return NewMI; return MIB;
} }
MachineInstr *MachineIRBuilder::buildSequence(LLT Ty, unsigned Res, MachineInstrBuilder MachineIRBuilder::buildSequence(LLT Ty, unsigned Res,
ArrayRef<unsigned> Ops) { ArrayRef<unsigned> Ops) {
MachineInstr *NewMI = buildInstr(TargetOpcode::G_SEQUENCE, Ty); MachineInstrBuilder MIB = buildInstr(TargetOpcode::G_SEQUENCE, Ty);
auto MIB = MachineInstrBuilder(getMF(), NewMI); MIB.addDef(Res);
MIB.addReg(Res, RegState::Define);
for (auto Op : Ops) for (auto Op : Ops)
MIB.addReg(Op); MIB.addUse(Op);
return NewMI; return MIB;
} }

View File

@ -45,8 +45,7 @@ bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
unsigned ResReg = (Size == 32) ? AArch64::W0 : AArch64::X0; unsigned ResReg = (Size == 32) ? AArch64::W0 : AArch64::X0;
// Set the insertion point to be right before Return. // Set the insertion point to be right before Return.
MIRBuilder.setInstr(*Return, /* Before */ true); MIRBuilder.setInstr(*Return, /* Before */ true);
MachineInstr *Copy = MachineInstr *Copy = MIRBuilder.buildCopy(ResReg, VReg);
MIRBuilder.buildInstr(TargetOpcode::COPY, ResReg, VReg);
(void)Copy; (void)Copy;
assert(Copy->getNextNode() == Return && assert(Copy->getNextNode() == Return &&
"The insertion did not happen where we expected"); "The insertion did not happen where we expected");
@ -85,7 +84,7 @@ bool AArch64CallLowering::lowerFormalArguments(
assert(VA.isRegLoc() && "Not yet implemented"); assert(VA.isRegLoc() && "Not yet implemented");
// Transform the arguments in physical registers into virtual ones. // Transform the arguments in physical registers into virtual ones.
MIRBuilder.getMBB().addLiveIn(VA.getLocReg()); MIRBuilder.getMBB().addLiveIn(VA.getLocReg());
MIRBuilder.buildInstr(TargetOpcode::COPY, VRegs[i], VA.getLocReg()); MIRBuilder.buildCopy(VRegs[i], VA.getLocReg());
switch (VA.getLocInfo()) { switch (VA.getLocInfo()) {
default: default: