-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
wip: overhaul EscapeAnalysis.jl #56849
base: master
Are you sure you want to change the base?
Conversation
f993acb
to
30e305b
Compare
30e305b
to
c4be5cc
Compare
0e1fc63
to
66eb5d2
Compare
can you move the renaming of |
The |
Rename in a first mechanical commit and then do the rest of the change? |
5664ae5
to
1b3d957
Compare
Of course it is possible to perform the rename, but the name That said, if it makes the review process easier, it might be worth trying. |
@nanosoldier |
c0a6fa9
to
f918549
Compare
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
df1b173
to
b3bc64f
Compare
6dacb43
to
9554f8c
Compare
- switched to the working set & flow-sensitive algorithm - implemented the new alias analysis design - added more tests - recovered inter-procedural propagation of escape information - made EA able to handle `new_nodes` - add explicit `nstmts::Int` field to `BlockEscapeState` - detect top escape with `TopLiveness` - define `AnalyzableIRElement` type alias - manage `aliasset` globally instead of on a per-block basis: While `aliasset` is necessary for propagating escape information, the convergence of the analysis is determined by the convergence of escape information, so the convergence of `aliasset` is not strictly required. - rename `escape_xxx` to `analyze_xxx` - propagate current state to handler state correctly Even if there are no changes made on current statement. - `analyze_invoke`: propagate the return value escape information - use generator for aliasset instead of collecting into array
9554f8c
to
4ff7d3f
Compare
x::Liveness == y::Liveness = begin | ||
@nospecialize | ||
if x === ⊥ₗ | ||
return y === ⊥ₗ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return y === ⊥ₗ | |
return (y === ⊥ₗ) || (y isa PCLiveness && length(y.pcs) == 0) |
If I got the lattice right, these are also lattice-equal (although I guess that's not strictly required)
""" | ||
x::EscapeInfo ⊔ₑ y::EscapeInfo = begin | ||
x::EscapeInfo ⊔ₑꜝ y::EscapeInfo = begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a while to realize that ⊔ₑꜝ
is an in-place union - the ꜝ
is hard enough to read that I think we might need a clearer call-out for the mutation
Maybe joinₑ!(...)
?
return new(aliases) | ||
end | ||
end | ||
struct UnknownMemoryInfo <: MemoryInfo end # not part of the `⊑ₘ` lattice, just a marker for `ssamemoryinfo` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not include this in the lattice as usual? Seems like an easy inclusion
@nospecialize | ||
if x isa MustAliasMemoryInfo | ||
if y isa MustAliasMemoryInfo | ||
return x.alias === y.alias |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this handle must-alias transitivity?
I'm thinking of must-alias chains e.g. x must-alias y
and y must-alias z
- Do we choose a 'representative' object for the set of must-aliases?
Thinking also of cases like a may-alias {x,y}
and then we prove x must-alias y
, which implies a must-alias y
# By incorporating some form of CFG information into `MemoryInfo`, it becomes possible | ||
# to enable load-forwarding even in cases where conflicts occur by inserting φ-nodes. | ||
|
||
abstract type MemoryInfo end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to AliasInfo
? Gives me a more immediate sense of what it does, anyway
end | ||
|
||
abstract type ObjectInfo end | ||
struct HasUnanalyzedMemory <: ObjectInfo end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct HasUnanalyzedMemory <: ObjectInfo end | |
struct NoMemoryContents <: ObjectInfo end |
Just a suggestion, but maybe closer to the intuition of this type as the oinfo lattice bottom
end | ||
xfields, yfields = x.fields, y.fields | ||
@goto compare_xfields_yfields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth a separate function
@@ -225,59 +328,114 @@ end | |||
The non-strict partial order over [`EscapeInfo`](@ref). | |||
""" | |||
x::EscapeInfo ⊑ₑ y::EscapeInfo = begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this relation is no longer used - is that temporary?
This PR aims to implement the new design for EscapeAnalysis.jl as proposed in https://hackmd.io/XKTmg0R0Tt2giLW56mWZ3A. While the actual implementation deviates slightly from the design document, the high-level goals and concepts remain the same.
The current EscapeAnalysis.jl is based on the old escape analysis design of Java's Graal compiler1 and suffers from the following issues:
The new EscapeAnalysis.jl takes inspiration from their (relatively) recent paper2 on partial escape analysis and adopts the following design:
With this new analysis, a more powerful SROA (or load-forwarding) and a better type inference for capturing closure become possible. It may also enable optimizations like allocation sinking and stack allocation in the future.
As a showcase to the capabilities achieved by the new EscapeAnalysis.jl, the analysis results for the examples presented in their paper2 are as follows:
Here,
builtin Base.getfield(%1, :idx)::Int64 (↦ _2)
andbuiltin Base.getfield(%1, :ref)::Any (↦ _3)
indicate that load-forwarding is possible for thesegetfield
operations. Additionally,EscapeAnalysis.has_no_escape(result.eresult.bbescapes[5][SSAValue(1)])
shows that%1 = %new(Main.Key, _2, _3)::Key
does not (yet) escape in basic block 5, which demonstrates that allocation sinking optimization is possible for this IR.While the new analysis captures significantly more information, its performance has not degraded substantially:
In particular, performance has improved for simpler cases:
Furthermore, while their paper does not discuss inter-procedural alias analysis in much detail, this PR also aims to implement inter-procedural alias analysis. Ultimately, the goal is to successfully analyze targets such as the following:
This PR is still a work in progress, and the following TODO list outlines the remaining tasks:
sroa_mutables!
using EscapeAnalysisMemoryInfo
CFG-aware(the actual application to optimizations (6–7) may be addressed in a separate PR)
Footnotes
Thomas Kotzmann and Hanspeter Mössenböck. 2005. Escape analysis in the context of dynamic compilation and deoptimization. In Proceedings of the 1st ACM/USENIX international conference on Virtual execution environments (VEE '05). Association for Computing Machinery, New York, NY, USA, 111–120. https://doi.org/10.1145/1064979.1064996 ↩
Lukas Stadler, Thomas Würthinger, and Hanspeter Mössenböck. 2018. Partial Escape Analysis and Scalar Replacement for Java. In Proceedings of Annual IEEE/ACM International Symposium on Code Generation and Optimization (CGO '14). Association for Computing Machinery, New York, NY, USA, 165–174. https://doi.org/10.1145/2544137.2544157 ↩ ↩2