Skip to content

Commit

Permalink
inference: fix too conservative effects for recursive cycles
Browse files Browse the repository at this point in the history
The `:terminates` effect bit must be conservatively tainted unless
recursion cycle has been fully resolved. As for other effects, there's
no need to taint them at this moment because they will be tainted as we
try to resolve the cycle.

- fixes #52938
- xref #51092
  • Loading branch information
aviatesk committed May 8, 2024
1 parent 88d6633 commit 4ad87c9
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
10 changes: 8 additions & 2 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,8 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
update_valid_age!(caller, frame.valid_worlds)
isinferred = is_inferred(frame)
edge = isinferred ? mi : nothing
effects = isinferred ? frame.result.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_effects
effects = isinferred ? frame.result.ipo_effects : # effects are adjusted already within `finish` for ipo_effects
adjust_effects(effects_for_cycle(frame.ipo_effects), method)
exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
# propagate newly inferred source to the inliner, allowing efficient inlining w/o deserialization:
# note that this result is cached globally exclusively, so we can use this local result destructively
Expand All @@ -877,11 +878,16 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
# return the current knowledge about this cycle
frame = frame::InferenceState
update_valid_age!(caller, frame.valid_worlds)
effects = adjust_effects(Effects(), method)
effects = adjust_effects(effects_for_cycle(frame.ipo_effects), method)
exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
return EdgeCallResult(frame.bestguess, exc_bestguess, nothing, effects)
end

# The `:terminates` effect bit must be conservatively tainted unless recursion cycle has
# been fully resolved. As for other effects, there's no need to taint them at this moment
# because they will be tainted as we try to resolve the cycle.
effects_for_cycle(effects::Effects) = Effects(effects; terminates=false)

function cached_return_type(code::CodeInstance)
rettype = code.rettype
isdefined(code, :rettype_const) || return rettype
Expand Down
10 changes: 9 additions & 1 deletion test/compiler/AbstractInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,18 @@ function CC.concrete_eval_eligible(interp::Issue48097Interp,
end
@overlay Issue48097MT @noinline Core.throw_inexacterror(f::Symbol, ::Type{T}, val) where {T} = return
issue48097(; kwargs...) = return 42
@test_broken fully_eliminated(; interp=Issue48097Interp(), retval=42) do
@test fully_eliminated(; interp=Issue48097Interp(), retval=42) do
issue48097(; a=1f0, b=1.0)
end

# https://github.com/JuliaLang/julia/issues/52938
@newinterp Issue52938Interp
@MethodTable ISSUE_52938_MT
CC.method_table(interp::Issue52938Interp) = CC.OverlayMethodTable(CC.get_inference_world(interp), ISSUE_52938_MT)
inner52938(x, types::Type, args...; kwargs...) = x
outer52938(x) = @inline inner52938(x, Tuple{}; foo=Ref(42), bar=1)
@test fully_eliminated(outer52938, (Any,); interp=Issue52938Interp(), retval=Argument(2))

# Should not concrete-eval overlayed methods in semi-concrete interpretation
@newinterp OverlaySinInterp
@MethodTable OverlaySinMT
Expand Down
10 changes: 5 additions & 5 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@ Base.@assume_effects :terminates_globally function recur_termination1(x)
0 x < 20 || error("bad fact")
return x * recur_termination1(x-1)
end
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination1, (Int,)))
function recur_termination2()
Base.@assume_effects :total !:terminates_globally
recur_termination1(12)
end
@test_broken fully_eliminated(recur_termination2)
@test fully_eliminated(recur_termination2)
@test fully_eliminated() do; recur_termination2(); end

Base.@assume_effects :terminates_globally function recur_termination21(x)
Expand All @@ -104,15 +104,15 @@ Base.@assume_effects :terminates_globally function recur_termination21(x)
return recur_termination22(x)
end
recur_termination22(x) = x * recur_termination21(x-1)
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination21, (Int,)))
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination22, (Int,)))
function recur_termination2x()
Base.@assume_effects :total !:terminates_globally
recur_termination21(12) + recur_termination22(12)
end
@test_broken fully_eliminated(recur_termination2x)
@test fully_eliminated(recur_termination2x)
@test fully_eliminated() do; recur_termination2x(); end

# anonymous function support for `@assume_effects`
Expand Down

0 comments on commit 4ad87c9

Please sign in to comment.