[Git][ghc/ghc][wip/simplifier-tweaks] Try effect of
Simon Peyton Jones (@simonpj)
gitlab at gitlab.haskell.org
Mon Jul 31 22:43:55 UTC 2023
Simon Peyton Jones pushed to branch wip/simplifier-tweaks at Glasgow Haskell Compiler / GHC
Commits:
d8cb0ea9 by Simon Peyton Jones at 2023-07-31T23:33:52+01:00
Try effect of
* making multi-branch cases not work free (fixes #22423)
* use plan A for dataToTag and tagToEnum
- - - - -
3 changed files:
- compiler/GHC/Core/Opt/Simplify/Inline.hs
- compiler/GHC/Core/Opt/Simplify/Iteration.hs
- compiler/GHC/Core/Utils.hs
Changes:
=====================================
compiler/GHC/Core/Opt/Simplify/Inline.hs
=====================================
@@ -214,7 +214,7 @@ Reflections and wrinkles
* See also Note [Do not add unfoldings to join points at birth] in
GHC.Core.Opt.Simplify.Iteration
-* The case total case depth is really the wrong thing; it will inhibit inlining of a
+* The total case depth is really the wrong thing; it will inhibit inlining of a
local function, just because there is some giant case nest further out. What we
want is the /difference/ in case-depth between the binding site and the call site.
That could be done quite easily by adding the case-depth to the Unfolding of the
=====================================
compiler/GHC/Core/Opt/Simplify/Iteration.hs
=====================================
@@ -59,7 +59,7 @@ import GHC.Types.Basic
import GHC.Types.Tickish
import GHC.Types.Var ( isTyCoVar )
import GHC.Types.Var.Set
-import GHC.Builtin.PrimOps ( PrimOp (SeqOp) )
+import GHC.Builtin.PrimOps ( PrimOp (SeqOp, DataToTagOp, TagToEnumOp) )
import GHC.Builtin.Types.Prim( realWorldStatePrimTy )
import GHC.Builtin.Names( runRWKey )
@@ -3783,7 +3783,8 @@ mkDupableContWithDmds env _
where
thumbsUpPlanA (StrictBind {}) = True
thumbsUpPlanA (Stop {}) = True
- thumbsUpPlanA (Select {}) = False -- Using Plan B benefits carryPropagate
+ thumbsUpPlanA (Select {}) = dup_fun fun
+-- False -- Using Plan B benefits carryPropagate
-- in nofib digits-of-e2
thumbsUpPlanA (StrictArg {}) = False
thumbsUpPlanA (CastIt { sc_cont = k }) = thumbsUpPlanA k
@@ -3791,6 +3792,14 @@ mkDupableContWithDmds env _
thumbsUpPlanA (ApplyToVal { sc_cont = k }) = thumbsUpPlanA k
thumbsUpPlanA (ApplyToTy { sc_cont = k }) = thumbsUpPlanA k
+ dup_fun fun | Just op <- isPrimOpId_maybe (ai_fun fun)
+ = case op of
+ DataToTagOp -> True
+ TagToEnumOp -> True
+ _ -> False
+ | otherwise
+ = False
+
mkDupableContWithDmds env dmds
(ApplyToTy { sc_cont = cont, sc_arg_ty = arg_ty, sc_hole_ty = hole_ty })
= do { (floats, cont') <- mkDupableContWithDmds env dmds cont
@@ -3955,8 +3964,7 @@ mkDupableAlt _env case_bndr jfloats (Alt con alt_bndrs alt_rhs_in)
-- See Note [Duplicated env]
ok_to_dup_alt :: OutId -> [OutVar] -> OutExpr -> Bool
--- See Note [Duplicating alternatives]
--- and Note [Duplicating join points] esp point (2)
+-- See Note [Duplicating join points] esp points (DJ2,DJ3)
ok_to_dup_alt case_bndr alt_bndrs alt_rhs
| exprIsTrivial alt_rhs
= True -- Includes things like (case x of {})
@@ -3964,7 +3972,7 @@ ok_to_dup_alt case_bndr alt_bndrs alt_rhs
| (Var v, args) <- collectArgs alt_rhs
, all exprIsTrivial args
= if isJust (isDataConId_maybe v)
- then -- See Note [Duplicating join points] for the
+ then -- See Note [Duplicating join points] (DJ3) for the
-- reason for this apparently strange test
exprsFreeIds args `subVarSet` bndr_set
else True -- Duplicating a simple call (f a b c) is fine,
@@ -4016,7 +4024,7 @@ Wrinkle
where $j is a freshly-born join point. After case-of-known-constructor
wo we end up substituting (join $j x = <rhs> in jblah) for `y` in `blah`;
and thus we re-simplify that join binding. In test T15630 this results in
- masssive duplication.
+ massive duplication.
So in `simplLetUnfolding` we spot this case a bit hackily; a freshly-born
join point will have OccInfo of ManyOccs, unlike an existing join point which
@@ -4027,30 +4035,6 @@ I can't quite articulate precisely why this is so important. But it makes a MAS
difference in T15630 (a fantastic test case); and at worst it'll merely delay inlining
join points by one simplifier iteration.
-Note [Duplicating alternatives]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-When should we duplicate an alternative, and when should we make a join point?
-We don't want to make a join point if it will /definitely/ be inlined; that
-just takes extra work to build, and an extra Simplifier iteration to do the
-inlining. So consider
-
- case (case x of True -> e2; False -> e2) of
- K1 a b -> f b a
- K2 x -> g x v
- K3 v -> Just v
-
-The (f b a) would turn into a join point like
- $j1 a b = f b a
-which would immediately inline again because the call is not smaller than the RHS.
-On the other hand, the (g x v) turns into
- $j2 x = g x v
-which won't imediately inline, because the call $j2 x is smaller than the RHS
-(g x v). Finally the (Just v) would turn into
- $j3 v = Just v
-and you might think that would immediately inline.
-
-TODO -- more here
-
Note [Fusing case continuations]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
It's important to fuse two successive case continuations when the
@@ -4094,7 +4078,7 @@ inlining join points. Consider
K g y -> blah[g,y]
Here the join-point RHS is very small, just a constructor
-application (K x y). So we might inline it to get
+application (K f x). So we might inline it to get
case (case v of )
( p1 -> K f x1 ) of
( p2 -> K f x2 )
@@ -4115,50 +4099,55 @@ what `f` is, instead of lambda-abstracting over it.
To achieve this:
-1. Do not postInlineUnconditionally a join point, ever. Doing
+(DJ1) Do not postInlineUnconditionally a join point, ever. Doing
postInlineUnconditionally is primarily to push allocation into cold
branches; but a join point doesn't allocate, so that's a non-motivation.
-2. In mkDupableAlt and mkDupableStrictBind, generate an alterative for all
- alternatives, except for exprIsTrival RHSs (see `ok_to_dup_alt`). Previously
- we used exprIsDupable. This generates a lot more join points, but makes them
- much more case-of-case friendly.
-
- We are happy to duplicate
- j a b = K b a
- where all the arguments of the constructor are parameters of the join point
- because then the "massive difference" described above can't happen.
-
- It is definitely worth checking for exprIsTrivial, otherwise we get
- an extra Simplifier iteration, because it is inlined in the next
- round.
-
-3. By the same token we want to use Plan B in
- Note [Duplicating StrictArg] when the RHS of the new join point
- is a data constructor application. See the call to isDataConId in
- the StrictArg case of mkDupableContWithDmds.
-
- That same Note [Duplicating StrictArg] explains why we sometimes
- want Plan A when the RHS of the new join point would be a
- non-data-constructor application
-
-4. You might worry that $j will be inlined by the call-site inliner,
- but it won't because the call-site context for a join is usually
- extremely boring (the arguments come from the pattern match).
- And if not, then perhaps inlining it would be a good idea.
-
- You might also wonder if we get UnfWhen, because the RHS of the
- join point is no bigger than the call. But in the cases we care
- about it will be a little bigger, because of that free `f` in
- $j x = K f x
- So for now we don't do anything special in callSiteInline
-
-There is a bit of tension between (2) and (3). Do we want to retain
+(DJ2) In mkDupableAlt and mkDupableStrictBind, generate an alterative for /all/
+ alternatives, /except/ for ones that will definitely inline unconditionally
+ straight away. (In that case it's silly to make a join point in the first
+ place; it just takes an extra Simplifier iteration to undo.) This choice is
+ made by `ok_to_dup_alt`.
+
+ This plan generates a lot of join points, but makes them much more
+ case-of-case friendly.
+
+(DJ3) When does a join point definitely inline unconditionally? That is, when
+ the UnfoldingGuidance is UnfWhen: the rhs of the join point is smaller than
+ the call. More specifically `ok_to_dup_alt` looks for
+ * (exprIsTrivial rhs); this includes uses of unsafeEqualityProof etc; seee
+ the defn of exprIsTrivial.
+ * the RHS is a call (f x y z), where the arguments are all trivial and f is not
+ data constructor
+ * if the RHS /is/ a data constructor we check whether all the args are bound by
+ the join-point lambdas; if so there is no point in creating a join point. But
+ if not (e.g. j x = K f x), then we /do/ want to creat a join point; see
+ the discussion of #19996 above.
+
+(DJ4) By the same token we want to use Plan B in Note [Duplicating StrictArg] when
+ the RHS of the new join point is a data constructor application. See the
+ call to isDataConId in the StrictArg case of mkDupableContWithDmds.
+
+ That same Note [Duplicating StrictArg] explains why we sometimes want Plan A
+ when the RHS of the new join point would be a non-data-constructor
+ application
+
+(DJ5) You might worry that $j will be inlined by the call-site inliner, but it
+ won't because the call-site context for a join is usually extremely boring
+ (the arguments come from the pattern match). And if not, then perhaps
+ inlining it would be a good idea.
+
+ You might also wonder if we get UnfWhen, because the RHS of the join point is
+ no bigger than the call. But in the cases we care about it will be a little
+ bigger, because of that free `f` in $j x = K f x So for now we don't do
+ anything special in callSiteInline
+
+There is a bit of tension between (DJ2) and (DJ3). Do we want to retain
the join point only when the RHS is
* a constructor application? or
* just non-trivial?
Currently, a bit ad-hoc, but we definitely want to retain the join
-point for data constructors in mkDupableALt (point 2); that is the
+point for data constructors in mkDupableALt (DJ2); that is the
whole point of #19996 described above.
Historical Note [Case binders and join points]
@@ -4567,17 +4556,6 @@ Wrinkles
in GHC.Core.Opt.Simplify.Utils. We uphold this because the join-point
case (bind_cxt = BC_Join {}) doesn't use eta_expand.
-Note [Heavily used join points]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-After inining join points we can end up with
- let $j x = <rhs>
- in case x1 of
- True -> case x2 of
- True -> $j blah1
- False -> $j blah2
- False -> case x3 of ....
-with a huge case tree
-
Note [Force bottoming field]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
We need to force bottoming, or the new unfolding holds
=====================================
compiler/GHC/Core/Utils.hs
=====================================
@@ -1266,8 +1266,11 @@ exprIsCheapX ok_app e
go _ (Type {}) = True
go _ (Coercion {}) = True
go n (Cast e _) = go n e
- go n (Case scrut _ _ alts) = ok scrut &&
- and [ go n rhs | Alt _ _ rhs <- alts ]
+ go n (Case scrut _ _ alts)
+ | [Alt _ _ rhs] <- alts = ok scrut && ok rhs
+ | otherwise = False
+-- go n (Case scrut _ _ alts) = ok scrut &&
+-- and [ go n rhs | Alt _ _ rhs <- alts ]
go n (Tick t e) | tickishCounts t = False
| otherwise = go n e
go n (Lam x e) | isRuntimeVar x = n==0 || go (n-1) e
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/d8cb0ea96671889e2c39245f82952454af4ba86e
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/d8cb0ea96671889e2c39245f82952454af4ba86e
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20230731/2fe94ce4/attachment-0001.html>
More information about the ghc-commits
mailing list