Skip to content

Commit

Permalink
Avoid creating invalid PhiNodes in IR passes
Browse files Browse the repository at this point in the history
As of #50158, irverify catches cases where PhiNodes show up in the
middle of a basic block (which is illegal). Unfortunately, it turns
out there were two cases in Base, where we created just such code:

1. When cfg_simplify! merged basic blocks, it didn't bother to delete
   (resp, replace by the one incoming edge) the PhiNodes in the basic
   block it was merging.

2. In irinterp we try to delete instructions that result in constants.
   This is not legal if the instruction is a PhiNode.

The second of these is somewhat unfortunate, but any subsequent compaction
will of course take care of it, so I don't think it's a huge issue
to just disable the replacement.
  • Loading branch information
Keno committed Jun 20, 2023
1 parent d48e17b commit 5faa867
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
2 changes: 1 addition & 1 deletion base/compiler/ssair/irinterp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ function reprocess_instruction!(interp::AbstractInterpreter, idx::Int, bb::Union
if rt !== nothing
if isa(rt, Const)
ir.stmts[idx][:type] = rt
if is_inlineable_constant(rt.val) && (ir.stmts[idx][:flag] & IR_FLAG_EFFECT_FREE) != 0
if is_inlineable_constant(rt.val) && !isa(inst, PhiNode) && (ir.stmts[idx][:flag] & IR_FLAG_EFFECT_FREE) != 0
ir.stmts[idx][:inst] = quoted(rt.val)
end
return true
Expand Down
11 changes: 10 additions & 1 deletion base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2246,6 +2246,7 @@ function cfg_simplify!(ir::IRCode)
result_idx = 1
for (idx, orig_bb) in enumerate(result_bbs)
ms = orig_bb
bb_start = true
while ms != 0
for i in bbs[ms].stmts
node = ir.stmts[i]
Expand Down Expand Up @@ -2304,7 +2305,14 @@ function cfg_simplify!(ir::IRCode)
isassigned(renamed_values, old_index) && kill_current_use!(compact, renamed_values[old_index])
end
end
compact.result[compact.result_idx][:inst] = PhiNode(edges, values)
if length(edges) == 0 || (length(edges) == 1 && !isassigned(values, 1))
compact.result[compact.result_idx][:inst] = nothing
elseif length(edges) == 1 && !bb_start
compact.result[compact.result_idx][:inst] = values[1]
else
@assert bb_start
compact.result[compact.result_idx][:inst] = PhiNode(edges, values)
end
else
ri = process_node!(compact, compact.result_idx, node, i, i, ms, true)
if ri == compact.result_idx
Expand All @@ -2318,6 +2326,7 @@ function cfg_simplify!(ir::IRCode)
compact.result_idx += 1
end
ms = merged_succ[ms]
bb_start = false
end
end
compact.idx = length(ir.stmts)
Expand Down

0 comments on commit 5faa867

Please sign in to comment.