diff options
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/coreSyn/CoreSyn.lhs | 31 | ||||
| -rw-r--r-- | compiler/coreSyn/CoreUnfold.lhs | 2 | ||||
| -rw-r--r-- | compiler/coreSyn/PprCore.lhs | 1 | ||||
| -rw-r--r-- | compiler/iface/IfaceSyn.lhs | 15 | ||||
| -rw-r--r-- | compiler/iface/MkIface.lhs | 1 | ||||
| -rw-r--r-- | compiler/iface/TcIface.lhs | 29 | ||||
| -rw-r--r-- | compiler/simplCore/OccurAnal.lhs | 70 |
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] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |