From bc3226305d1a6ba4dc603ea5ff4f3c40b98f0b75 Mon Sep 17 00:00:00 2001
From: Shuhei Kadowaki <aviatesk@gmail.com>
Date: Tue, 4 Apr 2023 00:10:09 +0900
Subject: [PATCH] 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.
---
 base/compiler/inferencestate.jl |  6 +++++-
 test/compiler/effects.jl        | 18 +++++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/base/compiler/inferencestate.jl b/base/compiler/inferencestate.jl
index 0e0409f755a0b..8aff8910f457f 100644
--- a/base/compiler/inferencestate.jl
+++ b/base/compiler/inferencestate.jl
@@ -854,7 +854,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
 
diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl
index ee1d5dc569702..bd8465f1e2611 100644
--- a/test/compiler/effects.jl
+++ b/test/compiler/effects.jl
@@ -874,7 +874,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
@@ -982,3 +982,19 @@ isassigned_effects(s) = isassigned(Ref(s))
 @test fully_eliminated(; retval=true) do
     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