Skip to content

Commit

Permalink
effects: relax recursion detection for effects analysis
Browse files Browse the repository at this point in the history
In a similar spirit to JuliaLang#40561, we can relax the recursion detection to
guarantee `:terminates` effect and allow the effects analysis to not
taint `:terminates` effect when there are no cycles in `MethodInstance`s
in a call graph.

fix JuliaLang#45781
  • Loading branch information
aviatesk committed Jul 12, 2022
1 parent c72625f commit c9c0c4d
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 14 deletions.
53 changes: 39 additions & 14 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -617,9 +617,13 @@ function abstract_call_method(interp::AbstractInterpreter, method::Method, @nosp
# this edge is known to terminate
effects = Effects(effects; terminates=ALWAYS_TRUE)
elseif edgecycle
# Some sort of recursion was detected. Even if we did not limit types,
# we cannot guarantee that the call will terminate
effects = Effects(effects; terminates=ALWAYS_FALSE)
# Some sort of recursion was detected.
if edge !== nothing && !edgelimited && !is_edge_recursed(edge, sv)
# no `MethodInstance` cycles -- don't taint :terminate
else
# we cannot guarantee that the call will terminate
effects = Effects(effects; terminates=ALWAYS_FALSE)
end
end

return MethodCallResult(rt, edgecycle, edgelimited, edge, effects)
Expand Down Expand Up @@ -691,6 +695,30 @@ function matches_sv(parent::InferenceState, sv::InferenceState)
return parent.linfo.def === sv.linfo.def && sv_method2 === parent_method2
end

function is_edge_recursed(edge::MethodInstance, sv::InferenceState)
return any(InfStackUnwind(sv)) do infstate
return edge === infstate.linfo
end
end

function is_method_recursed(method::Method, sv::InferenceState)
return any(InfStackUnwind(sv)) do infstate
return method === infstate.linfo.def
end
end

function is_constprop_edge_recursed(edge::MethodInstance, sv::InferenceState)
return any(InfStackUnwind(sv)) do infstate
return edge === infstate.linfo && any(infstate.result.overridden_by_const)
end
end

function is_constprop_method_recursed(method::Method, sv::InferenceState)
return any(InfStackUnwind(sv)) do infstate
return method === infstate.linfo.def && any(infstate.result.overridden_by_const)
end
end

# keeps result and context information of abstract_method_call, which will later be used for
# backedge computation, and concrete evaluation or constant-propagation
struct MethodCallResult
Expand Down Expand Up @@ -836,17 +864,14 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter, resul
if inf_result === nothing
# if there might be a cycle, check to make sure we don't end up
# calling ourselves here.
let result = result # prevent capturing
if result.edgecycle && _any(InfStackUnwind(sv)) do infstate
# if the type complexity limiting didn't decide to limit the call signature (`result.edgelimited = false`)
# we can relax the cycle detection by comparing `MethodInstance`s and allow inference to
# propagate different constant elements if the recursion is finite over the lattice
return (result.edgelimited ? match.method === infstate.linfo.def : mi === infstate.linfo) &&
any(infstate.result.overridden_by_const)
end
add_remark!(interp, sv, "[constprop] Edge cycle encountered")
return nothing
end
if result.edgecycle && (result.edgelimited ?
is_constprop_method_recursed(match.method, sv) :
# if the type complexity limiting didn't decide to limit the call signature (`result.edgelimited = false`)
# we can relax the cycle detection by comparing `MethodInstance`s and allow inference to
# propagate different constant elements if the recursion is finite over the lattice
is_constprop_edge_recursed(mi, sv))
add_remark!(interp, sv, "[constprop] Edge cycle encountered")
return nothing
end
inf_result = InferenceResult(mi, (arginfo, sv))
if !any(inf_result.overridden_by_const)
Expand Down
15 changes: 15 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,21 @@ include("irutils.jl")
for i = 1:n; end
end |> !Core.Compiler.is_terminates

# interprocedural-recursion should taint `terminates` **appropriately**
function sumrecur(a, x)
isempty(a) && return x
return sumrecur(Base.tail(a), x + first(a))
end
@test Base.infer_effects(sumrecur, (Tuple{Int,Int,Int},Int)) |> Core.Compiler.is_terminates
@test Base.infer_effects(sumrecur, (Tuple{Int,Int,Int,Vararg{Int}},Int)) |> !Core.Compiler.is_terminates

# https://github.com/JuliaLang/julia/issues/45781
@test Base.infer_effects((Float32,)) do a
out1 = promote_type(Irrational{}, Bool)
out2 = sin(a)
out1, out2
end |> Core.Compiler.is_terminates

# refine :consistent-cy effect inference using the return type information
@test Base.infer_effects((Any,)) do x
taint = Ref{Any}(x) # taints :consistent-cy, but will be adjusted
Expand Down

0 comments on commit c9c0c4d

Please sign in to comment.