Skip to content

Commit bd2a136

Browse files
committed
inference: always bail out const-prop' with non-const result limited
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
1 parent bf13a56 commit bd2a136

File tree

1 file changed

+20
-22
lines changed

1 file changed

+20
-22
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -814,12 +814,7 @@ end
814814
function abstract_call_method_with_const_args(interp::AbstractInterpreter,
815815
result::MethodCallResult, @nospecialize(f), arginfo::ArgInfo, si::StmtInfo,
816816
match::MethodMatch, sv::AbsIntState, invokecall::Union{Nothing,InvokeCall}=nothing)
817-
818-
if !const_prop_enabled(interp, sv, match)
819-
return nothing
820-
end
821-
if bail_out_const_call(interp, result, si)
822-
add_remark!(interp, sv, "[constprop] No more information to be gained")
817+
if !const_prop_enabled(interp, match, sv) || bail_out_const_call(interp, result, si, sv)
823818
return nothing
824819
end
825820
eligibility = concrete_eval_eligible(interp, f, result, arginfo, sv)
@@ -852,7 +847,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter,
852847
return const_prop_call(interp, mi, result, arginfo, sv, concrete_eval_result)
853848
end
854849

855-
function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match::MethodMatch)
850+
function const_prop_enabled(interp::AbstractInterpreter, match::MethodMatch, sv::AbsIntState)
856851
if !InferenceParams(interp).ipo_constant_propagation
857852
add_remark!(interp, sv, "[constprop] Disabled by parameter")
858853
return false
@@ -864,9 +859,11 @@ function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match:
864859
return true
865860
end
866861

867-
function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult, si::StmtInfo)
862+
function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult,
863+
si::StmtInfo, sv::AbsIntState)
868864
if is_removable_if_unused(result.effects)
869865
if isa(result.rt, Const) || call_result_unused(si)
866+
add_remark!(interp, sv, "[constprop] No more information to be gained")
870867
return true
871868
end
872869
elseif result.rt === Bottom
@@ -876,8 +873,14 @@ function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResu
876873
# precise enough to let us determine :consistency of `exct`, so we
877874
# would have to force constprop just to determine this, which is too
878875
# expensive.
876+
add_remark!(interp, sv, "[constprop] No more information to be gained")
879877
return true
880878
end
879+
elseif result.rt isa LimitedAccuracy
880+
# optimizations like inlining are disabled for limited frames,
881+
# thus there won't be much benefit in constant-prop' here
882+
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
883+
return true
881884
end
882885
return false
883886
end
@@ -974,7 +977,9 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter,
974977
match::MethodMatch, sv::AbsIntState)
975978
method = match.method
976979
force = force_const_prop(interp, f, method)
977-
force || const_prop_entry_heuristic(interp, result, si, sv) || return nothing
980+
if !force && !const_prop_entry_heuristic(interp, result, si, sv)
981+
return nothing
982+
end
978983
nargs::Int = method.nargs
979984
method.isva && (nargs -= 1)
980985
length(arginfo.argtypes) < nargs && return nothing
@@ -1021,21 +1026,14 @@ function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodC
10211026
elseif isa(rt, PartialStruct) || isa(rt, InterConditional) || isa(rt, InterMustAlias)
10221027
# could be improved to `Const` or a more precise wrapper
10231028
return true
1024-
elseif isa(rt, LimitedAccuracy)
1025-
# optimizations like inlining are disabled for limited frames,
1026-
# thus there won't be much benefit in constant-prop' here
1027-
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
1028-
return false
1029-
else
1030-
if isa(rt, Const)
1031-
if !is_nothrow(result.effects)
1032-
# Could still be improved to Bottom (or at least could see the effects improved)
1033-
return true
1034-
end
1029+
elseif isa(rt, Const)
1030+
if !is_nothrow(result.effects)
1031+
# Could still be improved to Bottom (or at least could see the effects improved)
1032+
return true
10351033
end
1036-
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
1037-
return false
10381034
end
1035+
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
1036+
return false
10391037
end
10401038

10411039
# determines heuristically whether if constant propagation can be worthwhile

0 commit comments

Comments
 (0)