Skip to content

Commit

Permalink
inference: make throw block deoptimization concrete-eval friendly
Browse files Browse the repository at this point in the history
The deoptimization can sometimes destroy the effects analysis and
disable [semi-]concrete evaluation that is otherwise possible.
This is because the deoptimization was designed with the type domain
profitability in mind (#35982), and it has not been aware of the effects
domain very well.

This commit makes the deoptimization aware of the effects domain more
and enables the `throw` block deoptimization only when the effects
already known to be ineligible for concrete-evaluation.

In our current effect system, `ALWAYS_FALSE`/`false` means that the
effect can not be refined to `ALWAYS_TRUE`/`true` anymore (unless given
user annotation later). Therefore we can enable the `throw` block
deoptimization without hindering the chance of concrete-evaluation when
any of the following conditions are met:
- `effects.consistent === ALWAYS_FALSE`
- `effects.effect_free === ALWAYS_FALSE`
- `effects.terminates`
- `effects.nonoverlayed`
```

Here are some numbers:

| Metric                  | master    | this commit | #35982 reverted (set `unoptimize_throw_blocks=false`) |
|-------------------------|-----------|-------------|--------------------------------------------|
| Base (seconds)          | 15.579300 | 15.206645   | 15.296319                                  |
| Stdlibs (seconds)       | 17.919013 | 17.667094   | 17.738128                                  |
| Total (seconds)         | 33.499279 | 32.874737   | 33.035448                                  |
| Precompilation (seconds) | 49.967516 | 49.421121  | 49.999998                                  |
| First time `plot(rand(10,3))` [^1] | `2.476678 seconds (11.74 M allocations)` | `2.430355 seconds (11.77 M allocations)` | `2.514874 seconds (11.64 M allocations)` |

[^1]: I got these numbers with disabling all the `@precompile_all_calls` statements in Plots.jl.

These numbers made me question if we are getting any actual benefit from
the `throw` block deoptimization anymore. Since it is sometimes harmful
for the effects analysis, we probably want to either merge this commit
or remove the `throw` block deoptimization completely.
  • Loading branch information
aviatesk committed Sep 20, 2023
1 parent 4923e95 commit 81287f2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
6 changes: 5 additions & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,11 @@ function should_infer_this_call(interp::AbstractInterpreter, sv::InferenceState)
end
function should_infer_for_effects(sv::InferenceState)
effects = sv.ipo_effects
return is_terminates(effects) && is_effect_free(effects)
effects.consistent === ALWAYS_FALSE && return false
effects.effect_free === ALWAYS_FALSE && return false
effects.terminates || return false
effects.nonoverlayed || return false
return true
end
should_infer_this_call(::AbstractInterpreter, ::IRInterpretationState) = true

Expand Down
18 changes: 17 additions & 1 deletion test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ end |> Core.Compiler.is_foldable
getfield(w, s)
end |> Core.Compiler.is_foldable

# Flow-sensitive consistenct for _typevar
# Flow-sensitive consistent for _typevar
@test Base.infer_effects() do
return WrapperOneField == (WrapperOneField{T} where T)
end |> Core.Compiler.is_foldable_nothrow
Expand Down Expand Up @@ -1001,6 +1001,22 @@ isassigned_effects(s) = isassigned(Ref(s))
isassigned_effects(:foo)
end

# inference on throw block should be disabled only when the effects are already known to be
# concrete-eval ineligible:
function optimize_throw_block_for_effects(x)
a = [x]
if x < 0
throw(ArgumentError(lazy"negative number given: $x"))
end
return a
end
let effects = Base.infer_effects(optimize_throw_block_for_effects, (Int,))
@test Core.Compiler.is_consistent_if_notreturned(effects)
@test Core.Compiler.is_effect_free(effects)
@test !Core.Compiler.is_nothrow(effects)
@test Core.Compiler.is_terminates(effects)
end

# :isdefined effects
@test @eval Base.infer_effects() do
@isdefined($(gensym("some_undef_symbol")))
Expand Down

0 comments on commit 81287f2

Please sign in to comment.