inference: follow up #54323, override ssaflags with new cycle effects

#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`.

This commit uses a somewhat hacky approach to resolve this. It
identifies statements involved in the cycle comparing `stmt_edges` to
`callers_in_cycle`, and updates `ssaflags` according to new cycle valid
effects if necessary. This resolves the issue, but ideally, it should be
implemented more safely with the new `edges` format that will be
implemented in the future. For now, this approach should be okay.

Author: aviatesk

base/compiler/typeinfer.jl

@@ -235,9 +235,35 @@ 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)
+        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
@@ -246,11 +272,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
@@ -265,8 +290,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)

test/compiler/inference.jl

@@ -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)