Commit e4a1d2d0 authored by Simon Peyton Jones's avatar Simon Peyton Jones

Remove the final vestiges of InlineWrappers

Part of Nick Frisby's patch (c080f727)
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.
parent 5f98d44d
......@@ -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
......
......@@ -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)
......
......@@ -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>")
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment