From 5faa86744187f270abc1adfa1d00a6837df9dec5 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 20 Jun 2023 19:35:54 +0000 Subject: [PATCH] Avoid creating invalid PhiNodes in IR passes 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. --- base/compiler/ssair/irinterp.jl | 2 +- base/compiler/ssair/passes.jl | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/base/compiler/ssair/irinterp.jl b/base/compiler/ssair/irinterp.jl index fc5085af426a1..1ba751f6d3fd4 100644 --- a/base/compiler/ssair/irinterp.jl +++ b/base/compiler/ssair/irinterp.jl @@ -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 diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 518a6512fc166..3b0159f5c3b85 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -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] @@ -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 @@ -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)