Skip to content

Commit

Permalink
Reland "CFIFixup] Factor CFI remember/restore insertion into a helper…
Browse files Browse the repository at this point in the history
… (NFC)" (llvm#113387)

The original patch (ac1a01f) dereferenced an end iterator, breaking some
tests (e.g. https://lab.llvm.org/buildbot/#/builders/17/builds/3116).
This updated patch only accesses the iterator when it's valid.
  • Loading branch information
dhoekwater authored and NoumanAmir657 committed Nov 4, 2024
1 parent 4d062ee commit 0be8de8
Showing 1 changed file with 42 additions and 18 deletions.
60 changes: 42 additions & 18 deletions llvm/lib/CodeGen/CFIFixup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,41 @@ findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
return nullptr;
}

// Represents the point within a basic block where we can insert an instruction.
// Note that we need the MachineBasicBlock* as well as the iterator since the
// iterator can point to the end of the block. Instructions are inserted
// *before* the iterator.
struct InsertionPoint {
MachineBasicBlock *MBB;
MachineBasicBlock::iterator Iterator;
};

// Inserts a `.cfi_remember_state` instruction before PrologueEnd and a
// `.cfi_restore_state` instruction before DstInsertPt. Returns an iterator
// to the first instruction after the inserted `.cfi_restore_state` instruction.
static InsertionPoint
insertRememberRestorePair(const InsertionPoint &RememberInsertPt,
const InsertionPoint &RestoreInsertPt) {
MachineFunction &MF = *RememberInsertPt.MBB->getParent();
const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();

// Insert the `.cfi_remember_state` instruction.
unsigned CFIIndex =
MF.addFrameInst(MCCFIInstruction::createRememberState(nullptr));
BuildMI(*RememberInsertPt.MBB, RememberInsertPt.Iterator, DebugLoc(),
TII.get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex);

// Insert the `.cfi_restore_state` instruction.
CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestoreState(nullptr));

return {RestoreInsertPt.MBB,
std::next(BuildMI(*RestoreInsertPt.MBB, RestoreInsertPt.Iterator,
DebugLoc(), TII.get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex)
->getIterator())};
}

bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
const TargetFrameLowering &TFL = *MF.getSubtarget().getFrameLowering();
if (!TFL.enableCFIFixup(MF))
Expand Down Expand Up @@ -174,15 +209,13 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
// Every block inherits the frame state (as recorded in the unwind tables)
// of the previous block. If the intended frame state is different, insert
// compensating CFI instructions.
const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
bool Change = false;
// `InsertPt` always points to the point in a preceding block where we have to
// insert a `.cfi_remember_state`, in the case that the current block needs a
// `.cfi_restore_state`.
MachineBasicBlock *InsertMBB = PrologueBlock;
MachineBasicBlock::iterator InsertPt = PrologueEnd;
InsertionPoint InsertPt = {PrologueBlock, PrologueEnd};

assert(InsertPt != PrologueBlock->begin() &&
assert(PrologueEnd != PrologueBlock->begin() &&
"Inconsistent notion of \"prologue block\"");

// No point starting before the prologue block.
Expand Down Expand Up @@ -210,20 +243,11 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
if (!Info.StrongNoFrameOnEntry && Info.HasFrameOnEntry && !HasFrame) {
// Reset to the "after prologue" state.

// Insert a `.cfi_remember_state` into the last block known to have a
// stack frame.
unsigned CFIIndex =
MF.addFrameInst(MCCFIInstruction::createRememberState(nullptr));
BuildMI(*InsertMBB, InsertPt, DebugLoc(),
TII.get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex);
// Insert a `.cfi_restore_state` at the beginning of the current block.
CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestoreState(nullptr));
InsertPt = BuildMI(*CurrBB, CurrBB->begin(), DebugLoc(),
TII.get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex);
++InsertPt;
InsertMBB = &*CurrBB;
// There's an earlier block known to have a stack frame. Insert a
// `.cfi_remember_state` instruction into that block and a
// `.cfi_restore_state` instruction at the beginning of the current block.
InsertPt = insertRememberRestorePair(
InsertPt, InsertionPoint{&*CurrBB, CurrBB->begin()});
Change = true;
} else if ((Info.StrongNoFrameOnEntry || !Info.HasFrameOnEntry) &&
HasFrame) {
Expand Down

0 comments on commit 0be8de8

Please sign in to comment.