From 89d59a91790b8fe6155c8cca16ac801e0bd4c863 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 28 Mar 2024 13:59:05 -0400 Subject: [PATCH] optimize: Delete incoming unreachable edges from PhiNode (#53877) Incoming IR very rarely contains PhiNodes, but we do allow it to make things easier on downstream packages like Diffractor that want to generate the same code structures in both typed and untyped mode. However, this does of course mean that once inference is finished, any PhiNodes in the original source must be adjusted to maintain IRCode invariants. One particular important invariant here is that the edges list in a PhiNode must match the predecessor list, so in particular if a predecessor becomes unreachable during inference, we must filter that edge before passing it on to the optimizer. --------- Co-authored-by: Shuhei Kadowaki --- base/compiler/optimize.jl | 14 +++++++ test/compiler/ssair.jl | 82 ++++++++++++++++++++++++++++++++++----- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index 325ba3eca0170..fbdd726a81e9a 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -1104,6 +1104,20 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState) code[i] = nothing end end + elseif isa(expr, PhiNode) + new_edges = Int32[] + new_vals = Any[] + for j = 1:length(expr.edges) + edge = expr.edges[j] + (edge in sv.unreachable || (ssavaluetypes[edge] === Union{} && !isa(code[edge], PhiNode))) && continue + push!(new_edges, edge) + if isassigned(expr.values, j) + push!(new_vals, expr.values[j]) + else + resize!(new_vals, length(new_edges)) + end + end + code[i] = PhiNode(new_edges, new_vals) end end end diff --git a/test/compiler/ssair.jl b/test/compiler/ssair.jl index 9082d1640e3be..9d5f7b617080d 100644 --- a/test/compiler/ssair.jl +++ b/test/compiler/ssair.jl @@ -9,16 +9,6 @@ include("irutils.jl") make_bb(preds, succs) = BasicBlock(Compiler.StmtRange(0, 0), preds, succs) -function make_ci(code) - ci = (Meta.@lower 1 + 1).args[1] - ci.code = code - nstmts = length(ci.code) - ci.ssavaluetypes = nstmts - ci.codelocs = fill(Int32(1), nstmts) - ci.ssaflags = fill(Int32(0), nstmts) - return ci -end - # TODO: this test is broken #let code = Any[ # GotoIfNot(SlotNumber(2), 4), @@ -763,3 +753,75 @@ end end end end + +# Test that things don't break if one branch of the frontend PhiNode becomes unreachable +const global_error_switch_const1::Bool = false +function gen_unreachable_phinode_edge1(world::UInt, source, args...) + ci = make_codeinfo(Any[ + # block 1 + GlobalRef(@__MODULE__, :global_error_switch_const1), + GotoIfNot(SSAValue(1), 4), + # block 2 + Expr(:call, identity, Argument(3)), + # block 3 + PhiNode(Int32[2, 3], Any[Argument(2), SSAValue(3)]), + ReturnNode(SSAValue(4)) + ]; slottypes=Any[Any,Int,Int]) + ci.slotnames = Symbol[:var"#self#", :x, :y] + return ci +end +@eval function f_unreachable_phinode_edge1(x, y) + $(Expr(:meta, :generated, gen_unreachable_phinode_edge1)) + $(Expr(:meta, :generated_only)) + #= no body =# +end +@test f_unreachable_phinode_edge1(1, 2) == 1 + +const global_error_switch_const2::Bool = true +function gen_unreachable_phinode_edge2(world::UInt, source, args...) + ci = make_codeinfo(Any[ + # block 1 + GlobalRef(@__MODULE__, :global_error_switch_const2), + GotoIfNot(SSAValue(1), 4), + # block 2 + Expr(:call, identity, Argument(3)), + # block 3 + PhiNode(Int32[2, 3], Any[Argument(2), SSAValue(3)]), + ReturnNode(SSAValue(4)) + ]; slottypes=Any[Any,Int,Int]) + ci.slotnames = Symbol[:var"#self#", :x, :y] + return ci +end +@eval function f_unreachable_phinode_edge2(x, y) + $(Expr(:meta, :generated, gen_unreachable_phinode_edge2)) + $(Expr(:meta, :generated_only)) + #= no body =# +end +@test f_unreachable_phinode_edge2(1, 2) == 2 + +global global_error_switch::Bool = true +function gen_must_throw_phinode_edge(world::UInt, source, _) + ci = make_codeinfo(Any[ + # block 1 + GlobalRef(@__MODULE__, :global_error_switch), + GotoIfNot(SSAValue(1), 4), + # block 2 + Expr(:call, error, "This error is expected"), + # block 3 + PhiNode(Int32[2, 3], Any[1, 2]), + ReturnNode(SSAValue(4)) + ]; slottypes=Any[Any]) + ci.slotnames = Symbol[:var"#self#"] + return ci +end +@eval function f_must_throw_phinode_edge() + $(Expr(:meta, :generated, gen_must_throw_phinode_edge)) + $(Expr(:meta, :generated_only)) + #= no body =# +end +let ir = first(only(Base.code_ircode(f_must_throw_phinode_edge))) + @test !any(@nospecialize(x)->isa(x,PhiNode), ir.stmts.stmt) +end +@test_throws ErrorException f_must_throw_phinode_edge() +global global_error_switch = false +@test f_must_throw_phinode_edge() == 1