Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

post-opt analysis: fix conditional successor visitation logic #53518

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -531,17 +531,22 @@ function any_stmt_may_throw(ir::IRCode, bb::Int)
return false
end

function conditional_successors_may_throw(lazypostdomtree::LazyPostDomtree, ir::IRCode, bb::Int)
function visit_conditional_successors(callback, lazypostdomtree::LazyPostDomtree, ir::IRCode, bb::Int)
visited = BitSet((bb,))
worklist = Int[bb]
while !isempty(worklist)
thisbb = pop!(worklist)
thisbb = popfirst!(worklist)
for succ in ir.cfg.blocks[thisbb].succs
succ in visited && continue
push!(visited, succ)
postdominates(get!(lazypostdomtree), succ, thisbb) && continue
any_stmt_may_throw(ir, succ) && return true
push!(worklist, succ)
if postdominates(get!(lazypostdomtree), succ, bb)
# this successor is not conditional, so no need to visit it further
continue
elseif callback(succ)
return true
else
push!(worklist, succ)
end
end
end
return false
Expand Down Expand Up @@ -833,8 +838,10 @@ function ((; sv)::ScanStmt)(inst::Instruction, lstmt::Int, bb::Int)
# inconsistent region.
if !sv.result.ipo_effects.terminates
sv.all_retpaths_consistent = false
elseif conditional_successors_may_throw(sv.lazypostdomtree, sv.ir, bb)
# Check if there are potential throws that require
elseif visit_conditional_successors(sv.lazypostdomtree, sv.ir, bb) do succ::Int
return any_stmt_may_throw(sv.ir, succ)
end
# check if this `GotoIfNot` leads to conditional throws, which taints consistency
sv.all_retpaths_consistent = false
else
(; cfg, domtree) = get!(sv.lazyagdomtree)
Expand Down
5 changes: 5 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1382,3 +1382,8 @@ let; Base.Experimental.@force_compile; func52843(); end
# pointerref nothrow for invalid pointer
@test !Core.Compiler.intrinsic_nothrow(Core.Intrinsics.pointerref, Any[Type{Ptr{Vector{Int64}}}, Int, Int])
@test !Core.Compiler.intrinsic_nothrow(Core.Intrinsics.pointerref, Any[Type{Ptr{T}} where T, Int, Int])

# post-opt :consistent-cy analysis correctness
# 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)))
29 changes: 29 additions & 0 deletions test/compiler/ssair.jl
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,35 @@ let ci = code_lowered(()->@isdefined(_not_def_37919_), ())[1]
@test Core.Compiler.verify_ir(ir) === nothing
end

let code = Any[
# block 1
GotoIfNot(Argument(2), 4),
# block 2
GotoNode(3),
# block 3
Expr(:call, throw, "potential throw"),
# block 4
Expr(:call, Core.Intrinsics.add_int, Argument(3), Argument(4)),
GotoNode(6),
# block 5
ReturnNode(SSAValue(4))
]
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
push!(visited, succ)
return false
end
@test 2 ∈ visited
@test 3 ∈ 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)
end

# Test dynamic update of domtree with edge insertions and deletions in the
# following CFG:
#
Expand Down