Skip to content

Commit ee45c04

Browse files
committed
inference: make throw block deoptimization concrete-eval friendly
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.
1 parent 1cf5091 commit ee45c04

2 files changed

Lines changed: 22 additions & 2 deletions

File tree

base/compiler/inferencestate.jl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,11 @@ function should_infer_this_call(interp::AbstractInterpreter, sv::InferenceState)
854854
end
855855
function should_infer_for_effects(sv::InferenceState)
856856
effects = sv.ipo_effects
857-
return is_terminates(effects) && is_effect_free(effects)
857+
effects.consistent === ALWAYS_FALSE && return false
858+
effects.effect_free === ALWAYS_FALSE && return false
859+
effects.terminates || return false
860+
effects.nonoverlayed || return false
861+
return true
858862
end
859863
should_infer_this_call(::AbstractInterpreter, ::IRInterpretationState) = true
860864

test/compiler/effects.jl

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ end |> Core.Compiler.is_foldable
874874
getfield(w, s)
875875
end |> Core.Compiler.is_foldable
876876

877-
# Flow-sensitive consistenct for _typevar
877+
# Flow-sensitive consistent for _typevar
878878
@test Base.infer_effects() do
879879
return WrapperOneField == (WrapperOneField{T} where T)
880880
end |> Core.Compiler.is_foldable_nothrow
@@ -982,3 +982,19 @@ isassigned_effects(s) = isassigned(Ref(s))
982982
@test fully_eliminated(; retval=true) do
983983
isassigned_effects(:foo)
984984
end
985+
986+
# inference on throw block should be disabled only when the effects are already known to be
987+
# concrete-eval ineligible:
988+
function optimize_throw_block_for_effects(x)
989+
a = [x]
990+
if x < 0
991+
throw(ArgumentError(lazy"negative number given: $x"))
992+
end
993+
return a
994+
end
995+
let effects = Base.infer_effects(optimize_throw_block_for_effects, (Int,))
996+
@test Core.Compiler.is_consistent_if_notreturned(effects)
997+
@test Core.Compiler.is_effect_free(effects)
998+
@test !Core.Compiler.is_nothrow(effects)
999+
@test Core.Compiler.is_terminates(effects)
1000+
end

0 commit comments

Comments
 (0)