Skip to content

Commit

Permalink
irinterp: Don't introduce invalid CFGs (#49797)
Browse files Browse the repository at this point in the history
This is yet another followup to #49692 and #49750.
With the introduced change, we kill the CFG edge
from the basic block with the discovered error to
its successors. However, we have an invariant in
the verifier that the CFG should always match the
IR. Turns out this is for good reason, as we assume
in a number of places (including, ironically in the
irinterp) that a GotoNode/GotoIfNot terminator means
that the BB has the corresponding number of successors
in the IR. Fix all this by killing the rest of the basic
block when we discover that it is unreachable and if
possible introducing an unreachable node at the end.
However, of course if the erroring statement is the
fallthrough terminator itself, there is no space for
an unreachable node. We fix this by tweaking the
verification to allow this case, as its really
no worse than the other problems with fall-through
terminators (#41476), but of course it would be good
to address that as part of a more general IR refactor.
  • Loading branch information
Keno authored May 13, 2023
1 parent 0a05a5b commit 7e1431f
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
15 changes: 10 additions & 5 deletions base/compiler/ssair/irinterp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,19 @@ function _ir_abstract_constant_propagation(interp::AbstractInterpreter, irsv::IR
inst = ir.stmts[idx][:inst]
typ = ir.stmts[idx][:type]
end
if idx == lstmt
process_terminator!(ir, inst, idx, bb, all_rets, bb_ip) && @goto residual_scan
(isa(inst, GotoNode) || isa(inst, GotoIfNot) || isa(inst, ReturnNode) || isexpr(inst, :enter)) && continue
end
if typ === Bottom && !isa(inst, PhiNode)
if typ === Bottom && !(isa(inst, PhiNode) || isa(inst, GotoNode) || isa(inst, GotoIfNot) || isa(inst, ReturnNode) || isexpr(inst, :enter))
kill_terminator_edges!(irsv, lstmt, bb)
if idx != lstmt
for idx2 in (idx+1:lstmt-1)
ir[SSAValue(idx2)] = nothing
end
ir[SSAValue(lstmt)][:inst] = ReturnNode()
end
break
end
if idx == lstmt
process_terminator!(ir, inst, idx, bb, all_rets, bb_ip) && @goto residual_scan
end
end
end
@goto compute_rt
Expand Down
10 changes: 8 additions & 2 deletions base/compiler/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,14 @@ function verify_ir(ir::IRCode, print::Bool=true,
end
isa(stmt, PhiNode) || break
end
@verify_error "Block $idx successors ($(block.succs)), does not match fall-through terminator ($terminator)"
error("")
if isempty(block.succs) && ir.stmts[idx][:type] == Union{}
# Allow fallthrough terminators that are known to error to
# be removed from the CFG. Ideally we'd add an unreachable
# here, but that isn't always possible.
else
@verify_error "Block $idx successors ($(block.succs)), does not match fall-through terminator ($terminator)"
error("")
end
end
end
end
Expand Down

0 comments on commit 7e1431f

Please sign in to comment.