From 9df47f238c44a92ca0962efbc4c42173d0c5ce54 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Tue, 19 Mar 2024 20:10:11 +0900 Subject: [PATCH] post-opt: add more test cases for `visit_conditional_successors` (#53642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes the first problem that was found while digging into JuliaLang/julia#53613. It turns out that the post-domtree constructed from regular `IRCode` doesn't work for visiting conditional successors for post-opt analysis in cases like: ```julia julia> let code = Any[ # block 1 GotoIfNot(Argument(2), 3), # block 2 ReturnNode(Argument(3)), # block 3 (we should visit this block) Expr(:call, throw, "potential throw"), ReturnNode(), # unreachable ] ir = make_ircode(code; slottypes=Any[Any,Bool,Bool]) visited = BitSet() @test !Core.Compiler.visit_conditional_successors(CC.LazyPostDomtree(ir), ir, #=bb=#1) do succ::Int push!(visited, succ) return false end @test 2 ∉ visited @test 3 ∈ visited end Test Failed at REPL[14]:16 Expression: 2 ∉ visited Evaluated: 2 ∉ BitSet([2]) ``` This might mean that we need to fix on the `postdominates` end, but for now, this commit tries to get around it by using the augmented post domtree in `visit_conditional_successors`. Since the augmented post domtree is enforced to have a single return, we can keep using the current `postdominates` to fix the issue. However, this commit isn't enough to fix the NeuralNetworkReachability segfault as reported in #53613, and we need to tackle the second issue reported there too (https://github.com/JuliaLang/julia/issues/53613#issuecomment-1983243419). --- base/compiler/optimize.jl | 2 ++ test/compiler/effects.jl | 10 ++++++ test/compiler/ssair.jl | 66 ++++++++++++++++++++++++++++++++------- 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index 81b025e588f7f..325ba3eca0170 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -527,6 +527,8 @@ function any_stmt_may_throw(ir::IRCode, bb::Int) return false end +visit_conditional_successors(callback, ir::IRCode, bb::Int) = # used for test + visit_conditional_successors(callback, LazyPostDomtree(ir), ir, bb) function visit_conditional_successors(callback, lazypostdomtree::LazyPostDomtree, ir::IRCode, bb::Int) visited = BitSet((bb,)) worklist = Int[bb] diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index fa70c8de9d853..cdf797fb2a77b 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -1387,3 +1387,13 @@ let; Base.Experimental.@force_compile; func52843(); end # https://github.com/JuliaLang/julia/issues/53508 @test !Core.Compiler.is_consistent(Base.infer_effects(getindex, (UnitRange{Int},Int))) @test !Core.Compiler.is_consistent(Base.infer_effects(getindex, (Base.OneTo{Int},Int))) + +@noinline f53613() = @assert isdefined(@__MODULE__, :v53613) +g53613() = f53613() +@test !Core.Compiler.is_consistent(Base.infer_effects(f53613)) +@test_broken !Core.Compiler.is_consistent(Base.infer_effects(g53613)) +@test_throws AssertionError f53613() +@test_throws AssertionError g53613() +global v53613 = nothing +@test f53613() === nothing +@test g53613() === nothing diff --git a/test/compiler/ssair.jl b/test/compiler/ssair.jl index 70a2ab4a3acd9..9082d1640e3be 100644 --- a/test/compiler/ssair.jl +++ b/test/compiler/ssair.jl @@ -233,35 +233,79 @@ let code = Any[ end # issue #37919 -let ci = code_lowered(()->@isdefined(_not_def_37919_), ())[1] +let ci = only(code_lowered(()->@isdefined(_not_def_37919_), ())) ir = Core.Compiler.inflate_ir(ci) @test Core.Compiler.verify_ir(ir) === nothing end let code = Any[ # block 1 - GotoIfNot(Argument(2), 4), + GotoIfNot(Argument(2), 4) # block 2 - GotoNode(3), + Expr(:call, throw, "potential throw") + ReturnNode() # unreachable # block 3 - Expr(:call, throw, "potential throw"), + ReturnNode(Argument(3)) + ] + ir = make_ircode(code; slottypes=Any[Any,Bool,Int]) + visited = BitSet() + @test !Core.Compiler.visit_conditional_successors(ir, #=bb=#1) do succ::Int + push!(visited, succ) + return false + end + @test 2 ∈ visited + @test 3 ∈ visited + oc = Core.OpaqueClosure(ir) + @test oc(false, 1) == 1 + @test_throws "potential throw" oc(true, 1) +end + +let code = Any[ + # block 1 + GotoIfNot(Argument(2), 3) + # block 2 + ReturnNode(Argument(3)) + # block 3 + Expr(:call, throw, "potential throw") + ReturnNode() # unreachable + ] + ir = make_ircode(code; slottypes=Any[Any,Bool,Int]) + visited = BitSet() + @test !Core.Compiler.visit_conditional_successors(ir, #=bb=#1) do succ::Int + push!(visited, succ) + return false + end + @test 2 ∈ visited + @test 3 ∈ visited + oc = Core.OpaqueClosure(ir) + @test oc(true, 1) == 1 + @test_throws "potential throw" oc(false, 1) +end + +let code = Any[ + # block 1 + GotoIfNot(Argument(2), 5) + # block 2 + GotoNode(3) + # block 3 + Expr(:call, throw, "potential throw") + ReturnNode() # block 4 - Expr(:call, Core.Intrinsics.add_int, Argument(3), Argument(4)), - GotoNode(6), + Expr(:call, Core.Intrinsics.add_int, Argument(3), Argument(4)) + GotoNode(7) # block 5 - ReturnNode(SSAValue(4)) + ReturnNode(SSAValue(5)) ] ir = make_ircode(code; slottypes=Any[Any,Bool,Int,Int]) - lazypostdomtree = Core.Compiler.LazyPostDomtree(ir) visited = BitSet() - @test !Core.Compiler.visit_conditional_successors(lazypostdomtree, ir, #=bb=#1) do succ::Int + @test !Core.Compiler.visit_conditional_successors(ir, #=bb=#1) do succ::Int push!(visited, succ) return false end @test 2 ∈ visited @test 3 ∈ visited - @test 4 ∉ visited - @test 5 ∉ visited + @test 4 ∈ visited + @test 5 ∈ visited oc = Core.OpaqueClosure(ir) @test oc(false, 1, 1) == 2 @test_throws "potential throw" oc(true, 1, 1)