Skip to content

Commit

Permalink
GlobalISel: simplify MachineIRBuilder interface.
Browse files Browse the repository at this point in the history
MachineIRBuilder had weird before/after and beginning/end flags for the insert
point. Unfortunately the non-default means that instructions will be inserted
in reverse order which is almost never what anyone wants.

Really, I think we just want (like IRBuilder has) the ability to insert at any
C++ iterator-style point (i.e. before any instruction or before MBB.end()). So
this fixes MIRBuilders to behave like IRBuilders in this respect.

llvm-svn: 288980
  • Loading branch information
TNorthover committed Dec 7, 2016
1 parent 64a0555 commit 05cc485
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 64 deletions.
23 changes: 14 additions & 9 deletions llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ class MachineIRBuilder {
/// Fields describing the insertion point.
/// @{
MachineBasicBlock *MBB;
MachineInstr *MI;
bool Before;
MachineBasicBlock::iterator II;
/// @}

std::function<void(MachineInstr *)> InsertedInstr;
Expand All @@ -75,22 +74,28 @@ class MachineIRBuilder {
}

/// Current insertion point for new instructions.
MachineBasicBlock::iterator getInsertPt();
MachineBasicBlock::iterator getInsertPt() {
return II;
}

/// Set the insertion point before the specified position.
/// \pre MBB must be in getMF().
/// \pre II must be a valid iterator in MBB.
void setInsertPt(MachineBasicBlock &MBB, MachineBasicBlock::iterator II);
/// @}

/// Setters for the insertion point.
/// @{
/// Set the MachineFunction where to build instructions.
void setMF(MachineFunction &);

/// Set the insertion point to the beginning (\p Beginning = true) or end
/// (\p Beginning = false) of \p MBB.
/// Set the insertion point to the end of \p MBB.
/// \pre \p MBB must be contained by getMF().
void setMBB(MachineBasicBlock &MBB, bool Beginning = false);
void setMBB(MachineBasicBlock &MBB);

/// Set the insertion point to before (\p Before = true) or after
/// (\p Before = false) \p MI.
/// Set the insertion point to before MI.
/// \pre MI must be in getMF().
void setInstr(MachineInstr &MI, bool Before = true);
void setInstr(MachineInstr &MI);
/// @}

/// Control where instructions we create are recorded (typically for
Expand Down
21 changes: 10 additions & 11 deletions llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,13 +744,18 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &MF) {

assert(PendingPHIs.empty() && "stale PHIs");

// Setup the arguments.
MachineBasicBlock &MBB = getOrCreateBB(F.front());
MIRBuilder.setMBB(MBB);
// Setup a separate basic-block for the arguments and constants, falling
// through to the IR-level Function's entry block.
MachineBasicBlock *EntryBB = MF.CreateMachineBasicBlock();
MF.push_back(EntryBB);
EntryBB->addSuccessor(&getOrCreateBB(F.front()));
EntryBuilder.setMBB(*EntryBB);

// Lower the actual args into this basic block.
SmallVector<unsigned, 8> VRegArgs;
for (const Argument &Arg: F.args())
VRegArgs.push_back(getOrCreateVReg(Arg));
bool Succeeded = CLI->lowerFormalArguments(MIRBuilder, F, VRegArgs);
bool Succeeded = CLI->lowerFormalArguments(EntryBuilder, F, VRegArgs);
if (!Succeeded) {
if (!TPC->isGlobalISelAbortEnabled()) {
MIRBuilder.getMF().getProperties().set(
Expand All @@ -761,13 +766,7 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &MF) {
report_fatal_error("Unable to lower arguments");
}

// Now that we've got the ABI handling code, it's safe to set a location for
// any Constants we find in the IR.
if (MBB.empty())
EntryBuilder.setMBB(MBB, /* Beginning */ true);
else
EntryBuilder.setInstr(MBB.back(), /* Before */ false);

// And translate the function!
for (const BasicBlock &BB: F) {
MachineBasicBlock &MBB = getOrCreateBB(BB);
// Set the insertion point of all the following translations to
Expand Down
27 changes: 11 additions & 16 deletions llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,29 @@ void MachineIRBuilder::setMF(MachineFunction &MF) {
this->MRI = &MF.getRegInfo();
this->TII = MF.getSubtarget().getInstrInfo();
this->DL = DebugLoc();
this->MI = nullptr;
this->II = MachineBasicBlock::iterator();
this->InsertedInstr = nullptr;
}

void MachineIRBuilder::setMBB(MachineBasicBlock &MBB, bool Beginning) {
void MachineIRBuilder::setMBB(MachineBasicBlock &MBB) {
this->MBB = &MBB;
this->MI = nullptr;
Before = Beginning;
this->II = MBB.end();
assert(&getMF() == MBB.getParent() &&
"Basic block is in a different function");
}

void MachineIRBuilder::setInstr(MachineInstr &MI, bool Before) {
void MachineIRBuilder::setInstr(MachineInstr &MI) {
assert(MI.getParent() && "Instruction is not part of a basic block");
setMBB(*MI.getParent());
this->MI = &MI;
this->Before = Before;
this->II = MI.getIterator();
}

MachineBasicBlock::iterator MachineIRBuilder::getInsertPt() {
if (MI) {
if (Before)
return MI;
if (!MI->getNextNode())
return getMBB().end();
return MI->getNextNode();
}
return Before ? getMBB().begin() : getMBB().end();
void MachineIRBuilder::setInsertPt(MachineBasicBlock &MBB,
MachineBasicBlock::iterator II) {
assert(MBB.getParent() == &getMF() &&
"Basic block is in a different function");
this->MBB = &MBB;
this->II = II;
}

void MachineIRBuilder::recordInsertions(
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/AArch64/AArch64CallLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,10 @@ bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
MachineFunction &MF = MIRBuilder.getMF();
const Function &F = *MF.getFunction();

MachineInstrBuilder MIB = MIRBuilder.buildInstr(AArch64::RET_ReallyLR);
assert(MIB.getInstr() && "Unable to build a return instruction?!");

auto MIB = MIRBuilder.buildInstrNoInsert(AArch64::RET_ReallyLR);
assert(((Val && VReg) || (!Val && !VReg)) && "Return value without a vreg");
bool Success = true;
if (VReg) {
MIRBuilder.setInstr(*MIB.getInstr(), /* Before */ true);
const AArch64TargetLowering &TLI = *getTLI<AArch64TargetLowering>();
CCAssignFn *AssignFn = TLI.CCAssignFnForReturn(F.getCallingConv());
MachineRegisterInfo &MRI = MF.getRegInfo();
Expand All @@ -221,9 +219,11 @@ bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
});

OutgoingArgHandler Handler(MIRBuilder, MRI, MIB);
return handleAssignments(MIRBuilder, AssignFn, SplitArgs, Handler);
Success = handleAssignments(MIRBuilder, AssignFn, SplitArgs, Handler);
}
return true;

MIRBuilder.insertInstr(MIB);
return Success;
}

bool AArch64CallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
Expand Down
Loading

0 comments on commit 05cc485

Please sign in to comment.