Commit 3aa93689 authored by Simon Peyton Jones's avatar Simon Peyton Jones

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.
parent 27a6bdf0
......@@ -980,8 +980,8 @@ data Unfolding
| BootUnfolding -- ^ We have no information about the unfolding, because
-- this 'Id' came from an @hi-boot@ file.
-- See Note [Inlining and hs-boot files] for what
-- this is used for.
-- See Note [Inlining and hs-boot files] in ToIface
-- for what this is used for.
| OtherCon [AltCon] -- ^ It ain't one of these constructors.
-- @OtherCon xs@ also indicates that something has been evaluated
......
......@@ -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.
-}
......@@ -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.
-}
......@@ -276,6 +276,9 @@ Loop breaking is surprisingly subtle. First read the section 4 of
We avoid infinite inlinings by choosing loop breakers, and
ensuring that a loop breaker cuts each loop.
See also Note [Inlining and hs-boot files] in ToIface, which deals
with a closely related source of infinite loops.
Fundamentally, we do SCC analysis on a graph. For each recursive
group we choose a loop breaker, delete all edges to that node,
re-analyse the SCC, and iterate.
......
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