diff options
author | Simon Peyton Jones <simonpj@microsoft.com> | 2016-11-28 17:41:27 +0000 |
---|---|---|
committer | Simon Peyton Jones <simonpj@microsoft.com> | 2016-11-28 17:41:27 +0000 |
commit | 3aa936893cac8d7c3b242c882731e9a38a4ae425 (patch) | |
tree | ed01d54b6d4a834e9e693f56b1af8ce2645c95da /compiler/iface | |
parent | 27a6bdf029491a7bbd50377932ae86ede5b5020a (diff) | |
download | haskell-3aa936893cac8d7c3b242c882731e9a38a4ae425.tar.gz |
Comments only (related to #12789)
It took me some time to find the right Note for the
fix to #12789. This comment patch tries to add pointers
from relevant places.
Diffstat (limited to 'compiler/iface')
-rw-r--r-- | compiler/iface/MkIface.hs | 72 | ||||
-rw-r--r-- | compiler/iface/ToIface.hs | 71 |
2 files changed, 71 insertions, 72 deletions
diff --git a/compiler/iface/MkIface.hs b/compiler/iface/MkIface.hs index 9e6fa62af9..4d45efd2b4 100644 --- a/compiler/iface/MkIface.hs +++ b/compiler/iface/MkIface.hs @@ -1769,75 +1769,3 @@ bogusIfaceRule id_name ifRuleBndrs = [], ifRuleHead = id_name, ifRuleArgs = [], ifRuleRhs = IfaceExt id_name, ifRuleOrph = IsOrphan, ifRuleAuto = True } - ---------------------- -{- -Note [Inlining and hs-boot files] -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Consider this example (Trac #10083): - - ---------- RSR.hs-boot ------------ - module RSR where - data RSR - eqRSR :: RSR -> RSR -> Bool - - ---------- SR.hs ------------ - module SR where - import {-# SOURCE #-} RSR - data SR = MkSR RSR - eqSR (MkSR r1) (MkSR r2) = eqRSR r1 r2 - - ---------- RSR.hs ------------ - module RSR where - import SR - data RSR = MkRSR SR -- deriving( Eq ) - eqRSR (MkRSR s1) (MkRSR s2) = (eqSR s1 s2) - foo x y = not (eqRSR x y) - -When compiling RSR we get this code - - RSR.eqRSR :: RSR -> RSR -> Bool - RSR.eqRSR = \ (ds1 :: RSR.RSR) (ds2 :: RSR.RSR) -> - case ds1 of _ { RSR.MkRSR s1 -> - case ds2 of _ { RSR.MkRSR s2 -> - SR.eqSR s1 s2 }} - - RSR.foo :: RSR -> RSR -> Bool - RSR.foo = \ (x :: RSR) (y :: RSR) -> not (RSR.eqRSR x y) - -Now, when optimising foo: - Inline eqRSR (small, non-rec) - Inline eqSR (small, non-rec) -but the result of inlining eqSR from SR is another call to eqRSR, so -everything repeats. Neither eqSR nor eqRSR are (apparently) loop -breakers. - -Solution: in the unfolding of eqSR in SR.hi, replace `eqRSR` in SR -with `noinline eqRSR`, so that eqRSR doesn't get inlined. This means -that when GHC inlines `eqSR`, it will not also inline `eqRSR`, exactly -as would have been the case if `foo` had been defined in SR.hs (and -marked as a loop-breaker). - -But how do we arrange for this to happen? There are two ingredients: - - 1. When we serialize out unfoldings to IfaceExprs (toIfaceVar), - for every variable reference we see if we are referring to an - 'Id' that came from an hs-boot file. If so, we add a `noinline` - to the reference. - - 2. But how do we know if a reference came from an hs-boot file - or not? We could record this directly in the 'IdInfo', but - actually we deduce this by looking at the unfolding: 'Id's - that come from boot files are given a special unfolding - (upon typechecking) 'BootUnfolding' which say that there is - no unfolding, and the reason is because the 'Id' came from - a boot file. - -Here is a solution that doesn't work: when compiling RSR, -add a NOINLINE pragma to every function exported by the boot-file -for RSR (if it exists). Doing so makes the bootstrapped GHC itself -slower by 8% overall (on Trac #9872a-d, and T1969: the reason -is that these NOINLINE'd functions now can't be profitably inlined -outside of the hs-boot loop. - --} diff --git a/compiler/iface/ToIface.hs b/compiler/iface/ToIface.hs index 8de3e3ea9b..8e80bb3d42 100644 --- a/compiler/iface/ToIface.hs +++ b/compiler/iface/ToIface.hs @@ -497,3 +497,74 @@ toIfaceVar v | isExternalName name = IfaceExt name | otherwise = IfaceLcl (getOccFS name) where name = idName v + + +{- Note [Inlining and hs-boot files] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Consider this example (Trac #10083, #12789): + + ---------- RSR.hs-boot ------------ + module RSR where + data RSR + eqRSR :: RSR -> RSR -> Bool + + ---------- SR.hs ------------ + module SR where + import {-# SOURCE #-} RSR + data SR = MkSR RSR + eqSR (MkSR r1) (MkSR r2) = eqRSR r1 r2 + + ---------- RSR.hs ------------ + module RSR where + import SR + data RSR = MkRSR SR -- deriving( Eq ) + eqRSR (MkRSR s1) (MkRSR s2) = (eqSR s1 s2) + foo x y = not (eqRSR x y) + +When compiling RSR we get this code + + RSR.eqRSR :: RSR -> RSR -> Bool + RSR.eqRSR = \ (ds1 :: RSR.RSR) (ds2 :: RSR.RSR) -> + case ds1 of _ { RSR.MkRSR s1 -> + case ds2 of _ { RSR.MkRSR s2 -> + SR.eqSR s1 s2 }} + + RSR.foo :: RSR -> RSR -> Bool + RSR.foo = \ (x :: RSR) (y :: RSR) -> not (RSR.eqRSR x y) + +Now, when optimising foo: + Inline eqRSR (small, non-rec) + Inline eqSR (small, non-rec) +but the result of inlining eqSR from SR is another call to eqRSR, so +everything repeats. Neither eqSR nor eqRSR are (apparently) loop +breakers. + +Solution: in the unfolding of eqSR in SR.hi, replace `eqRSR` in SR +with `noinline eqRSR`, so that eqRSR doesn't get inlined. This means +that when GHC inlines `eqSR`, it will not also inline `eqRSR`, exactly +as would have been the case if `foo` had been defined in SR.hs (and +marked as a loop-breaker). + +But how do we arrange for this to happen? There are two ingredients: + + 1. When we serialize out unfoldings to IfaceExprs (toIfaceVar), + for every variable reference we see if we are referring to an + 'Id' that came from an hs-boot file. If so, we add a `noinline` + to the reference. + + 2. But how do we know if a reference came from an hs-boot file + or not? We could record this directly in the 'IdInfo', but + actually we deduce this by looking at the unfolding: 'Id's + that come from boot files are given a special unfolding + (upon typechecking) 'BootUnfolding' which say that there is + no unfolding, and the reason is because the 'Id' came from + a boot file. + +Here is a solution that doesn't work: when compiling RSR, +add a NOINLINE pragma to every function exported by the boot-file +for RSR (if it exists). Doing so makes the bootstrapped GHC itself +slower by 8% overall (on Trac #9872a-d, and T1969: the reason +is that these NOINLINE'd functions now can't be profitably inlined +outside of the hs-boot loop. + +-} |