Try harder not to make DFuns into loop breakers

[ghc-hetmet.git] / compiler / simplCore / OccurAnal.lhs

diff --git a/compiler/simplCore/OccurAnal.lhs b/compiler/simplCore/OccurAnal.lhs
index a3e3732..8cef0fc 100644 (file)
--- a/compiler/simplCore/OccurAnal.lhs
+++ b/compiler/simplCore/OccurAnal.lhs
@@ -177,7 +177,7 @@ However things are made quite a bit more complicated by RULES.  Remember
     "loop"?  In particular, a RULE is like an equation for 'f' that
     is *always* inlined if it is applicable.  We do *not* disable
     rules for loop-breakers.  It's up to whoever makes the rules to
-    make sure that the rules themselves alwasys terminate.  See Note
+    make sure that the rules themselves always terminate.  See Note
     [Rules for recursive functions] in Simplify.lhs
 
     Hence, if
@@ -526,23 +526,28 @@ reOrderCycle depth (bind : binds) pairs
 
     score :: Node Details -> Int        -- Higher score => less likely to be picked as loop breaker
     score (ND bndr rhs _ _, _, _)
-        | exprIsTrivial rhs        = 10  -- Practically certain to be inlined
-                -- Used to have also: && not (isExportedId bndr)
-                -- But I found this sometimes cost an extra iteration when we have
-                --      rec { d = (a,b); a = ...df...; b = ...df...; df = d }
-                -- where df is the exported dictionary. Then df makes a really
-                -- bad choice for loop breaker
+        | isDFunId bndr = 9   -- Never choose a DFun as a loop breaker
+                             -- Note [DFuns should not be loop breakers]
 
         | Just (inl_rule_info, _) <- isInlineRule_maybe (idUnfolding bndr)
        = case inl_rule_info of
             InlWrapper {} -> 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
                 
-        | is_con_app rhs = 5    -- Data types help with cases
-                               -- Includes dict funs
-                -- Note [Constructor applictions]
+        | is_con_app rhs = 5  -- Data types help with cases: Note [Constructor applications]
+                
+        | exprIsTrivial rhs = 10  -- Practically certain to be inlined
+                -- Used to have also: && not (isExportedId bndr)
+                -- But I found this sometimes cost an extra iteration when we have
+                --      rec { d = (a,b); a = ...df...; b = ...df...; df = d }
+                -- where df is the exported dictionary. Then df makes a really
+                -- bad choice for loop breaker
 
+       
 -- If an Id is marked "never inline" then it makes a great loop breaker
 -- The only reason for not checking that here is that it is rare
 -- and I've never seen a situation where it makes a difference,
@@ -555,8 +560,6 @@ reOrderCycle depth (bind : binds) pairs
                 -- the Id has some kind of unfolding
 
         | otherwise = 0
-        where
-         
 
        -- Checking for a constructor application
         -- Cheap and cheerful; the simplifer moves casts out of the way
@@ -615,13 +618,14 @@ linear in the number of instance declarations.
 
 Note [INLINE pragmas]
 ~~~~~~~~~~~~~~~~~~~~~
-Never choose a function with an INLINE pramga as the loop breaker!  
+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.
 
-A particular 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:
+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....
 
@@ -645,6 +649,24 @@ happened when we gave is_con_app a lower score than inline candidates:
 
 Here we do *not* want to choose 'repTree' as the loop breaker.
 
+Note [DFuns should not be loop breakers]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+It's particularly bad to make a DFun into a loop breaker.  See
+Note [How instance declarations are translated] in TcInstDcls
+
+We give DFuns a higher score than ordinary CONLIKE things because 
+if there's a choice we want the DFun to be the non-looop breker. Eg
+ 
+rec { sc = /\ a \$dC. $fBWrap (T a) ($fCT @ a $dC)
+
+      $fCT :: forall a_afE. (Roman.C a_afE) => Roman.C (Roman.T a_afE)
+      {-# DFUN #-}
+      $fCT = /\a \$dC. MkD (T a) ((sc @ a $dC) |> blah) ($ctoF @ a $dC)
+    }
+
+Here 'sc' (the superclass) looks CONLIKE, but we'll never get to it
+if we can't unravel the DFun first.
+
 Note [Constructor applications]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 It's really really important to inline dictionaries.  Real