inference: refactoring to allow irinterp to perform `:call` inference #48913

aviatesk · 2023-03-06T15:04:30Z

This commit implements a significant refactoring of the inference routines,
which is necessary to enable :call inference in irinterp. While this
commit does not yet enable :call inference, a subsequent small commit
will do so. This is because external AbstractInterpreters first need
to adjust their code for this refactoring, and in the event of a
regression detected by the recursive :call inference, we will be able
to simply revert the small commit.

Additionally, this commit improves the robustness of irinterp by
allowing it to handle invoke calls, which currently result in a crash.

TODOs:

implement a simple recursion detection mechanism for IRInterpretationState
add proper invalidation support
allow constant inference from semi-concrete interpretation
propagate callinfo and allow double inlining

- remove `update_valid_age!(edge::InferenceState, sv::InferenceState)` and replace all the usages with `update_valid_age!(sv, edge.valid_worlds)`: this will simplify the incoming `AbsIntState` interface (see #48913) - remove `Effects(sv::InferenceState)` utility: replace all the usages with `sv.ipo_effects`, which is more explictly saying that we are looking at IPO-valid effects - normalize more `li::MethodInstance` to `mi::MethodInstance` - import `Core.MethodTable` - fix up `setindex!` return values

base/compiler/ssair/irinterp.jl

aviatesk · 2023-03-14T16:18:50Z

I am still working on item 1, which is to implement a proper recursion detection support for irinterp. I am currently wondering if a simple recursion detection mechanism is sufficient, as the existing recursion detection mechanism with InferenceState may be overkill for irinterp, given that we only perform irinterp on methods that are proven to be :terminates, indicating that there is no heavy recursion (unless specified by user annotation). Additionally, I am also considering whether we should implement the LimitedAccuracy-related system for irinterp. Any thoughts on this?

Keno · 2023-03-14T19:41:56Z

I'm not sure. :terminates is a pretty strong condition, so in theory it should probably be ok, but I'm not sure I'm comfortable saying that it can't happen. However, if it does happen, we probably have the same issue right now without this change, so if you want to finish this up and keep working on the termination change separately I think that would be ok as long as nothing shows up on pkgeval. I do think it would be good to eventually relax the :terminates requirement, for which strong recursion detection would be required, but that can be separate.

aviatesk · 2023-03-16T15:24:14Z

Okay, now I think this PR is ready. I also tweaked miscellaneous inference subroutines like abstract_invoke so that they are reused during irinterp. Now irinterp should be more robust.

@nanosoldier runtests(ALL, vs = ":master")

aviatesk · 2023-03-16T15:25:08Z

@nanosoldier runbenchmarks("inference", vs=":master")

base/compiler/inferencestate.jl

nanosoldier · 2023-03-30T10:18:33Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

Keno

Thanks for splitting this up.

This commit implements a significant refactoring of the inference routines, which is necessary to enable `:call` inference in irinterp. While this commit does not yet enable `:call` inference, a subsequent small commit will do so. This is because external `AbstractInterpreter`s first need to adjust their code for this refactoring, and in the event of a regression detected by the recursive `:call` inference, we will be able to simply revert the small commit. Additionally, this commit improves the robustness of irinterp by allowing it to handle invoke calls, which currently result in a crash. TODOs: - [x] implement a simple recursion detection mechanism for `IRInterpretationState` - [x] add proper invalidation support - [x] allow constant inference from semi-concrete interpretation - [x] propagate callinfo and allow double inlining

Built on top of #48913, this commit enables `:call` inference in irinterp. In a case when some regression is detected, we can simply revert this commit rather than reverting the whole refactoring from #48913.

aviatesk · 2023-04-03T15:16:32Z

@nanosoldier runtests()

staticfloat · 2023-04-04T00:09:41Z

Nanosoldier is down, but we don't expect this to change performance that much (yet); it's the follow-on PR that will have significant impact.

Built on top of #48913, this commit enables `:call` inference in irinterp. In a case when some regression is detected, we can simply revert this commit rather than reverting the whole refactoring from #48913.

xref: JuliaLang/julia#48913

vtjnash · 2023-04-05T20:08:35Z

Nanosoldier is not affected by the PkgEval outage

xref: JuliaLang/julia#48913

Built on top of #48913, this commit enables `:call` inference in irinterp. In a case when some regression is detected, we can simply revert this commit rather than reverting the whole refactoring from #48913.

- remove `update_valid_age!(edge::InferenceState, sv::InferenceState)` and replace all the usages with `update_valid_age!(sv, edge.valid_worlds)`: this will simplify the incoming `AbsIntState` interface (see JuliaLang#48913) - remove `Effects(sv::InferenceState)` utility: replace all the usages with `sv.ipo_effects`, which is more explictly saying that we are looking at IPO-valid effects - normalize more `li::MethodInstance` to `mi::MethodInstance` - import `Core.MethodTable` - fix up `setindex!` return values

Built on top of #48913, this commit enables `:call` inference in irinterp. In a case when some regression is detected, we can simply revert this commit rather than reverting the whole refactoring from #48913.

* inference: enable `:call` inference in irinterp Built on top of #48913, this commit enables `:call` inference in irinterp. In a case when some regression is detected, we can simply revert this commit rather than reverting the whole refactoring from #48913. * fix irinterp lattice Now `LimitedAccuracy` can appear in irinterp, so we should include `InferenceLattice` for `[typeinf|ipo]_lattice` for irinterp.

If the LimitedAccuracy was supposed to resolve against the top-most frame (or hypothetically a non-InferenceState frame), it would not have a parentframe, preventing it from reaching the subsequent poison_callstack line that is required for reliable inference (avoiding caching bad results). This should restore the original intent of this code (pre #48913)

If the LimitedAccuracy was supposed to resolve against the top-most frame (or hypothetically a non-InferenceState frame), it would not have a parentframe, preventing it from reaching the subsequent poison_callstack line that is required for reliable inference (avoiding caching bad results). This should restore the original intent of this code (pre #48913) (cherry picked from commit d1b1a5d)

If the LimitedAccuracy was supposed to resolve against the top-most frame (or hypothetically a non-InferenceState frame), it would not have a parentframe, preventing it from reaching the subsequent poison_callstack line that is required for reliable inference (avoiding caching bad results). This should restore the original intent of this code (pre JuliaLang#48913)

aviatesk added compiler:inference Type inference status:DO NOT MERGE Do not merge this PR! labels Mar 6, 2023

aviatesk changed the title ~~very wip: inference: allow semi-concrete interpret to perform recursi…~~ very wip: inference: allow semi-concrete interpret to perform recursive inference Mar 6, 2023

aviatesk force-pushed the avi/irsv branch from db75e9a to 9a7aecf Compare March 7, 2023 09:49

aviatesk mentioned this pull request Mar 7, 2023

tidy up compiler implementation #48930

Merged

aviatesk force-pushed the avi/irsv branch 2 times, most recently from 56bea36 to 1010057 Compare March 7, 2023 17:36

aviatesk force-pushed the avi/irsv branch 4 times, most recently from a4d3be7 to 513c5e6 Compare March 8, 2023 10:56

aviatesk changed the title ~~very wip: inference: allow semi-concrete interpret to perform recursive inference~~ wip: inference: allow semi-concrete interpret to perform recursive inference Mar 8, 2023

aviatesk force-pushed the avi/irsv branch 2 times, most recently from c01f6cc to 4f0d57f Compare March 8, 2023 16:10

Keno reviewed Mar 10, 2023

View reviewed changes

base/compiler/ssair/irinterp.jl Outdated Show resolved Hide resolved

aviatesk force-pushed the avi/irsv branch 3 times, most recently from 965c082 to 53809c6 Compare March 12, 2023 09:40

aviatesk force-pushed the avi/irsv branch from 53809c6 to 45a6049 Compare March 16, 2023 14:34

aviatesk changed the title ~~wip: inference: allow semi-concrete interpret to perform recursive inference~~ inference: allow semi-concrete interpret to perform recursive inference Mar 16, 2023

aviatesk force-pushed the avi/irsv branch from 45a6049 to b780071 Compare March 16, 2023 15:21

aviatesk removed the status:DO NOT MERGE Do not merge this PR! label Mar 16, 2023

aviatesk commented Mar 16, 2023

View reviewed changes

base/compiler/inferencestate.jl Outdated Show resolved Hide resolved

aviatesk force-pushed the avi/irsv branch from b780071 to 53799df Compare March 16, 2023 16:10

aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Mar 30, 2023

adjust to JuliaLang/julia#48913

ba1a5ef

Keno approved these changes Mar 30, 2023

View reviewed changes

aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Mar 31, 2023

adjust to JuliaLang/julia#48913

be25bbd

aviatesk force-pushed the avi/irsv branch from 4674928 to 10e2001 Compare April 3, 2023 06:12

staticfloat merged commit 2ec27e9 into master Apr 4, 2023

staticfloat deleted the avi/irsv branch April 4, 2023 00:09

aviatesk added a commit to JuliaDebug/Cthulhu.jl that referenced this pull request Apr 4, 2023

adjust to JuliaLang/julia#48913 (#419)

af0fd55

aviatesk added a commit to JuliaDiff/Diffractor.jl that referenced this pull request Apr 5, 2023

adjust to the upstream irinterp refactoring

af1b39a

xref: JuliaLang/julia#48913

aviatesk mentioned this pull request Apr 5, 2023

adjust to the upstream irinterp refactoring JuliaDiff/Diffractor.jl#135

Merged

aviatesk added a commit to JuliaDiff/Diffractor.jl that referenced this pull request Apr 6, 2023

adjust to the upstream irinterp refactoring

a9ae1d2

xref: JuliaLang/julia#48913

christiangnrd mentioned this pull request Jul 13, 2024

Stack overflow during type inference #55102

Closed

vtjnash mentioned this pull request Aug 3, 2024

inference: fix missing LimitedAccuracy markers #55362

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

inference: refactoring to allow irinterp to perform `:call` inference #48913

inference: refactoring to allow irinterp to perform `:call` inference #48913

aviatesk commented Mar 6, 2023 •

edited

Loading

aviatesk commented Mar 14, 2023

Keno commented Mar 14, 2023

aviatesk commented Mar 16, 2023 •

edited

Loading

aviatesk commented Mar 16, 2023

nanosoldier commented Mar 30, 2023

Keno left a comment

aviatesk commented Apr 3, 2023

staticfloat commented Apr 4, 2023

vtjnash commented Apr 5, 2023

inference: refactoring to allow irinterp to perform :call inference #48913

inference: refactoring to allow irinterp to perform :call inference #48913

Conversation

aviatesk commented Mar 6, 2023 • edited Loading

aviatesk commented Mar 14, 2023

Keno commented Mar 14, 2023

aviatesk commented Mar 16, 2023 • edited Loading

aviatesk commented Mar 16, 2023

nanosoldier commented Mar 30, 2023

Keno left a comment

Choose a reason for hiding this comment

aviatesk commented Apr 3, 2023

staticfloat commented Apr 4, 2023

vtjnash commented Apr 5, 2023

inference: refactoring to allow irinterp to perform `:call` inference #48913

inference: refactoring to allow irinterp to perform `:call` inference #48913

aviatesk commented Mar 6, 2023 •

edited

Loading

aviatesk commented Mar 16, 2023 •

edited

Loading