Skip to content

Commit 1b8cff9

Browse files
authored
Reland "CFIFixup] Factor CFI remember/restore insertion into a helper (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.
1 parent 75d0281 commit 1b8cff9

File tree

1 file changed

+42
-18
lines changed

1 file changed

+42
-18
lines changed

llvm/lib/CodeGen/CFIFixup.cpp

+42-18
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,41 @@ findPrologueEnd(MachineFunction &MF, MachineBasicBlock::iterator &PrologueEnd) {
116116
return nullptr;
117117
}
118118

119+
// Represents the point within a basic block where we can insert an instruction.
120+
// Note that we need the MachineBasicBlock* as well as the iterator since the
121+
// iterator can point to the end of the block. Instructions are inserted
122+
// *before* the iterator.
123+
struct InsertionPoint {
124+
MachineBasicBlock *MBB;
125+
MachineBasicBlock::iterator Iterator;
126+
};
127+
128+
// Inserts a `.cfi_remember_state` instruction before PrologueEnd and a
129+
// `.cfi_restore_state` instruction before DstInsertPt. Returns an iterator
130+
// to the first instruction after the inserted `.cfi_restore_state` instruction.
131+
static InsertionPoint
132+
insertRememberRestorePair(const InsertionPoint &RememberInsertPt,
133+
const InsertionPoint &RestoreInsertPt) {
134+
MachineFunction &MF = *RememberInsertPt.MBB->getParent();
135+
const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
136+
137+
// Insert the `.cfi_remember_state` instruction.
138+
unsigned CFIIndex =
139+
MF.addFrameInst(MCCFIInstruction::createRememberState(nullptr));
140+
BuildMI(*RememberInsertPt.MBB, RememberInsertPt.Iterator, DebugLoc(),
141+
TII.get(TargetOpcode::CFI_INSTRUCTION))
142+
.addCFIIndex(CFIIndex);
143+
144+
// Insert the `.cfi_restore_state` instruction.
145+
CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestoreState(nullptr));
146+
147+
return {RestoreInsertPt.MBB,
148+
std::next(BuildMI(*RestoreInsertPt.MBB, RestoreInsertPt.Iterator,
149+
DebugLoc(), TII.get(TargetOpcode::CFI_INSTRUCTION))
150+
.addCFIIndex(CFIIndex)
151+
->getIterator())};
152+
}
153+
119154
bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
120155
const TargetFrameLowering &TFL = *MF.getSubtarget().getFrameLowering();
121156
if (!TFL.enableCFIFixup(MF))
@@ -174,15 +209,13 @@ bool CFIFixup::runOnMachineFunction(MachineFunction &MF) {
174209
// Every block inherits the frame state (as recorded in the unwind tables)
175210
// of the previous block. If the intended frame state is different, insert
176211
// compensating CFI instructions.
177-
const TargetInstrInfo &TII = *MF.getSubtarget().getInstrInfo();
178212
bool Change = false;
179213
// `InsertPt` always points to the point in a preceding block where we have to
180214
// insert a `.cfi_remember_state`, in the case that the current block needs a
181215
// `.cfi_restore_state`.
182-
MachineBasicBlock *InsertMBB = PrologueBlock;
183-
MachineBasicBlock::iterator InsertPt = PrologueEnd;
216+
InsertionPoint InsertPt = {PrologueBlock, PrologueEnd};
184217

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

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

213-
// Insert a `.cfi_remember_state` into the last block known to have a
214-
// stack frame.
215-
unsigned CFIIndex =
216-
MF.addFrameInst(MCCFIInstruction::createRememberState(nullptr));
217-
BuildMI(*InsertMBB, InsertPt, DebugLoc(),
218-
TII.get(TargetOpcode::CFI_INSTRUCTION))
219-
.addCFIIndex(CFIIndex);
220-
// Insert a `.cfi_restore_state` at the beginning of the current block.
221-
CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestoreState(nullptr));
222-
InsertPt = BuildMI(*CurrBB, CurrBB->begin(), DebugLoc(),
223-
TII.get(TargetOpcode::CFI_INSTRUCTION))
224-
.addCFIIndex(CFIIndex);
225-
++InsertPt;
226-
InsertMBB = &*CurrBB;
246+
// There's an earlier block known to have a stack frame. Insert a
247+
// `.cfi_remember_state` instruction into that block and a
248+
// `.cfi_restore_state` instruction at the beginning of the current block.
249+
InsertPt = insertRememberRestorePair(
250+
InsertPt, InsertionPoint{&*CurrBB, CurrBB->begin()});
227251
Change = true;
228252
} else if ((Info.StrongNoFrameOnEntry || !Info.HasFrameOnEntry) &&
229253
HasFrame) {

0 commit comments

Comments
 (0)