Skip to content

Commit

Permalink
inference: improve constant-prop' heuristics
Browse files Browse the repository at this point in the history
- remove `const_prop_rettype_heuristic` since it handles rare cases,
  where const-prop' doens't seem to be worthwhile, e.g. it won't be
  so useful to try to propagate `Const(Tuple{DataType,DataType})` for
  `Const(convert)(::Const(Tuple{DataType,DataType}), ::Tuple{DataType,DataType} -> Tuple{DataType,DataType}`
- rename `is_allconst` to `is_all_overridden`
- also minor refactors and improvements added
  • Loading branch information
aviatesk committed Oct 12, 2021
1 parent edd2866 commit 4d598c7
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 37 deletions.
44 changes: 19 additions & 25 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -583,18 +583,16 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter, result::Me
nargs::Int = method.nargs
method.isva && (nargs -= 1)
length(arginfo.argtypes) < nargs && return nothing
if !(const_prop_argument_heuristic(interp, arginfo) || const_prop_rettype_heuristic(interp, result.rt))
if !const_prop_argument_heuristic(interp, arginfo, sv)
add_remark!(interp, sv, "[constprop] Disabled by argument and rettype heuristics")
return nothing
end
allconst = is_allconst(arginfo)
if !force
if !const_prop_function_heuristic(interp, f, arginfo, nargs, allconst)
add_remark!(interp, sv, "[constprop] Disabled by function heuristic")
return nothing
end
all_overridden = is_all_overridden(arginfo)
if !force && !const_prop_function_heuristic(interp, f, arginfo, nargs, all_overridden, sv)
add_remark!(interp, sv, "[constprop] Disabled by function heuristic")
return nothing
end
force |= allconst
force |= all_overridden
mi = specialize_method(match; preexisting=!force)
if mi === nothing
add_remark!(interp, sv, "[constprop] Failed to specialize")
Expand All @@ -618,11 +616,15 @@ function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodC
return false
end

# see if propagating constants may be worthwhile
function const_prop_argument_heuristic(interp::AbstractInterpreter, (; fargs, argtypes)::ArgInfo)
for a in argtypes
# determines heuristically whether if constant propagation can be worthwhile
# by checking if any of given `argtypes` is "interesting" enough to be propagated
function const_prop_argument_heuristic(_::AbstractInterpreter, (; fargs, argtypes)::ArgInfo, _::InferenceState)
for i in 1:length(argtypes)
a = argtypes[i]
if isa(a, Conditional) && fargs !== nothing
return is_const_prop_profitable_conditional(a, fargs)
if is_const_prop_profitable_conditional(a, fargs)
return true
end
end
a = widenconditional(a)
if has_nontrivial_const_info(a) && is_const_prop_profitable_arg(a)
Expand Down Expand Up @@ -662,22 +664,14 @@ function find_constrained_arg(cnd::Conditional, fargs::Vector{Any})
end
end

function const_prop_rettype_heuristic(interp::AbstractInterpreter, @nospecialize(rettype))
return improvable_via_constant_propagation(rettype)
end

function is_allconst((; fargs, argtypes)::ArgInfo)
function is_all_overridden((; fargs, argtypes)::ArgInfo)
for a in argtypes
if isa(a, Conditional) && fargs !== nothing
if is_const_prop_profitable_conditional(a, fargs)
continue
end
end
a = widenconditional(a)
# TODO unify these condition with `has_nontrivial_const_info`
if !isa(a, Const) && !isconstType(a) && !isa(a, PartialStruct) && !isa(a, PartialOpaque)
return false
end
is_forwardable_argtype(widenconditional(a)) || return false
end
return true
end
Expand All @@ -691,7 +685,7 @@ end

function const_prop_function_heuristic(
interp::AbstractInterpreter, @nospecialize(f), (; argtypes)::ArgInfo,
nargs::Int, allconst::Bool)
nargs::Int, all_overridden::Bool, _::InferenceState)
if nargs > 1
if istopfunction(f, :getindex) || istopfunction(f, :setindex!)
arrty = argtypes[2]
Expand All @@ -708,7 +702,7 @@ function const_prop_function_heuristic(
end
end
end
if !allconst && (istopfunction(f, :+) || istopfunction(f, :-) || istopfunction(f, :*) ||
if !all_overridden && (istopfunction(f, :+) || istopfunction(f, :-) || istopfunction(f, :*) ||
istopfunction(f, :(==)) || istopfunction(f, :!=) ||
istopfunction(f, :<=) || istopfunction(f, :>=) || istopfunction(f, :<) || istopfunction(f, :>) ||
istopfunction(f, :<<) || istopfunction(f, :>>))
Expand Down Expand Up @@ -1250,7 +1244,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
fargs′ = fargs[4:end]
pushfirst!(fargs′, fargs[1])
arginfo = ArgInfo(fargs′, argtypes′)
const_prop_argument_heuristic(interp, arginfo) || const_prop_rettype_heuristic(interp, rt) || return CallMeta(rt, InvokeCallInfo(match, nothing))
const_prop_argument_heuristic(interp, arginfo, sv) || return CallMeta(rt, InvokeCallInfo(match, nothing))
# # typeintersect might have narrowed signature, but the accuracy gain doesn't seem worth the cost involved with the lattice comparisons
# for i in 1:length(argtypes′)
# t, a = ti.parameters[i], argtypes′[i]
Expand Down
12 changes: 9 additions & 3 deletions base/compiler/inferenceresult.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@
function is_argtype_match(@nospecialize(given_argtype),
@nospecialize(cache_argtype),
overridden_by_const::Bool)
if isa(given_argtype, Const) || isa(given_argtype, PartialStruct) ||
isa(given_argtype, PartialOpaque) || isa(given_argtype, Conditional)
if is_forwardable_argtype(given_argtype)
return is_lattice_equal(given_argtype, cache_argtype)
end
return !overridden_by_const
end

function is_forwardable_argtype(@nospecialize x)
return isa(x, Const) ||
isa(x, Conditional) ||
isa(x, PartialStruct) ||
isa(x, PartialOpaque)
end

# In theory, there could be a `cache` containing a matching `InferenceResult`
# for the provided `linfo` and `given_argtypes`. The purpose of this function is
# to return a valid value for `cache_lookup(linfo, argtypes, cache).argtypes`,
Expand Down Expand Up @@ -65,7 +71,7 @@ function matching_cache_argtypes(
for i in 1:nargs
given_argtype = given_argtypes[i]
cache_argtype = cache_argtypes[i]
if !is_argtype_match(given_argtype, cache_argtype, overridden_by_const[i])
if !is_argtype_match(given_argtype, cache_argtype, false)
# prefer the argtype we were given over the one computed from `linfo`
cache_argtypes[i] = given_argtype
overridden_by_const[i] = true
Expand Down
9 changes: 0 additions & 9 deletions base/compiler/typeutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,6 @@ unioncomplexity(u::UnionAll) = max(unioncomplexity(u.body)::Int, unioncomplexity
unioncomplexity(t::Core.TypeofVararg) = isdefined(t, :T) ? unioncomplexity(t.T)::Int : 0
unioncomplexity(@nospecialize(x)) = 0

function improvable_via_constant_propagation(@nospecialize(t))
if isconcretetype(t) && t <: Tuple
for p in t.parameters
p === DataType && return true
end
end
return false
end

# convert a Union of Tuple types to a Tuple of Unions
function unswitchtupleunion(u::Union)
ts = uniontypes(u)
Expand Down

0 comments on commit 4d598c7

Please sign in to comment.