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

@@ -231,11 +231,46 @@ function finish!(interp::AbstractInterpreter, caller::InferenceState)
     return nothing
 end
 
+# override statement flags for a frame within the cycle so that they are consistent with
+# the new effects that are valid for the cycle
+function cycle_ssaflags!(sv::InferenceState, callers_in_cycle::Vector{InferenceState},
+                         cycle_effects::Effects)
+    new_flags = flags_for_effects(cycle_effects)
+    old_currpc = sv.currpc
+    for i = 1:length(sv.src.ssaflags)
+        edges = sv.stmt_edges[i]
+        edges === nothing && continue
+        if any(callers_in_cycle) do caller::InferenceState
+                mi = caller.linfo
+                for edge in edges
+                    edge isa MethodInstance || continue
+                    if edge === mi
+                        return true
+                    end
+                end
+                return false
+            end
+            sv.currpc = i
+            set_curr_ssaflag!(sv, new_flags)
+        end
+    end
+    sv.currpc = old_currpc
+    return nothing
+end
+
 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)
+        has_cycle = false
+    elseif length(frames) == 1
+        @assert frames[1] === frame "invalid callers_in_cycle"
+        has_cycle = false
+    else
+        has_cycle = true
+    end
     cycle_valid_worlds = WorldRange()
     cycle_effects = EFFECTS_TOTAL
     for caller in frames
@@ -251,6 +286,7 @@ function _typeinf(interp::AbstractInterpreter, frame::InferenceState)
     for caller in frames
         caller.valid_worlds = cycle_valid_worlds
         caller.ipo_effects = cycle_effects
+        has_cycle && cycle_ssaflags!(caller, frames, cycle_effects)
         finish(caller, caller.interp)
     end
     for caller in frames

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)