Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inference: always bail out const-prop' with non-const result limited #52836

Merged
merged 1 commit into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 30 additions & 26 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -814,12 +814,7 @@ end
function abstract_call_method_with_const_args(interp::AbstractInterpreter,
result::MethodCallResult, @nospecialize(f), arginfo::ArgInfo, si::StmtInfo,
match::MethodMatch, sv::AbsIntState, invokecall::Union{Nothing,InvokeCall}=nothing)

if !const_prop_enabled(interp, sv, match)
return nothing
end
if bail_out_const_call(interp, result, si)
add_remark!(interp, sv, "[constprop] No more information to be gained")
if !const_prop_enabled(interp, match, sv) || bail_out_const_call(interp, result, si, sv)
return nothing
end
eligibility = concrete_eval_eligible(interp, f, result, arginfo, sv)
Expand Down Expand Up @@ -852,7 +847,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter,
return const_prop_call(interp, mi, result, arginfo, sv, concrete_eval_result)
end

function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match::MethodMatch)
function const_prop_enabled(interp::AbstractInterpreter, match::MethodMatch, sv::AbsIntState)
if !InferenceParams(interp).ipo_constant_propagation
add_remark!(interp, sv, "[constprop] Disabled by parameter")
return false
Expand All @@ -864,9 +859,11 @@ function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match:
return true
end

function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult, si::StmtInfo)
function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult,
si::StmtInfo, sv::AbsIntState)
if is_removable_if_unused(result.effects)
if isa(result.rt, Const) || call_result_unused(si)
add_remark!(interp, sv, "[constprop] No more information to be gained (const)")
return true
end
elseif result.rt === Bottom
Expand All @@ -876,6 +873,7 @@ function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResu
# precise enough to let us determine :consistency of `exct`, so we
# would have to force constprop just to determine this, which is too
# expensive.
add_remark!(interp, sv, "[constprop] No more information to be gained (bottom)")
return true
end
end
Expand Down Expand Up @@ -974,7 +972,10 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter,
match::MethodMatch, sv::AbsIntState)
method = match.method
force = force_const_prop(interp, f, method)
force || const_prop_entry_heuristic(interp, result, si, sv) || return nothing
if !const_prop_entry_heuristic(interp, result, si, sv, force)
# N.B. remarks are emitted within `const_prop_entry_heuristic`
return nothing
end
nargs::Int = method.nargs
method.isva && (nargs -= 1)
length(arginfo.argtypes) < nargs && return nothing
Expand All @@ -1001,8 +1002,17 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter,
return mi
end

function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodCallResult, si::StmtInfo, sv::AbsIntState)
if call_result_unused(si) && result.edgecycle
function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodCallResult,
si::StmtInfo, sv::AbsIntState, force::Bool)
if result.rt isa LimitedAccuracy
# optimizations like inlining are disabled for limited frames,
# thus there won't be much benefit in constant-prop' here
# N.B. don't allow forced constprop' for safety (xref #52763)
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
return false
elseif force
return true
elseif call_result_unused(si) && result.edgecycle
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (edgecycle with unused result)")
return false
end
Expand All @@ -1015,27 +1025,21 @@ function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodC
if rt === Bottom
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (erroneous result)")
return false
else
return true
end
return true
elseif isa(rt, PartialStruct) || isa(rt, InterConditional) || isa(rt, InterMustAlias)
# could be improved to `Const` or a more precise wrapper
return true
elseif isa(rt, LimitedAccuracy)
# optimizations like inlining are disabled for limited frames,
# thus there won't be much benefit in constant-prop' here
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
return false
else
if isa(rt, Const)
if !is_nothrow(result.effects)
# Could still be improved to Bottom (or at least could see the effects improved)
return true
end
elseif isa(rt, Const)
if is_nothrow(result.effects)
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (nothrow const)")
return false
end
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
return false
# Could still be improved to Bottom (or at least could see the effects improved)
return true
end
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
return false
end

# determines heuristically whether if constant propagation can be worthwhile
Expand Down
13 changes: 12 additions & 1 deletion test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4937,10 +4937,21 @@ g() = empty_nt_values(Base.inferencebarrier(Tuple{}))
# This is somewhat sensitive to the exact recursion level that inference is willing to do, but the intention
# is to test the case where inference limited a recursion, but then a forced constprop nevertheless managed
# to terminate the call.
@newinterp RecurseInterpreter
let CC = Core.Compiler
function CC.const_prop_entry_heuristic(interp::RecurseInterpreter, result::CC.MethodCallResult,
si::CC.StmtInfo, sv::CC.AbsIntState, force::Bool)
if result.rt isa CC.LimitedAccuracy
return force # allow forced constprop to recurse into unresolved cycles
end
return @invoke CC.const_prop_entry_heuristic(interp::CC.AbstractInterpreter, result::CC.MethodCallResult,
si::CC.StmtInfo, sv::CC.AbsIntState, force::Bool)
end
end
Base.@constprop :aggressive type_level_recurse1(x...) = x[1] == 2 ? 1 : (length(x) > 100 ? x : type_level_recurse2(x[1] + 1, x..., x...))
Base.@constprop :aggressive type_level_recurse2(x...) = type_level_recurse1(x...)
type_level_recurse_entry() = Val{type_level_recurse1(1)}()
@test Base.return_types(type_level_recurse_entry, ()) |> only == Val{1}
@test Base.infer_return_type(type_level_recurse_entry, (); interp=RecurseInterpreter()) == Val{1}

# Test that inference doesn't give up if it can potentially refine effects,
# even if the return type is Any.
Expand Down