Floats let bindings #76
Conversation
|
I'm assuming the top-level special case is to avoid turning into Because at the top-level it is still advantageous to deref into these structures. If we float here, then it may prevent access into otherwise statically known fields. Is that correct? If so, I have a few thoughts:
Additionally, on main, I've merged a Edit: I think an additional reason it might be slow is that this is relying on traverse, which build balanced tree of effects, where in the case of let bindings we always want right associated continuations. So we have to essentially build the intermediate Free tree, and then another pass to rewrite the tree to be right associated ( |
|
Can you remind why this is specifically done in eval? Is this only because of recursive eval? In the past I asked if we could unify let floating into a single pass as right now it's split between eval and build. The reason I ask, is that let floating in
Whereas if let floating is done only in build you would be able to float and do inline analysis in the same pass, only requiring the one subsequent eval. This makes me question if efficient let floating should all be done in |
|
FWIW, I think the main reason why we couldn't do all floating in eval is because |
|
Ah, I knew I had forgotten something. In the original PR I had done this: https://github.com/mikesol/purescript-backend-optimizer/blob/63fca1f8d71c888a4250d4ba76b8c77b3a9f3007/src/PureScript/Backend/Optimizer/Semantics.purs#L583C1-L594C15. I just added it back for completenesses sake to evaluate the approach in full, I'll address your question about |
Yeah, that's why. I had tried to put everything into eval (including the rewrites) but, for some reason, I couldn't replicate the behavior of the rewrite rule during the eval stage. We can try adding it back (on the The issue with putting it all in let x = 42 in {a: {b: {c:{d:{e:{f:let y=43 in y+x}}}}}}Obv this example would get inlined anyway but imagine that it didn't and the optimizer chose let floating. This approach, with the newly minded recursive So to summarize:
|
| @@ -613,12 +613,10 @@ evalAssocLet :: Env -> BackendSemantics -> (Env -> BackendSemantics -> BackendSe | |||
| evalAssocLet env sem go = case sem of | |||
| SemLet ident val k -> | |||
| SemLet ident val \nextVal1 -> | |||
| makeLet Nothing (k nextVal1) \nextVal2 -> | |||
| go (bindLocal (bindLocal env (One nextVal1)) (One nextVal2)) nextVal2 | |||
| evalAssocLet env (k nextVal1) go | |||
There was a problem hiding this comment.
I'm not sure this is recursive in the right way. I think to reassociate left-associated lets, would you want to recurse greedily on val rather than under the continuation?
I don't think this is necessarily true:
If it's not floating in one pass (floating is bubbling lets bottom up), then that just seems to be that you're missing other floating rules within But now that I think about it, I'm not sure if this works well with levels. If you pull out multiple lets in a record/array, they will all have the same levels (since they are essentially parallel evaluation contexts), so you can't naively just put them in the same block, as the references won't make any sense any more, you would have to do additional book keeping to propagate levels to the right places, which... is exactly what eval is doing. So that makes me think that floating should be in eval, and we should make a left recursive |
| runFloatLetsWithEnv atTop ee ff = go ff ee | ||
| where | ||
| go = resume >>> case _ of | ||
| Left (FloatMe sem f) -> \e1 -> if atTop then go (f sem) e1 else guardFail sem \sem' -> evalAssocLet e1 sem' \e2 v -> go (f v) e2 |
There was a problem hiding this comment.
evalAssocLet already guards on Fail, so the guardFail check here is unnecessary.
|
I think before moving forward more with this, I myself would like to take some time to understand and reason about eval-based let floating with regards to eliminating the build rewrite constraints in it's own PR. |
|
Ready for review 👍 |
| const $2 = f(); | ||
| const $3 = g(); |
There was a problem hiding this comment.
I still find this odd that these definitions are being swapped around. I'm not sure if that's incorrect, it just seems like a strange artifact. Do you have any idea what is happening here? It may be easier to tell if all the invocations were different functions.
There was a problem hiding this comment.
Just making sure things are correct & then I'll respond to the point about why this is happening:
In the old one, 0 == g, 1 == f, 2 == g, 3 == f, 4 == g, so the final result of 0,1,2,3,4 was g,f,g,f,g.
In the current one, 0 == g, 1 == f, 2 == f, 3 == g, 4 == f, so the final result of 0,2,3,1,4 is g, f, g, f, g.
So it's correct, but as to why it's happening, I don't know.
Here's the trace from the new one:
== Snapshot.FunctionCompose03 ==================================================
================================================================================
++ Snapshot.FunctionCompose03.test4 Step 1 (Original) ++++++++++++++++++++++++++
\f@0 g@1 ->
Snapshot.FunctionCompose03.compose
( Snapshot.FunctionCompose03.compose
( Snapshot.FunctionCompose03.compose
(g@1 Data.Unit.unit)
(f@0 Data.Unit.unit)
)
(g@1 Data.Unit.unit)
)
( Snapshot.FunctionCompose03.compose
(f@0 Data.Unit.unit)
(g@1 Data.Unit.unit)
)
-- Snapshot.FunctionCompose03.test4 Step 2 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = f@0 #[prim.undefined] in
{- Inline -}
let @5 = \x@5 -> @2 (@4 x@5) in
{- Inline -}
let
@6 =
let
@6 =
{- Inline -}
let
@6 =
let @6 = g@1 #[prim.undefined] in
\x@7 -> @5 (@6 x@7) in
@6 in
let @7 = g@1 #[prim.undefined] in
{- Inline -}
let @8 = \x@8 -> @3 (@7 x@8) in
\x@9 -> @6 (@8 x@9) in
@6
-- Snapshot.FunctionCompose03.test4 Step 3 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = f@0 #[prim.undefined] in
let @5 = g@1 #[prim.undefined] in
{- Inline -}
let
@6 =
\x@6 ->
{- Inline -}
let @7 = @5 x@6 in
@2 (@4 @7) in
let @7 = g@1 #[prim.undefined] in
\x@8 -> @6 (@3 (@7 x@8))
-- Snapshot.FunctionCompose03.test4 Step 4 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = f@0 #[prim.undefined] in
let @5 = g@1 #[prim.undefined] in
let @6 = g@1 #[prim.undefined] in
\x@7 ->
{- Inline -}
let @8 = @3 (@6 x@7) in
@2 (@4 (@5 @8))
-- Snapshot.FunctionCompose03.test4 Step 5 (Final) -----------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = f@0 #[prim.undefined] in
let @5 = g@1 #[prim.undefined] in
let @6 = g@1 #[prim.undefined] in
\x@7 -> @2 (@4 (@5 (@3 (@6 x@7))))and here's the old one:
== Snapshot.FunctionCompose03 ==================================================
================================================================================
++ Snapshot.FunctionCompose03.test4 Step 1 (Original) ++++++++++++++++++++++++++
\f@0 g@1 ->
Snapshot.FunctionCompose03.compose
( Snapshot.FunctionCompose03.compose
( Snapshot.FunctionCompose03.compose
(g@1 Data.Unit.unit)
(f@0 Data.Unit.unit)
)
(g@1 Data.Unit.unit)
)
( Snapshot.FunctionCompose03.compose
(f@0 Data.Unit.unit)
(g@1 Data.Unit.unit)
)
-- Snapshot.FunctionCompose03.test4 Step 2 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
{- Inline -}
let @4 = \x@4 -> @2 (@3 x@4) in
let
@5 =
{- Inline -}
let
@5 =
let @5 = g@1 #[prim.undefined] in
\x@6 -> @4 (@5 x@6) in
@5 in
{- Inline -}
let
@6 =
let @6 = f@0 #[prim.undefined] in
let
@7 =
let @7 = g@1 #[prim.undefined] in
{- Inline -}
let @8 = \x@8 -> @6 (@7 x@8) in
@8 in
\x@8 -> @5 (@7 x@8) in
@6
-- Snapshot.FunctionCompose03.test4 Step 3 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = g@1 #[prim.undefined] in
{- Inline -}
let
@5 =
\x@5 ->
{- Inline -}
let @6 = @4 x@5 in
@2 (@3 @6) in
let @6 = f@0 #[prim.undefined] in
let @7 = g@1 #[prim.undefined] in
{- Inline -}
let @8 = \x@8 -> @6 (@7 x@8) in
\x@9 -> @5 (@8 x@9)
-- Snapshot.FunctionCompose03.test4 Step 4 -------------------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = g@1 #[prim.undefined] in
let @5 = f@0 #[prim.undefined] in
let @6 = g@1 #[prim.undefined] in
\x@7 ->
{- Inline -}
let @8 = @5 (@6 x@7) in
@2 (@3 (@4 @8))
-- Snapshot.FunctionCompose03.test4 Step 5 (Final) -----------------------------
\f@0 g@1 ->
let @2 = g@1 #[prim.undefined] in
let @3 = f@0 #[prim.undefined] in
let @4 = g@1 #[prim.undefined] in
let @5 = f@0 #[prim.undefined] in
let @6 = g@1 #[prim.undefined] in
\x@7 -> @2 (@3 (@4 (@5 (@6 x@7))))Just reading through those, I can't figure out exactly what's going on yet.
There was a problem hiding this comment.
It's odd to me that in Step 2 there are a bunch of left-associative binds (level 6). I wouldn't expect that to happen since eval should be reassociating all binds. It looks like this is happening in the second case as well though.
|
I haven't forgotten about this, and this is something I want to integrate. This PR handles the top-level flattening of recursive binding groups by encoding information in the name and parsing it back out later. This is not something I want to merge, but I assume it was done to avoid refactoring this module. I've been working on this refactoring in some WIP commits, but I have not fully completed it yet due to life demands. I will try to get back to it as soon as I can. |
|
No worries!
I'm still a fan of the idea, but I stopped working on it because the stringly-typed programming felt icky and I wasn't sure about a clean way to refactor. If you've found a path forward there then that's great!
Mike Solomon
C.E.O. @meeshkan
https://www.meeshkan.com
Sent via Superhuman ( ***@***.*** )
…On Fri, Sep 22, 2023 at 01:52:56, Nathan Faubion < ***@***.*** > wrote:
I haven't forgotten about this, and this is something I want to integrate.
This PR handles the top-level flattening of recursive binding groups by
encoding information in the name and parsing it back out later. This is
not something I want to merge, but I assume it was done to avoid
refactoring this module. I've been working on this refactoring in some WIP
commits, but I have not fully completed it yet due to life demands. I will
try to get back to it as soon as I can.
—
Reply to this email directly, view it on GitHub (
#76 (comment)
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAEAIJXDRCQNUX2PTPZIE4TX3TAMRANCNFSM6AAAAAA25YMHN4
).
You are receiving this because you authored the thread. Message ID: <aristanetworks/purescript-backend-optimizer/pull/76/c1730452466
@ github. com>
|
Floats let bindings when strictness allows. This makes it easier to reason about
BackendSemanticsbecause, when we case on something, there's a higher chance that its subtree won't have closures fromletbindings.