summaryrefslogtreecommitdiff
path: root/compiler/GHC
diff options
context:
space:
mode:
authorAlexis King <lexi.lambda@gmail.com>2020-04-17 16:43:49 -0500
committerMarge Bot <ben+marge-bot@smart-cactus.org>2020-04-22 23:11:12 -0400
commit6c9fae2342f19ab3e6ac688825a3817b23bf1fcc (patch)
treeb996d1bdca3e275c77b61de77e54ea107b771d19 /compiler/GHC
parent401f7bb312aa6c570287d313f8b587aaebca72b2 (diff)
downloadhaskell-6c9fae2342f19ab3e6ac688825a3817b23bf1fcc.tar.gz
Mark DataCon wrappers CONLIKE
Now that DataCon wrappers don’t inline until phase 0 (see commit b78cc64e923716ac0512c299f42d4d0012306c05), it’s important that case-of-known-constructor and RULE matching be able to see saturated applications of DataCon wrappers in unfoldings. Making them conlike is a natural way to do it, since they are, in fact, precisely the sort of thing the CONLIKE pragma exists to solve. Fixes #18012. This also bumps the version of the parsec submodule to incorporate a patch that avoids a metric increase on the haddock perf tests. The increase was not really a flaw in this patch, as parsec was implicitly relying on inlining heuristics. The patch to parsec just adds some INLINABLE pragmas, and we get a nice performance bump out of it (well beyond the performance we lost from this patch). Metric Decrease: T12234 WWRec haddock.Cabal haddock.base haddock.compiler
Diffstat (limited to 'compiler/GHC')
-rw-r--r--compiler/GHC/Core/SimpleOpt.hs61
-rw-r--r--compiler/GHC/Core/Utils.hs17
-rw-r--r--compiler/GHC/Types/Id.hs2
-rw-r--r--compiler/GHC/Types/Id/Make.hs45
4 files changed, 111 insertions, 14 deletions
diff --git a/compiler/GHC/Core/SimpleOpt.hs b/compiler/GHC/Core/SimpleOpt.hs
index 0728ea11c8..7545209b77 100644
--- a/compiler/GHC/Core/SimpleOpt.hs
+++ b/compiler/GHC/Core/SimpleOpt.hs
@@ -889,6 +889,10 @@ And now we have a known-constructor MkT that we can return.
Notice that both (2) and (3) require exprIsConApp_maybe to gather and return
a bunch of floats, both let and case bindings.
+Note that this strategy introduces some subtle scenarios where a data-con
+wrapper can be replaced by a data-con worker earlier than we’d like, see
+Note [exprIsConApp_maybe for data-con wrappers: tricky corner].
+
Note [beta-reduction in exprIsConApp_maybe]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The unfolding a definition (_e.g._ a let-bound variable or a datacon wrapper) is
@@ -949,6 +953,60 @@ exprIsConApp_maybe does not return Just) then nothing happens, and nothing
will happen the next time either.
See test T16254, which checks the behavior of newtypes.
+
+Note [exprIsConApp_maybe for data-con wrappers: tricky corner]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Generally speaking
+
+ * exprIsConApp_maybe honours the inline phase; that is, it does not look
+ inside the unfolding for an Id unless its unfolding is active in this phase.
+ That phase-sensitivity is expressed in the InScopeEnv (specifically, the
+ IdUnfoldingFun component of the InScopeEnv) passed to exprIsConApp_maybe.
+
+ * Data-constructor wrappers are active only in phase 0 (the last phase);
+ see Note [Activation for data constructor wrappers] in GHC.Types.Id.Make.
+
+On the face of it that means that exprIsConApp_maybe won't look inside data
+constructor wrappers until phase 0. But that seems pretty Bad. So we cheat.
+For data con wrappers we unconditionally look inside its unfolding, regardless
+of phase, so that we get case-of-known-constructor to fire in every phase.
+
+Perhaps unsurprisingly, this cheating can backfire. An example:
+
+ data T = C !A B
+ foo p q = let x = C e1 e2 in seq x $ f x
+ {-# RULE "wurble" f (C a b) = b #-}
+
+In Core, the RHS of foo is
+
+ let x = $WC e1 e2 in case x of y { C _ _ -> f x }
+
+and after doing a binder swap and inlining x, we have:
+
+ case $WC e1 e2 of y { C _ _ -> f y }
+
+Case-of-known-constructor fires, but now we have to reconstruct a binding for
+`y` (which was dead before the binder swap) on the RHS of the case alternative.
+Naturally, we’ll use the worker:
+
+ case e1 of a { DEFAULT -> let y = C a e2 in f y }
+
+and after inlining `y`, we have:
+
+ case e1 of a { DEFAULT -> f (C a e2) }
+
+Now we might hope the "wurble" rule would fire, but alas, it will not: we have
+replaced $WC with C, but the (desugared) rule matches on $WC! We weren’t
+supposed to inline $WC yet for precisely that reason (see Note [Activation for
+data constructor wrappers]), but our cheating in exprIsConApp_maybe came back to
+bite us.
+
+This is rather unfortunate, especially since this can happen inside stable
+unfoldings as well as ordinary code (which really happened, see !3041). But
+there is no obvious solution except to delay case-of-known-constructor on
+data-con wrappers, and that cure would be worse than the disease.
+
+This Note exists solely to document the problem.
-}
data ConCont = CC [CoreExpr] Coercion
@@ -1033,7 +1091,8 @@ exprIsConApp_maybe (in_scope, id_unf) expr
-- Look through data constructor wrappers: they inline late (See Note
-- [Activation for data constructor wrappers]) but we want to do
- -- case-of-known-constructor optimisation eagerly.
+ -- case-of-known-constructor optimisation eagerly (see Note
+ -- [exprIsConApp_maybe on data constructors with wrappers]).
| isDataConWrapId fun
, let rhs = uf_tmpl (realIdUnfolding fun)
= go (Left in_scope) floats rhs cont
diff --git a/compiler/GHC/Core/Utils.hs b/compiler/GHC/Core/Utils.hs
index a0704ef03a..d954374eef 100644
--- a/compiler/GHC/Core/Utils.hs
+++ b/compiler/GHC/Core/Utils.hs
@@ -91,7 +91,7 @@ import GHC.Builtin.Types.Prim
import FastString
import Maybes
import ListSetOps ( minusList )
-import GHC.Types.Basic ( Arity, isConLike )
+import GHC.Types.Basic ( Arity )
import Util
import Pair
import Data.ByteString ( ByteString )
@@ -1387,15 +1387,14 @@ isExpandableApp fn n_val_args
| isWorkFreeApp fn n_val_args = True
| otherwise
= case idDetails fn of
- DataConWorkId {} -> True -- Actually handled by isWorkFreeApp
- RecSelId {} -> n_val_args == 1 -- See Note [Record selection]
- ClassOpId {} -> n_val_args == 1
- PrimOpId {} -> False
- _ | isBottomingId fn -> False
+ RecSelId {} -> n_val_args == 1 -- See Note [Record selection]
+ ClassOpId {} -> n_val_args == 1
+ PrimOpId {} -> False
+ _ | isBottomingId fn -> False
-- See Note [isExpandableApp: bottoming functions]
- | isConLike (idRuleMatchInfo fn) -> True
- | all_args_are_preds -> True
- | otherwise -> False
+ | isConLikeId fn -> True
+ | all_args_are_preds -> True
+ | otherwise -> False
where
-- See if all the arguments are PredTys (implicit params or classes)
diff --git a/compiler/GHC/Types/Id.hs b/compiler/GHC/Types/Id.hs
index fab72d23de..713f1c6258 100644
--- a/compiler/GHC/Types/Id.hs
+++ b/compiler/GHC/Types/Id.hs
@@ -768,7 +768,7 @@ idRuleMatchInfo :: Id -> RuleMatchInfo
idRuleMatchInfo id = inlinePragmaRuleMatchInfo (idInlinePragma id)
isConLikeId :: Id -> Bool
-isConLikeId id = isDataConWorkId id || isConLike (idRuleMatchInfo id)
+isConLikeId id = isConLike (idRuleMatchInfo id)
{-
---------------------------------
diff --git a/compiler/GHC/Types/Id/Make.hs b/compiler/GHC/Types/Id/Make.hs
index ce5012458a..d9d137a13b 100644
--- a/compiler/GHC/Types/Id/Make.hs
+++ b/compiler/GHC/Types/Id/Make.hs
@@ -510,19 +510,21 @@ mkDataConWorkId wkr_name data_con
alg_wkr_info = noCafIdInfo
`setArityInfo` wkr_arity
`setCprInfo` mkCprSig wkr_arity (dataConCPR data_con)
+ `setInlinePragInfo` wkr_inline_prag
`setUnfoldingInfo` evaldUnfolding -- Record that it's evaluated,
-- even if arity = 0
`setLevityInfoWithType` wkr_ty
-- NB: unboxed tuples have workers, so we can't use
-- setNeverLevPoly
+ wkr_inline_prag = defaultInlinePragma { inl_rule = ConLike }
wkr_arity = dataConRepArity data_con
----------- Workers for newtypes --------------
univ_tvs = dataConUnivTyVars data_con
arg_tys = dataConRepArgTys data_con -- Should be same as dataConOrigArgTys
nt_work_info = noCafIdInfo -- The NoCaf-ness is set by noCafIdInfo
`setArityInfo` 1 -- Arity 1
- `setInlinePragInfo` alwaysInlinePragma
+ `setInlinePragInfo` dataConWrapperInlinePragma
`setUnfoldingInfo` newtype_unf
`setLevityInfoWithType` wkr_ty
id_arg1 = mkTemplateLocal 1 (head arg_tys)
@@ -652,8 +654,8 @@ mkDataConRep dflags fam_envs wrap_name mb_bangs data_con
mk_dmd str | isBanged str = evalDmd
| otherwise = topDmd
- wrap_prag = alwaysInlinePragma `setInlinePragmaActivation`
- activeDuringFinal
+ wrap_prag = dataConWrapperInlinePragma
+ `setInlinePragmaActivation` activeDuringFinal
-- See Note [Activation for data constructor wrappers]
-- The wrapper will usually be inlined (see wrap_unf), so its
@@ -763,6 +765,12 @@ mkDataConRep dflags fam_envs wrap_name mb_bangs data_con
; expr <- mk_rep_app prs (mkVarApps con_app rep_ids)
; return (unbox_fn expr) }
+
+dataConWrapperInlinePragma :: InlinePragma
+-- See Note [DataCon wrappers are conlike]
+dataConWrapperInlinePragma = alwaysInlinePragma { inl_rule = ConLike
+ , inl_inline = Inline }
+
{- Note [Activation for data constructor wrappers]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The Activation on a data constructor wrapper allows it to inline only in Phase
@@ -784,6 +792,37 @@ the order of type argument could make previously working RULEs fail.
See also https://gitlab.haskell.org/ghc/ghc/issues/15840 .
+Note [DataCon wrappers are conlike]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+DataCon workers are clearly ConLike --- they are the “Con” in
+“ConLike”, after all --- but what about DataCon wrappers? Should they
+be marked ConLike, too?
+
+Yes, absolutely! As described in Note [CONLIKE pragma] in
+GHC.Types.Basic, isConLike influences GHC.Core.Utils.exprIsExpandable,
+which is used by both RULE matching and the case-of-known-constructor
+optimization. It’s crucial that both of those things can see
+applications of DataCon wrappers:
+
+ * User-defined RULEs match on wrappers, not workers, so we might
+ need to look through an unfolding built from a DataCon wrapper to
+ determine if a RULE matches.
+
+ * Likewise, if we have something like
+ let x = $WC a b in ... case x of { C y z -> e } ...
+ we still want to apply case-of-known-constructor.
+
+Therefore, it’s important that we consider DataCon wrappers conlike.
+This is especially true now that we don’t inline DataCon wrappers
+until the final simplifier phase; see Note [Activation for data
+constructor wrappers].
+
+For further reading, see:
+ * Note [Conlike is interesting] in GHC.Core.Op.Simplify.Utils
+ * Note [Lone variables] in GHC.Core.Unfold
+ * Note [exprIsConApp_maybe on data constructors with wrappers]
+ in GHC.Core.SimpleOpt
+ * #18012
Note [Bangs on imported data constructors]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~