Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractInterpreter: refactor the lifetimes of OptimizationState and IRCode #43994

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Jan 31, 2022

This commit limits the lifetimes of OptimizationState and IRCode
for a more dataflow clarity. It also avoids duplicated calls of ir_to_codeinf!.
With this change, for NativeInterpreter, Julia-level optimization-specific
information won't leak to other parts of the compilation pipeline, and
such information gets transformed to cacheable information at the end of
the optimization phase.

Note that external AbstractInterpreters can still extend their
lifetimes to cache additional information, as described by this
newly added documentation of finish!:

finish!(interp::AbstractInterpreter,
    opt::OptimizationState, ir::IRCode, caller::InferenceResult)

Runs post-Julia-level optimization process and caches information for later uses:

  • computes "purity" (i.e. side-effect-freeness) of the optimized frame
  • computes inlining cost and cache the inlineability in opt.src.inlineable
  • stores the result of optimization in caller.src
  • by default, caller.src will be an optimized CodeInfo object transformed from ir
  • in a case when this frame has been proven pure, ConstAPI object wrapping the constant
    value will be kept in caller.src instead, so that the runtime system will use
    the constant calling convention

!!! note
The lifetimes of opt and ir end by the end of this process.
Still external AbstractInterpreter can override transform_optresult_for_cache
as necessary to cache them. Note that transform_result_for_cache should be overloaded
also in such cases, otherwise the default implmentation of transform_result_for_cache
will discard any information other than CodeInfo, Vector{UInt8} or ConstAPI.


This commit also adds a new overload infresult_iterator so that external
interpreters can tweak the behavior of post processings of _typeinf.
Especially, this change is motivated by the need for JET, whose post-optimization
processing needs references of InferenceState.

@aviatesk
Copy link
Member Author

Cthulhu can be updated like this PR: JuliaDebug/Cthulhu.jl#264

@aviatesk aviatesk force-pushed the avi/optlifetime branch 3 times, most recently from 27afd1b to 37dc402 Compare February 1, 2022 05:44
aviatesk added a commit that referenced this pull request Feb 1, 2022
to make it easier to customize the behaviors of post processing of `_typeinf`.
Especially, this change is motivated by a need for JET, whose post processing
requires references of `InferenceState`s.

Separated from #43994.
@vtjnash
Copy link
Member

vtjnash commented Feb 9, 2022

seems good to me, if it works for you to make things better

aviatesk added a commit that referenced this pull request Feb 21, 2022
to make it easier to customize the behaviors of post processing of `_typeinf`.
Especially, this change is motivated by a need for JET, whose post processing
requires references of `InferenceState`s.

Separated from #43994.
@aviatesk aviatesk force-pushed the avi/optlifetime branch 4 times, most recently from 8e88697 to 8eb7175 Compare March 28, 2022 03:48
@aviatesk
Copy link
Member Author

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

@aviatesk
Copy link
Member Author

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo -n /nanosoldier/cset/bin/cset shield -e -- sudo -n -u nanosoldier-worker -- /nanosoldier/workdir/jl_q9V9Lu/benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo -n /nanosoldier/cset/bin/cset shield -e -- sudo -n -u nanosoldier-worker -- /nanosoldier/workdir/jl_wofuk5/benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk
Copy link
Member Author

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

@nanosoldier
Copy link
Collaborator

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

@aviatesk aviatesk force-pushed the avi/optlifetime branch 2 times, most recently from 4deac14 to 33c709d Compare April 4, 2022 08:28
…and `IRCode`

This commit limits the lifetimes of `OptimizationState` and `IRCode`
for a more dataflow clarity. It also avoids duplicated calls of `ir_to_codeinf!`.

Note that external `AbstractInterpreter`s can still extend their
lifetimes to cache additional information, as described by this
newly added documentation of `finish!`:

>     finish!(interp::AbstractInterpreter,
>         opt::OptimizationState, ir::IRCode, caller::InferenceResult)
>
> Runs post-Julia-level optimization process and caches information for later uses:
> - computes "purity" (i.e. side-effect-freeness) of the optimized frame
> - computes inlining cost and cache the inlineability in `opt.src.inlineable`
> - stores the result of optimization in `caller.src`
> * by default, `caller.src` will be an optimized `CodeInfo` object transformed from `ir`
> * in a case when this frame has been proven pure, `ConstAPI` object wrapping the constant
> value will be kept in `caller.src` instead, so that the runtime system will use
> the constant calling convention
>
> !!! note
>     The lifetimes of `opt` and `ir` end by the end of this process.
>     Still external `AbstractInterpreter` can override this method as necessary to cache them.
>     Note that `transform_result_for_cache` should be overloaded also in such cases,
>     otherwise the default `transform_result_for_cache` implmentation will discard any information
>     other than `CodeInfo`, `Vector{UInt8}` or `ConstAPI`.

This commit also adds a new overload `infresult_iterator` so that external
interpreters can tweak the behavior of post processings of `_typeinf`.
Especially, this change is motivated by the need for JET, whose post-optimization
processing needs references of `InferenceState`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants