inference: Model type propagation through exceptions #51754
Conversation
Keno
force-pushed
the
kf/errortypeprop
branch
from
0397be1
to
cde05ac
Compare
| if !⊑(𝕃ₚ, exct, frame.exc_bestguess) | ||
| frame.exc_bestguess = tmerge(𝕃ₚ, frame.exc_bestguess, exct) | ||
| for (caller, caller_pc) in frame.cycle_backedges | ||
| push!(caller.ip, block_for_inst(caller.cfg, caller_pc)) |
There was a problem hiding this comment.
Similar to return type, we could avoid revisiting this whole basic block if the caller frame or catch stack is already known to be Any
brenhinkeller
added
performance
aviatesk
force-pushed
the
kf/errortypeprop
branch
from
cde05ac
to
1a1735b
Compare
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
Keno
force-pushed
the
kf/errortypeprop
branch
from
1a1735b
to
a0f5598
Compare
|
I fixed a number of the errors here. The remainder needs to have #51970 addressed first. |
Keno
force-pushed
the
kf/errortypeprop
branch
4 times, most recently
from
e21e77d
to
20dde5e
Compare
|
@nanosoldier |
There was a problem hiding this comment.
@nanosoldier runbenchmarks("inference", vs=":master")
Overall LGTM.
|
The package evaluation job you requested has completed - possible new issues were detected. |
Keno
force-pushed
the
kf/errortypeprop
branch
from
20dde5e
to
1704053
Compare
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. |
|
I've looked into the stack overflow in MIRTio. I don't think that's really related to these changed, it's just trying to infer the massive signature: which has a lot of (bounded) recursion, but the only thing that this PR changes in this regard is that some of the inference stack frames get marginally larger because of the couple extra locals, so it doesn't quite make it through the end. It would probably be good to terminate the recursion for these with a loop, as we do for other things that take a massive unbounded amount of varargs, but I'm gonna call that outside the scope of this PR. |
|
So from my perspective this is basically good to go. I want to do a few more perf improvements, because I did notice some regressions on this branch, but unfortunately, the biggest regressions are just due to the bug fixes in the optimizer for issues exposed (but not root caused by this PR), so I'm not sure how much can be done about that. |
|
@nanosoldier |
base/compiler/inferencestate.jl
Outdated
| @@ -203,6 +203,12 @@ const CACHE_MODE_GLOBAL = 0x01 << 0 # cached globally, optimization allowed | |||
| const CACHE_MODE_LOCAL = 0x01 << 1 # cached locally, optimization allowed | |||
| const CACHE_MODE_VOLATILE = 0x01 << 2 # not cached, optimization allowed | |||
|
|
|||
| mutable struct TryCatchFrame | |||
| exct | |||
| const enter_idx | |||
There was a problem hiding this comment.
| const enter_idx | |
| const enter_idx::Int |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
| this_argtypes = isa(matches, MethodMatches) ? argtypes : matches.applicable_argtypes[i] | ||
| this_arginfo = ArgInfo(fargs, this_argtypes) | ||
| const_call_result = abstract_call_method_with_const_args(interp, | ||
| result, f, this_arginfo, si, match, sv) | ||
| const_result = volatile_inf_result | ||
| if const_call_result !== nothing | ||
| if const_call_result.rt ⊑ₚ rt | ||
| if !(rt ⊑ₚ const_call_result.rt) |
There was a problem hiding this comment.
This change and the related one in the other branch that was done in a previous PR is likely ill advised, because it results in us no longer using the const-propped version of the IR for inlining.
|
@nanosoldier |
|
I think this should be good to go at this point. |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
Currently the type of a caught exception is always modeled as `Any`.
This isn't a huge problem, because control flow in Julia is generally
assumed to be somewhat slow, so the extra type imprecision of not
knowing the return type does not matter all that much. However,
there are a few situations where it matters. For example:
```
maybe_getindex(A, i) =
try; A[i]; catch e; isa(e, BoundsError) && return nothing; rethrow(); end
```
At present, we cannot infer :nothrow for this method, even if that
is the only error type that `A[i]` can throw. This is particularly
noticable, since we can now optimize away `:nothrow` exception frames
entirely (#51674). Note that this PR still does not make the above
example particularly efficient (at least interprocedurally), though
specialized codegen could be added on top of this to make that happen.
It does however improve the inference result.
A second major motivation of this change is that reasoning about
exception types is likely to be a major aspect of any future work
on interface checking (since interfaces imply the absence of
MethodErrors), so this PR lays the groundwork for appropriate modeling
of these error paths.
Note that this PR adds all the required plumbing, but does not yet have
a particularly precise model of error types for our builtins, bailing
to `Any` for any builtin not known to be `:nothrow`. This can be improved
in follow up PRs as required.
adce_pass! runs its own compaction, so we only need to run another run of compaction if it made any post-compaction changes.
This PR included a change that only used constant results if the result was *better*. However, this is not what we want, because even though the result may have been the same, the IR we computed will likely still have more precise (and fewer) statements, because we optimized a bunch of things away based on constant information.
Currently, we cannot assume that the error type thrown from a concrete eval will be `:consistent`, so constprop often returns more precise rt information, but less precise exct information. To deal with the consider them separately, and only override exct if the constprop information was actually better.
Keno
force-pushed
the
kf/errortypeprop
branch
from
fdfc6e4
to
f33bef8
Compare
Composed of: - typeinf_local: factor into `update_cycle_worklists!` utility (78f7b4e) - inference: fix exception type of `typename` call (fac36d8) - add missing type annotations (7ce140e) - inference: refine exct information if `:nothrow` is proven (76143d3) - ~~improve exception type inference for core math functions (525bd6c)~~ Separated into another PR
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Integration for JuliaLang/julia#51754. Statement-wise and call-wise information is available only after `v"1.11.0-DEV.1127"`.
Currently the type of a caught exception is always modeled as
Any. This isn't a huge problem, because control flow in Julia is generally assumed to be somewhat slow, so the extra type imprecision of not knowing the return type does not matter all that much. However, there are a few situations where it matters. For example:At present, we cannot infer :nothrow for this method, even if that is the only error type that
A[i]can throw. This is particularly noticable, since we can now optimize away:nothrowexception frames entirely (#51674). Note that this PR still does not make the above example particularly efficient (at least interprocedurally), though specialized codegen could be added on top of this to make that happen. It does however improve the inference result.A second major motivation of this change is that reasoning about exception types is likely to be a major aspect of any future work on interface checking (since interfaces imply the absence of MethodErrors), so this PR lays the groundwork for appropriate modeling of these error paths.
Note that this PR adds all the required plumbing, but does not yet have a particularly precise model of error types for our builtins, bailing to
Anyfor any builtin not known to be:nothrow. This can be improved in follow up PRs as required.