From 1390613d29220acac1c375e102ae2f2428c2fb95 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Tue, 12 Jul 2022 18:09:23 +0900 Subject: [PATCH] Merge pull request #45993 from JuliaLang/avi/recursion-effects effects: relax recursion detection for effects analysis --- base/compiler/abstractinterpretation.jl | 64 ++++++++++++++++++------- test/compiler/effects.jl | 15 ++++++ 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 4feba83574a8e..31441407dc31a 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -652,14 +652,43 @@ function abstract_call_method(interp::AbstractInterpreter, method::Method, @nosp # this edge is known to terminate edge_effects = Effects(edge_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 - edge_effects = Effects(edge_effects; terminates=TRISTATE_UNKNOWN) + # 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 + edge_effects = Effects(edge_effects; terminates=ALWAYS_FALSE) + end end return MethodCallResult(rt, edgecycle, edgelimited, edge, edge_effects) end -# keeps result and context information of abstract method call, will be used by succeeding constant-propagation +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 rt edgecycle::Bool @@ -801,17 +830,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) @@ -922,8 +948,8 @@ function is_const_prop_profitable_arg(@nospecialize(arg)) isa(arg, PartialOpaque) && return true isa(arg, Const) || return true val = arg.val - # don't consider mutable values or Strings useful constants - return isa(val, Symbol) || isa(val, Type) || (!isa(val, String) && !ismutable(val)) + # don't consider mutable values useful constants + return isa(val, Symbol) || isa(val, Type) || !ismutable(val) end function is_const_prop_profitable_conditional(cnd::Conditional, fargs::Vector{Any}, sv::InferenceState) @@ -1275,7 +1301,7 @@ function abstract_apply(interp::AbstractInterpreter, argtypes::Vector{Any}, sv:: end cti = Any[Vararg{argt}] end - if _any(t -> t === Bottom, cti) + if any(@nospecialize(t) -> t === Bottom, cti) continue end for j = 1:length(ctypes) @@ -1954,6 +1980,8 @@ function abstract_eval_statement(interp::AbstractInterpreter, @nospecialize(e), for i = 3:length(e.args) if abstract_eval_value(interp, e.args[i], vtypes, sv) === Bottom t = Bottom + tristate_merge!(sv, EFFECTS_THROWS) + @goto t_computed end end cconv = e.args[5] diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index 55b99dee6a968..101adfb97683c 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -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 + # effects propagation for `Core.invoke` calls # https://github.com/JuliaLang/julia/issues/44763 global x44763::Int = 0