Skip to content

Commit

Permalink
[MachineSink] Ensure that arthimetic that stores results onto the
Browse files Browse the repository at this point in the history
stack is not sunk into a setjmp construct, specifically, between the
longjmp destination and the test.  Addresses issue llvm#78.
  • Loading branch information
neboat committed Jul 1, 2023
1 parent 626d7fe commit fc99ffc
Show file tree
Hide file tree
Showing 2 changed files with 635 additions and 0 deletions.
45 changes: 45 additions & 0 deletions llvm/lib/CodeGen/MachineSink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ namespace {

bool hasStoreBetween(MachineBasicBlock *From, MachineBasicBlock *To,
MachineInstr &MI);
bool possiblyHasSetjmpBetween(MachineBasicBlock *From,
MachineBasicBlock *To, MachineInstr &MI);

/// Postpone the splitting of the given critical
/// edge (\p From, \p To).
Expand Down Expand Up @@ -849,6 +851,22 @@ bool MachineSinking::isProfitableToSinkTo(Register Reg, MachineInstr &MI,
return true;
}

// Helper function to check if MBB contains a terminator that might correspond
// with EH_SjLj_Setup.
static bool blockMayContainSetjmpSetup(const MachineBasicBlock *MBB,
const MachineBasicBlock *Succ) {
for (const MachineInstr &MI : MBB->terminators())
// It seems hard to check for EH_SjLj_Setup directly, since that instruction
// seems to be target-dependent. Instead we simply check if the terminator
// has unmodeled side effects.
if (MI.hasUnmodeledSideEffects() &&
llvm::any_of(MI.operands(), [&](const MachineOperand &Op) {
return Op.isMBB() && Op.getMBB() == Succ;
}))
return true;
return false;
}

/// Get the sorted sequence of successors for this MachineBasicBlock, possibly
/// computing it if it was not already cached.
SmallVector<MachineBasicBlock *, 4> &
Expand Down Expand Up @@ -1341,6 +1359,26 @@ static bool blockPrologueInterferes(MachineBasicBlock *BB,
return false;
}

// possiblyHasSetjmpBetween - Check for setjmps along the path from block From
// to block To.
bool MachineSinking::possiblyHasSetjmpBetween(MachineBasicBlock *From,
MachineBasicBlock *To,
MachineInstr &MI) {
// For now we examine just the predecessors of predecessors of To for possible
// setjmp-setup constructs.
for (MachineBasicBlock *BB : To->predecessors()) {
if (BB->hasAddressTaken() && PDT->dominates(To, BB)) {
for (MachineBasicBlock *Pred : BB->predecessors()) {
if (PDT->dominates(To, Pred)) {
if (blockMayContainSetjmpSetup(Pred, BB))
return true;
}
}
}
}
return false;
}

/// SinkInstruction - Determine whether it is safe to sink the specified machine
/// instruction out of its current block into a successor.
bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
Expand Down Expand Up @@ -1423,6 +1461,13 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
TryBreak = true;
}

// Don't sink instructions into successors of setjmps that may execute
// multiple times.
if (!TryBreak && possiblyHasSetjmpBetween(ParentBlock, SuccToSinkTo, MI)) {
LLVM_DEBUG(dbgs() << " *** NOTE: Possible setjmp setup found\n");
TryBreak = true;
}

// Otherwise we are OK with sinking along a critical edge.
if (!TryBreak)
LLVM_DEBUG(dbgs() << "Sinking along critical edge.\n");
Expand Down
Loading

0 comments on commit fc99ffc

Please sign in to comment.