Skip to content

Commit

Permalink
inference: Fix correctness and ensure termination in the presence of …
Browse files Browse the repository at this point in the history
…PhiNodes

There's two related, but distinct, issues here:
1. We were not using `tmerge` for merging SSA results inside loops,
   which could cause infinite looping. In the absence of PhiNodes, things
   usually have to go through a slot to be able to make the round trip,
   which would usually put a PhiNode on the path, but it's possible there
   may be other ways to smuggle things around (e.g. through exception handling).

2. We were not properly accounting for the fact that PhiNode uses do not need to
   be linearly ordered in the same BB, so we were getting the type of the testcase
   here incorrect by failing to re-schedule the PhiNode.

The first of these shows up in the Diffractor test suite, the second was found
by writing the test case.
  • Loading branch information
Keno committed Mar 27, 2024
1 parent 6737a1d commit 2e01bd5
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
8 changes: 4 additions & 4 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -685,16 +685,16 @@ _topmod(sv::InferenceState) = _topmod(frame_module(sv))
function record_ssa_assign!(𝕃ᵢ::AbstractLattice, ssa_id::Int, @nospecialize(new), frame::InferenceState)
ssavaluetypes = frame.ssavaluetypes
old = ssavaluetypes[ssa_id]
if old !== NOT_FOUND
new = tmerge(𝕃ᵢ, new, old)
end
if old === NOT_FOUND || !is_lattice_equal(𝕃ᵢ, new, old)
ssavaluetypes[ssa_id] = new
W = frame.ip
for r in frame.ssavalue_uses[ssa_id]
if was_reached(frame, r)
usebb = block_for_inst(frame.cfg, r)
# We're guaranteed to visit the statement if it's in the current
# basic block, since SSA values can only ever appear after their
# def.
if usebb != frame.currbb
if usebb != frame.currbb || r < ssa_id
push!(W, usebb)
end
end
Expand Down
28 changes: 28 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5645,3 +5645,31 @@ let t = ntuple(i -> i % 8 == 1 ? Int64 : Float64, 4000)
@test only(Base.return_types(Base.promote_typeof, t)) == Type{Float64}
@test only(Base.return_types(vcat, t)) == Vector{Float64}
end

# Infinite loop in inference on SSA assignment
const stop_infinite_loop::Base.Threads.Atomic{Bool} = Base.Threads.Atomic{Bool}(false)
function gen_infinite_loop_ssa_generator(world::UInt, source, _)
ci = make_codeinfo(Any[
# Block 1
(),
# Block 2
PhiNode(Int32[1, 5], Any[SSAValue(1), SSAValue(3)]),
Expr(:call, tuple, SSAValue(2)),
Expr(:call, getindex, GlobalRef(@__MODULE__, :stop_infinite_loop)),
GotoIfNot(SSAValue(4), 2),
# Block 3
ReturnNode(SSAValue(2))
]; slottypes=Any[Any])
ci.slotnames = Symbol[:var"#self#"]
ci
end

@eval function gen_infinite_loop_ssa()
$(Expr(:meta, :generated, gen_infinite_loop_ssa_generator))
$(Expr(:meta, :generated_only))
#= no body =#
end

# We want to make sure that both this returns `Tuple` and that
# it doesn't infinite loop inside inference.
@test Core.Compiler.return_type(gen_infinite_loop_ssa, Tuple{}) === Tuple

0 comments on commit 2e01bd5

Please sign in to comment.