Waitall!: Work around const-prop failure in setproperty!#492
Waitall!: Work around const-prop failure in setproperty!#492
Conversation
|
This is because the return type is already accurate enough and so there is no need for constant-prop' in terms of type-inference: especially, it hits this line with I wonder we actually don't need constant-prop' here, since inlinear will inline this |
|
The issue is that this is actually poisoning codegen. So the produced LLVM IR is suboptimal. |
|
Yes, it looks like this actually prevents inlining: This seems better to be fixed, I will look into the solutions. For now, manual inlining as you did looks reasonable. |
|
It turns out the inlining failure comes from the fact that setproperty!(x, f::Symbol, v) in Base at Base.jl:43
│ ─ %-1 = invoke setproperty!(::MPI.Request,::Symbol,::Nothing)::…
43 1 ─ 0 %1 = Base.fieldtype(MPI.Request, _3)::Union{Type{Any}, Type{Int32}}
│ 1 %2 = (isa)(%1, Type{Any})::Bool │
└── 0 goto #3 if not %2 │
2 ─ 0 goto #4 │
3 ─ 1000 %5 = Base.convert(%1, _4)::Core.Const(nothing) │
└── 0 goto #4 │
4 ┄ 0 %7 = φ (#2 => nothing, #3 => %5)::Core.Const(nothing) │
│ 3 %8 = Base.setfield!(_2, _3, %7)::Core.Const(nothing) │
└── 0 return %8 |
…uristic`
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. `!const_prop_function_heuristic` with `force_const_prop`'s custom heuristic
4. `MethodInstance` specialization and `const_prop_methodinstance_heuristic`
This PR changes it so that it now works:
1. `force_const_prop` custom heuristic & `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 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
```
(motivated by @vchuravy's example at <JuliaParallel/MPI.jl#492>)
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
❯ ./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)
❯ ./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)
```
|
Should this be closed then? |
|
This inlining failure is fixed on the very latest master. I think it's still reasonable to have this change if you work on v1.7 or lower. |
|
I only needed this to get Enzyme to differentiate through some MPI.jl code. I don't think it matters for other use-cases. |
@aviatesk any ideas why
setproperty!failed to const-prop?On 1.7 I am seeing: