Skip to content

Commit

Permalink
inference: add missing limit-cycle poisoning
Browse files Browse the repository at this point in the history
Seems to have been missing since the original implementation of IPO constant-prop.
It's impressive how long we got away with just poisoning the cache with `Any`
if there was a self-recursive call.

Fix #29923
  • Loading branch information
vtjnash committed Nov 20, 2018
1 parent 02d8da2 commit 33549ba
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 22 deletions.
20 changes: 1 addition & 19 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -310,25 +310,7 @@ function abstract_call_method(method::Method, @nospecialize(sig), sparams::Simpl
# (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
return Any, false, nothing
end
infstate = sv
topmost = topmost::InferenceState
while !(infstate === topmost.parent)
if call_result_unused(infstate)
# If we won't propagate the result any further (since it's typically unused),
# it's OK that we keep and cache the "limited" result in the parents
# (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
# TODO: we might be able to halt progress much more strongly here,
# since now we know we won't be able to keep anything much that we learned.
# We were mainly only here to compute the calling convention return type,
# but in most situations now, we are unlikely to be able to use that information.
break
end
infstate.limited = true
for infstate_cycle in infstate.callers_in_cycle
infstate_cycle.limited = true
end
infstate = infstate.parent
end
poison_callstack(sv, topmost::InferenceState, true)
sig = newsig
sparams = svec()
end
Expand Down
22 changes: 22 additions & 0 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,28 @@ function add_mt_backedge!(mt::Core.MethodTable, @nospecialize(typ), caller::Infe
nothing
end

function poison_callstack(infstate::InferenceState, topmost::InferenceState, poison_topmost::Bool)
poison_topmost && (topmost = topmost.parent)
while !(infstate === topmost)
if call_result_unused(infstate)
# If we won't propagate the result any further (since it's typically unused),
# it's OK that we keep and cache the "limited" result in the parents
# (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
# TODO: we might be able to halt progress much more strongly here,
# since now we know we won't be able to keep anything much that we learned.
# We were mainly only here to compute the calling convention return type,
# but in most situations now, we are unlikely to be able to use that information.
break
end
infstate.limited = true
for infstate_cycle in infstate.callers_in_cycle
infstate_cycle.limited = true
end
infstate = infstate.parent
infstate === nothing && return
end
end

function is_specializable_vararg_slot(@nospecialize(arg), nargs::Int, vargs::Vector{Any})
return (isa(arg, Slot) && slot_id(arg) == nargs && !isempty(vargs))
end
Expand Down
15 changes: 12 additions & 3 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ function typeinf(result::InferenceResult, cached::Bool, params::Params)
end

function typeinf(frame::InferenceState)
cached = frame.cached
typeinf_nocycle(frame) || return false # frame is now part of a higher cycle
# with no active ip's, frame is done
frames = frame.callers_in_cycle
Expand All @@ -28,6 +27,7 @@ function typeinf(frame::InferenceState)
# empty!(frames)
min_valid = frame.min_valid
max_valid = frame.max_valid
cached = frame.cached
if cached || frame.parent !== nothing
for caller in results
opt = caller.src
Expand Down Expand Up @@ -442,13 +442,22 @@ function resolve_call_cycle!(linfo::MethodInstance, parent::InferenceState)
uncached |= !frame.cached # ensure we never add an uncached frame to a cycle
limited |= frame.limited
if frame.linfo === linfo
uncached && return true
if uncached
# our attempt to speculate into a constant call lead to an undesired self-cycle
# that cannot be converged: poison our call-stack (up to the discovered duplicate frame)
# with the limited flag and abort (set return type to Any) now
poison_callstack(parent, frame, false)
return true
end
merge_call_chain!(parent, frame, frame, limited)
return frame
end
for caller in frame.callers_in_cycle
if caller.linfo === linfo
uncached && return true
if uncached
poison_callstack(parent, frame, false)
return true
end
merge_call_chain!(parent, frame, caller, limited)
return caller
end
Expand Down

0 comments on commit 33549ba

Please sign in to comment.