Skip to content

Commit

Permalink
inference: follow up #54323, override ssaflags with new cycle effects
Browse files Browse the repository at this point in the history
#54323 ensures that all frames within a cycle have the
same, cycle valid effects. However, `src.ssaflags` is calculated using
partial effects, so when the effects of a `frame` within the cycle are
updated, there would be an inconsistency between `frame.ipo_effects` and
`frame.src.ssaflags`. Due to this inconsistency, #54323
breaks the test cases from #51092, when backported to
v1.11. On the surface this is because #52999 hasn't been
backported to v1.11, but the fundamental issue lies in this
inconsistency between cycle effects and `ssaflags`.

To resolve this issue, this commit traverses `cycle_backedges` to visit
statements involved in the cycle, and updates each `ssaflags` according
to new cycle valid effects if necessary.
  • Loading branch information
aviatesk committed Jun 6, 2024
1 parent ec32170 commit 8ac9368
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 7 deletions.
54 changes: 47 additions & 7 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,36 @@ function _typeinf(interp::AbstractInterpreter, frame::InferenceState)
typeinf_nocycle(interp, frame) || return false # frame is now part of a higher cycle
# with no active ip's, frame is done
frames = frame.callers_in_cycle
isempty(frames) && push!(frames, frame)
if isempty(frames)
push!(frames, frame)
finish_nocycle(interp, frame)
elseif length(frames) == 1
@assert frames[1] === frame "invalid callers_in_cycle"
finish_nocycle(interp, frame)
else
finish_cycle(interp, frames)
end
empty!(frames)
return true
end

function finish_nocycle(::AbstractInterpreter, frame::InferenceState)
frame.dont_work_on_me = true
finish(frame, frame.interp)
opt = frame.result.src
if opt isa OptimizationState # implies `may_optimize(caller.interp) === true`
optimize(frame.interp, opt, frame.result)
end
finish!(frame.interp, frame)
if is_cached(frame)
cache_result!(frame.interp, frame.result)
end
return nothing
end

function finish_cycle(::AbstractInterpreter, frames::Vector{InferenceState})
cycle_valid_worlds = WorldRange()
cycle_effects = EFFECTS_TOTAL
cycle_valid_effects = EFFECTS_TOTAL
for caller in frames
@assert !(caller.dont_work_on_me)
caller.dont_work_on_me = true
Expand All @@ -246,11 +273,10 @@ function _typeinf(interp::AbstractInterpreter, frame::InferenceState)
# that are simply the intersection of each partial computation, without having
# dependencies on each other (unlike rt and exct)
cycle_valid_worlds = intersect(cycle_valid_worlds, caller.valid_worlds)
cycle_effects = merge_effects(cycle_effects, caller.ipo_effects)
cycle_valid_effects = merge_effects(cycle_valid_effects, caller.ipo_effects)
end
for caller in frames
caller.valid_worlds = cycle_valid_worlds
caller.ipo_effects = cycle_effects
adjust_cycle_frame!(caller, cycle_valid_worlds, cycle_valid_effects)
finish(caller, caller.interp)
end
for caller in frames
Expand All @@ -265,8 +291,22 @@ function _typeinf(interp::AbstractInterpreter, frame::InferenceState)
cache_result!(caller.interp, caller.result)
end
end
empty!(frames)
return true
return nothing
end

function adjust_cycle_frame!(sv::InferenceState, cycle_valid_worlds::WorldRange, cycle_valid_effects::Effects)
sv.valid_worlds = cycle_valid_worlds
sv.ipo_effects = cycle_valid_effects
# traverse the callees of this cycle that are tracked within `sv.cycle_backedges`
# and adjust their statements so that they are consistent with the new `cycle_valid_effects`
new_flags = flags_for_effects(cycle_valid_effects)
for (callee, pc) in sv.cycle_backedges
old_currpc = callee.currpc
callee.currpc = pc
set_curr_ssaflag!(callee, new_flags, IR_FLAGS_EFFECTS)
callee.currpc = old_currpc
end
return nothing
end

function is_result_constabi_eligible(result::InferenceResult)
Expand Down
9 changes: 9 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5291,6 +5291,15 @@ end
foo51090(b) = return bar51090(b)
@test !fully_eliminated(foo51090, (Int,))

Base.@assume_effects :terminates_globally @noinline function bar51090_terminates(b)
b == 0 && return
r = foo51090_terminates(b - 1)
Base.donotdelete(b)
return r
end
foo51090_terminates(b) = return bar51090_terminates(b)
@test !fully_eliminated(foo51090_terminates, (Int,))

# exploit throwness from concrete eval for intrinsics
@test Base.return_types() do
Base.or_int(true, 1)
Expand Down

0 comments on commit 8ac9368

Please sign in to comment.