Merge pull request JuliaLang#45993 from JuliaLang/avi/recursion-effects

effects: relax recursion detection for effects analysis

base/compiler/abstractinterpretation.jl

@@ -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)
@@ -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
@@ -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)
@@ -963,8 +988,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)
@@ -1345,7 +1370,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)
@@ -2031,6 +2056,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]

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
+
 # 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