From c9a556031832e0c5b41a072a6687a0e1dc0d932c Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Thu, 6 Jun 2024 01:49:28 +0900 Subject: [PATCH] inference: follow up #54323, override ssaflags with new cycle effects JuliaLang/julia#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, JuliaLang/julia#54323 breaks the test cases from JuliaLang/julia#51092, when backported to v1.11. On the surface this is because JuliaLang/julia#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. --- base/compiler/typeinfer.jl | 38 +++++++++++++++++++++++++++++++++++++- test/compiler/inference.jl | 9 +++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index 5abc92ee19eb79..2783d05b8fa2bf 100644 --- a/base/compiler/typeinfer.jl +++ b/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 diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index c45f5f11bf62e9..eca937dddc5aba 100644 --- a/test/compiler/inference.jl +++ b/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)