summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon Peyton Jones <simonpj@microsoft.com>2013-09-02 09:46:59 +0100
committerSimon Peyton Jones <simonpj@microsoft.com>2013-09-02 09:52:45 +0100
commite4a1d2d0a71bf335a04eaf93deb440b709f9430e (patch)
tree0656b50cd5455ffb0a960be910ea52fb45444252
parent5f98d44d8617756971cf47c040f2556de4e98f63 (diff)
downloadhaskell-e4a1d2d0a71bf335a04eaf93deb440b709f9430e.tar.gz
Remove the final vestiges of InlineWrappers
Part of Nick Frisby's patch (c080f727ba5f83921b842fcff71e9066adbdc250) for late demand-analysis removed the over-zealous short-cut whereby strictness wrappers were not spelled out in detail in interface files. This patch completes the process by * removing InlineWrapper from UnfoldingSource * removing IfWrapper from IfaceUnfolding There was a tiny bit of special ad-hocery for wrappers, in OccurAnal, but fortunately that too turns out to be rendered irrelevant by the more uniform treatment, and after that there was no need to remember which functions are wrappers.
-rw-r--r--compiler/coreSyn/CoreSyn.lhs31
-rw-r--r--compiler/coreSyn/CoreUnfold.lhs2
-rw-r--r--compiler/coreSyn/PprCore.lhs1
-rw-r--r--compiler/iface/IfaceSyn.lhs15
-rw-r--r--compiler/iface/MkIface.lhs1
-rw-r--r--compiler/iface/TcIface.lhs29
-rw-r--r--compiler/simplCore/OccurAnal.lhs70
7 files changed, 63 insertions, 86 deletions
diff --git a/compiler/coreSyn/CoreSyn.lhs b/compiler/coreSyn/CoreSyn.lhs
index dd7307d190..baa28bc0cc 100644
--- a/compiler/coreSyn/CoreSyn.lhs
+++ b/compiler/coreSyn/CoreSyn.lhs
@@ -714,7 +714,9 @@ data Unfolding
------------------------------------------------
data UnfoldingSource
- = InlineRhs -- The current rhs of the function
+ = -- See also Note [Historical note: unfoldings for wrappers]
+
+ InlineRhs -- The current rhs of the function
-- Replace uf_tmpl each time around
| InlineStable -- From an INLINE or INLINABLE pragma
@@ -739,13 +741,6 @@ data UnfoldingSource
-- (see MkId.lhs, calls to mkCompulsoryUnfolding).
-- Inline absolutely always, however boring the context.
- | InlineWrapper -- This unfolding is the wrapper in a
- -- worker/wrapper split from the strictness
- -- analyser
- --
- -- cf some history in TcIface's Note [wrappers
- -- in interface files]
-
-- | 'UnfoldingGuidance' says when unfolding should take place
@@ -775,6 +770,25 @@ data UnfoldingGuidance
| UnfNever -- The RHS is big, so don't inline it
\end{code}
+Note [Historical note: unfoldings for wrappers]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+We used to have a nice clever scheme in interface files for
+wrappers. A wrapper's unfolding can be reconstructed from its worker's
+id and its strictness. This decreased .hi file size (sometimes
+significantly, for modules like GHC.Classes with many high-arity w/w
+splits) and had a slight corresponding effect on compile times.
+
+However, when we added the second demand analysis, this scheme lead to
+some Core lint errors. The second analysis could change the strictness
+signatures, which sometimes resulted in a wrapper's regenerated
+unfolding applying the wrapper to too many arguments.
+
+Instead of repairing the clever .hi scheme, we abandoned it in favor
+of simplicity. The .hi sizes are usually insignificant (excluding the
++1M for base libraries), and compile time barely increases (~+1% for
+nofib). The nicer upshot is that the UnfoldingSource no longer mentions
+an Id, so, eg, substitutions need not traverse them.
+
Note [DFun unfoldings]
~~~~~~~~~~~~~~~~~~~~~~
@@ -844,7 +858,6 @@ isStableSource :: UnfoldingSource -> Bool
-- Keep the unfolding template
isStableSource InlineCompulsory = True
isStableSource InlineStable = True
-isStableSource InlineWrapper = True
isStableSource InlineRhs = False
-- | Retrieves the template of an unfolding: panics if none is known
diff --git a/compiler/coreSyn/CoreUnfold.lhs b/compiler/coreSyn/CoreUnfold.lhs
index bbf9e0eb40..896f3723d2 100644
--- a/compiler/coreSyn/CoreUnfold.lhs
+++ b/compiler/coreSyn/CoreUnfold.lhs
@@ -103,7 +103,7 @@ mkDFunUnfolding bndrs con ops
mkWwInlineRule :: CoreExpr -> Arity -> Unfolding
mkWwInlineRule expr arity
- = mkCoreUnfolding InlineWrapper True
+ = mkCoreUnfolding InlineStable True
(simpleOptExpr expr) arity
(UnfWhen unSaturatedOk boringCxtNotOk)
diff --git a/compiler/coreSyn/PprCore.lhs b/compiler/coreSyn/PprCore.lhs
index 64e7d63590..00f9a9346f 100644
--- a/compiler/coreSyn/PprCore.lhs
+++ b/compiler/coreSyn/PprCore.lhs
@@ -422,7 +422,6 @@ instance Outputable UnfoldingGuidance where
instance Outputable UnfoldingSource where
ppr InlineCompulsory = ptext (sLit "Compulsory")
- ppr InlineWrapper = ptext (sLit "Wrapper")
ppr InlineStable = ptext (sLit "InlineStable")
ppr InlineRhs = ptext (sLit "<vanilla>")
diff --git a/compiler/iface/IfaceSyn.lhs b/compiler/iface/IfaceSyn.lhs
index 8dc4188bb9..f6e68e2836 100644
--- a/compiler/iface/IfaceSyn.lhs
+++ b/compiler/iface/IfaceSyn.lhs
@@ -583,8 +583,6 @@ data IfaceUnfolding
Bool -- OK to inline even if context is boring
IfaceExpr
- | IfWrapper IfaceExpr -- cf TcIface's Note [wrappers in interface files]
-
| IfDFunUnfold [IfaceBndr] [IfaceExpr]
instance Binary IfaceUnfolding where
@@ -598,15 +596,12 @@ instance Binary IfaceUnfolding where
put_ bh b
put_ bh c
put_ bh d
- put_ bh (IfWrapper e) = do
- putByte bh 2
- put_ bh e
put_ bh (IfDFunUnfold as bs) = do
- putByte bh 3
+ putByte bh 2
put_ bh as
put_ bh bs
put_ bh (IfCompulsory e) = do
- putByte bh 4
+ putByte bh 3
put_ bh e
get bh = do
h <- getByte bh
@@ -619,9 +614,7 @@ instance Binary IfaceUnfolding where
c <- get bh
d <- get bh
return (IfInlineRule a b c d)
- 2 -> do e <- get bh
- return (IfWrapper e)
- 3 -> do as <- get bh
+ 2 -> do as <- get bh
bs <- get bh
return (IfDFunUnfold as bs)
_ -> do e <- get bh
@@ -1288,7 +1281,6 @@ instance Outputable IfaceUnfolding where
ppr (IfInlineRule a uok bok e) = sep [ptext (sLit "InlineRule")
<+> ppr (a,uok,bok),
pprParendIfaceExpr e]
- ppr (IfWrapper e) = ptext (sLit "Wrapper:") <+> parens (ppr e)
ppr (IfDFunUnfold bs es) = hang (ptext (sLit "DFun:") <+> sep (map ppr bs) <> dot)
2 (sep (map pprParendIfaceExpr es))
@@ -1446,7 +1438,6 @@ freeNamesIfUnfold :: IfaceUnfolding -> NameSet
freeNamesIfUnfold (IfCoreUnfold _ e) = freeNamesIfExpr e
freeNamesIfUnfold (IfCompulsory e) = freeNamesIfExpr e
freeNamesIfUnfold (IfInlineRule _ _ _ e) = freeNamesIfExpr e
-freeNamesIfUnfold (IfWrapper e) = freeNamesIfExpr e
freeNamesIfUnfold (IfDFunUnfold bs es) = fnList freeNamesIfBndr bs &&& fnList freeNamesIfExpr es
freeNamesIfExpr :: IfaceExpr -> NameSet
diff --git a/compiler/iface/MkIface.lhs b/compiler/iface/MkIface.lhs
index d3b56d1f7b..44f99d520e 100644
--- a/compiler/iface/MkIface.lhs
+++ b/compiler/iface/MkIface.lhs
@@ -1762,7 +1762,6 @@ toIfUnfolding lb (CoreUnfolding { uf_tmpl = rhs, uf_arity = arity
-> case guidance of
UnfWhen unsat_ok boring_ok -> IfInlineRule arity unsat_ok boring_ok if_rhs
_other -> IfCoreUnfold True if_rhs
- InlineWrapper -> IfWrapper if_rhs
InlineCompulsory -> IfCompulsory if_rhs
InlineRhs -> IfCoreUnfold False if_rhs
-- Yes, even if guidance is UnfNever, expose the unfolding
diff --git a/compiler/iface/TcIface.lhs b/compiler/iface/TcIface.lhs
index dffd69b9ed..e1077e0f2d 100644
--- a/compiler/iface/TcIface.lhs
+++ b/compiler/iface/TcIface.lhs
@@ -1204,25 +1204,6 @@ do_one (IfaceRec pairs) thing_inside
%* *
%************************************************************************
-Note [wrappers in interface files]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-We used to have a nice clever scheme in interface files for
-wrappers. A wrapper's unfolding can be reconstructed from its worker's
-id and its strictness. This decreased .hi file size (sometimes
-significantly, for modules like GHC.Classes with many high-arity w/w
-splits) and had a slight corresponding effect on compile times.
-
-However, when we added the second demand analysis, this scheme lead to
-some Core lint errors. The second analysis could change the strictness
-signatures, which sometimes resulted in a wrapper's regenerated
-unfolding applying the wrapper to too many arguments.
-
-Instead of repairing the clever .hi scheme, we abandoned it in favor
-of simplicity. The .hi sizes are usually insignificant (excluding the
-+1M for base libraries), and compile time barely increases (~+1% for
-nofib). The nicer upshot is that unfolding sources no longer include
-an Id, so, eg, substitutions need not traverse them any longer.
-
\begin{code}
tcIdDetails :: Type -> IfaceIdDetails -> IfL IdDetails
tcIdDetails _ IfVanillaId = return VanillaId
@@ -1300,16 +1281,6 @@ tcUnfolding name dfun_ty _ (IfDFunUnfold bs ops)
where
doc = text "Class ops for dfun" <+> ppr name
(_, _, cls, _) = tcSplitDFunTy dfun_ty
-
-tcUnfolding name _ info (IfWrapper if_expr)
- = do { mb_expr <- tcPragExpr name if_expr
- ; return $ case mb_expr of
- Nothing -> NoUnfolding
- Just expr -> mkWwInlineRule expr arity -- see Note [wrappers in interface files]
- }
- where
- -- Arity should occur before unfolding!
- arity = arityInfo info
\end{code}
For unfoldings we try to do the job lazily, so that we never type check
diff --git a/compiler/simplCore/OccurAnal.lhs b/compiler/simplCore/OccurAnal.lhs
index 75d5364f63..d17b0561f5 100644
--- a/compiler/simplCore/OccurAnal.lhs
+++ b/compiler/simplCore/OccurAnal.lhs
@@ -878,14 +878,13 @@ reOrderNodes depth bndr_set weak_fvs (node : nodes) binds
| isDFunId bndr = 9 -- Never choose a DFun as a loop breaker
-- Note [DFuns should not be loop breakers]
- | Just inl_source <- isStableCoreUnfolding_maybe (idUnfolding bndr)
- = case inl_source of
- InlineWrapper -> 10 -- Note [INLINE pragmas]
- _other -> 3 -- Data structures are more important than this
- -- so that dictionary/method recursion unravels
- -- Note that this case hits all InlineRule things, so we
- -- never look at 'rhs' for InlineRule stuff. That's right, because
- -- 'rhs' is irrelevant for inlining things with an InlineRule
+ | Just _ <- isStableCoreUnfolding_maybe (idUnfolding bndr)
+ = 3 -- Note [INLINE pragmas]
+ -- Data structures are more important than INLINE pragmas
+ -- so that dictionary/method recursion unravels
+ -- Note that this case hits all InlineRule things, so we
+ -- never look at 'rhs' for InlineRule stuff. That's right, because
+ -- 'rhs' is irrelevant for inlining things with an InlineRule
| is_con_app rhs = 5 -- Data types help with cases: Note [Constructor applications]
@@ -968,32 +967,37 @@ Avoid choosing a function with an INLINE pramga as the loop breaker!
If such a function is mutually-recursive with a non-INLINE thing,
then the latter should be the loop-breaker.
-Usually this is just a question of optimisation. But a particularly
-bad case is wrappers generated by the demand analyser: if you make
-then into a loop breaker you may get an infinite inlining loop. For
-example:
- rec {
- $wfoo x = ....foo x....
+ ----> Historical note, dating from when strictness wrappers
+ were generated from the strictness signatures:
- {-loop brk-} foo x = ...$wfoo x...
- }
-The interface file sees the unfolding for $wfoo, and sees that foo is
-strict (and hence it gets an auto-generated wrapper). Result: an
-infinite inlining in the importing scope. So be a bit careful if you
-change this. A good example is Tree.repTree in
-nofib/spectral/minimax. If the repTree wrapper is chosen as the loop
-breaker then compiling Game.hs goes into an infinite loop. This
-happened when we gave is_con_app a lower score than inline candidates:
-
- Tree.repTree
- = __inline_me (/\a. \w w1 w2 ->
- case Tree.$wrepTree @ a w w1 w2 of
- { (# ww1, ww2 #) -> Branch @ a ww1 ww2 })
- Tree.$wrepTree
- = /\a w w1 w2 ->
- (# w2_smP, map a (Tree a) (Tree.repTree a w1 w) (w w2) #)
-
-Here we do *not* want to choose 'repTree' as the loop breaker.
+ Usually this is just a question of optimisation. But a particularly
+ bad case is wrappers generated by the demand analyser: if you make
+ then into a loop breaker you may get an infinite inlining loop. For
+ example:
+ rec {
+ $wfoo x = ....foo x....
+
+ {-loop brk-} foo x = ...$wfoo x...
+ }
+ The interface file sees the unfolding for $wfoo, and sees that foo is
+ strict (and hence it gets an auto-generated wrapper). Result: an
+ infinite inlining in the importing scope. So be a bit careful if you
+ change this. A good example is Tree.repTree in
+ nofib/spectral/minimax. If the repTree wrapper is chosen as the loop
+ breaker then compiling Game.hs goes into an infinite loop. This
+ happened when we gave is_con_app a lower score than inline candidates:
+
+ Tree.repTree
+ = __inline_me (/\a. \w w1 w2 ->
+ case Tree.$wrepTree @ a w w1 w2 of
+ { (# ww1, ww2 #) -> Branch @ a ww1 ww2 })
+ Tree.$wrepTree
+ = /\a w w1 w2 ->
+ (# w2_smP, map a (Tree a) (Tree.repTree a w1 w) (w w2) #)
+
+ Here we do *not* want to choose 'repTree' as the loop breaker.
+
+ -----> End of historical note
Note [DFuns should not be loop breakers]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~