Skip to content

Commit f44f531

Browse files
committed
Fix LICM GuardType hoisting segfault: check deopt metadata for loop-body refs
LICM was hoisting GuardType instructions from loop bodies to preheaders based only on explicit operands being loop-invariant. However, GuardType inherits from DeoptBase and carries a FrameState with live_regs that reference registers defined inside the loop body. When the hoisted guard fired deoptimisation in the preheader, it tried to materialise values that were not yet defined, causing a segfault. Fix: replace allOperandsOutsideLoop (which only checks GetOperand) with allUsesOutsideLoop (which uses visitUses to check ALL register references including FrameState stack/locals and live_regs). Guards whose deopt metadata references loop-body-defined registers are no longer hoisted. Fixes: nqueens benchmark segfault (Bug 2).
1 parent 89e86ee commit f44f531

1 file changed

Lines changed: 31 additions & 19 deletions

File tree

cinderx/Jit/hir/licm.cpp

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,26 +106,38 @@ bool isHoistableGuard(const Instr& instr) {
106106
return instr.IsGuardType() || instr.IsGuardIs();
107107
}
108108

109-
// Check if all operands of an instruction are defined outside the loop.
110-
bool allOperandsOutsideLoop(
111-
const Instr& instr,
109+
// Check if a register is defined outside the loop.
110+
bool isDefinedOutsideLoop(
111+
Register* reg,
112112
const std::unordered_set<BasicBlock*>& loop_body) {
113-
for (size_t i = 0; i < instr.NumOperands(); i++) {
114-
Register* operand = instr.GetOperand(i);
115-
if (operand == nullptr) {
116-
continue;
117-
}
118-
Instr* def = operand->instr();
119-
if (def == nullptr) {
120-
continue; // Function argument or constant — always outside loop
121-
}
122-
BasicBlock* def_block = def->block();
123-
if (def_block != nullptr && loop_body.count(def_block) > 0) {
124-
// Operand defined inside the loop — not hoistable
125-
return false;
126-
}
113+
if (reg == nullptr) {
114+
return true;
115+
}
116+
Instr* def = reg->instr();
117+
if (def == nullptr) {
118+
return true; // Function argument or constant — always outside loop
127119
}
128-
return true;
120+
BasicBlock* def_block = def->block();
121+
return def_block == nullptr || loop_body.count(def_block) == 0;
122+
}
123+
124+
// Check if all uses of an instruction are defined outside the loop.
125+
// For DeoptBase instructions (GuardType, GuardIs), this also checks the
126+
// FrameState and live_regs — registers referenced by deoptimisation
127+
// metadata must also be defined outside the loop, otherwise deopt after
128+
// hoisting would reference uninitialised values and segfault.
129+
bool allUsesOutsideLoop(
130+
Instr& instr,
131+
const std::unordered_set<BasicBlock*>& loop_body) {
132+
bool all_outside = true;
133+
instr.visitUses([&](Register*& reg) -> bool {
134+
if (!isDefinedOutsideLoop(reg, loop_body)) {
135+
all_outside = false;
136+
return false; // Stop visiting
137+
}
138+
return true;
139+
});
140+
return all_outside;
129141
}
130142

131143
// Hoist loop-invariant guards from a single loop to its preheader.
@@ -147,7 +159,7 @@ int hoistInvariantGuards(LoopInfo& loop) {
147159
if (instr.IsPhi()) {
148160
continue; // Never hoist phi nodes
149161
}
150-
if (allOperandsOutsideLoop(instr, loop.body)) {
162+
if (allUsesOutsideLoop(instr, loop.body)) {
151163
to_hoist.push_back(&instr);
152164
}
153165
}

0 commit comments

Comments
 (0)