Skip to content

Commit

Permalink
inference: prioritize force_constant_prop over `const_prop_entry_he…
Browse files Browse the repository at this point in the history
…uristic` (#41882)

Currently our constant-prop' heuristics work in the following way:
1. `const_prop_entry_heuristic`
2. `const_prop_argument_heuristic` & `const_prop_rettype_heuristic`
3. `force_const_prop` custom heuristic & `!const_prop_function_heuristic`
4. `MethodInstance` specialization and `const_prop_methodinstance_heuristic`

This PR changes it so that the step 1. now works like:

1. `force_const_prop` custom heuristic & `const_prop_entry_heuristic`

and the steps 2., 3. and 4. don't change

This change particularly allows us to more forcibly constant-propagate
for `getproperty` and `setproperty!`, and inline them more, e.g.:
```julia
# if we don't force constant-prop', `T = fieldtype(Foo, ::Symbol)` will be union-split to
# `Union{Type{Any},Type{Int}` and it will make `convert(T, nothing)` too costly
# and it leads to inlining failure
mutable struct Foo
    val
    _::Int
end

function setter(xs)
    for x in xs
        x.val = nothing # `setproperty!` can be inlined with this PR
    end
end
```

It might be useful because now we can intervene into the constant-prop'
heuristic in a more reliable way with the `aggressive_constprop` interface.

I did the simple benchmark below, and it looks like this change doesn't
cause the latency problem for this particular example:
```zsh
~/julia master aviatesk@amdci2 6s
❯ ./usr/bin/julia -e '@time using Plots; @time plot(rand(10,3))'
  3.708500 seconds (7.28 M allocations: 506.128 MiB, 3.45% gc time, 1.13% compilation time)
  2.817794 seconds (3.45 M allocations: 195.127 MiB, 7.84% gc time, 53.76% compilation time)

~/julia avi/forceconstantprop aviatesk@amdci2 6s
❯ ./usr/bin/julia -e '@time using Plots; @time plot(rand(10,3))'
  3.622109 seconds (7.02 M allocations: 481.710 MiB, 4.19% gc time, 1.17% compilation time)
  2.863419 seconds (3.44 M allocations: 194.210 MiB, 8.02% gc time, 53.53% compilation time)
```
  • Loading branch information
aviatesk committed Aug 24, 2021
1 parent 292f1a9 commit cdd2f30
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
19 changes: 13 additions & 6 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -567,16 +567,21 @@ end
function maybe_get_const_prop_profitable(interp::AbstractInterpreter, result::MethodCallResult,
@nospecialize(f), argtypes::Vector{Any}, match::MethodMatch,
sv::InferenceState)
const_prop_entry_heuristic(interp, result, sv) || return nothing
if !InferenceParams(interp).ipo_constant_propagation
add_remark!(interp, sv, "[constprop] Disabled by parameter")
return nothing
end
method = match.method
force = force_const_prop(interp, f, method)
force || const_prop_entry_heuristic(interp, result, sv) || return nothing
nargs::Int = method.nargs
method.isva && (nargs -= 1)
if length(argtypes) < nargs
length(argtypes) < nargs && return nothing
if !(const_prop_argument_heuristic(interp, argtypes) || const_prop_rettype_heuristic(interp, result.rt))
add_remark!(interp, sv, "[constprop] Disabled by argument and rettype heuristics")
return nothing
end
const_prop_argument_heuristic(interp, argtypes) || const_prop_rettype_heuristic(interp, result.rt) || return nothing
allconst = is_allconst(argtypes)
force = force_const_prop(interp, f, method)
if !force
if !const_prop_function_heuristic(interp, f, argtypes, nargs, allconst)
add_remark!(interp, sv, "[constprop] Disabled by function heuristic")
Expand All @@ -599,10 +604,12 @@ end

function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodCallResult, sv::InferenceState)
if call_result_unused(sv) && result.edgecycle
add_remark!(interp, sv, "[constprop] Edgecycle with unused result")
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (edgecycle with unused result)")
return false
end
return is_improvable(result.rt) && InferenceParams(interp).ipo_constant_propagation
is_improvable(result.rt) && return true
add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable return type)")
return false
end

# see if propagating constants may be worthwhile
Expand Down
23 changes: 23 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,29 @@ end
end
end

# force constant-prop' for `setproperty!`
let m = Module()
ci = @eval m begin
# if we don't force constant-prop', `T = fieldtype(Foo, ::Symbol)` will be union-split to
# `Union{Type{Any},Type{Int}` and it will make `convert(T, nothing)` too costly
# and it leads to inlining failure
mutable struct Foo
val
_::Int
end

function setter(xs)
for x in xs
x.val = nothing
end
end

$code_typed1(setter, (Vector{Foo},))
end

@test !any(x->isinvoke(x, :setproperty!), ci.code)
end

# Issue #41299 - inlining deletes error check in :>
g41299(f::Tf, args::Vararg{Any,N}) where {Tf,N} = f(args...)
@test_throws TypeError g41299(>:, 1, 2)

0 comments on commit cdd2f30

Please sign in to comment.