Don't look through SCC in exprIsConApp_maybe
[ghc-hetmet.git] / compiler / coreSyn / CoreUnfold.lhs
index 0510e90..e54acc0 100644 (file)
@@ -1,4 +1,4 @@
-calcU%
+%
 % (c) The University of Glasgow 2006
 % (c) The AQUA Project, Glasgow University, 1994-1998
 %
@@ -19,8 +19,9 @@ module CoreUnfold (
        Unfolding, UnfoldingGuidance,   -- Abstract types
 
        noUnfolding, mkImplicitUnfolding, 
-       mkTopUnfolding, mkUnfolding, mkCoreUnfolding,
-       mkInlineRule, mkWwInlineRule,
+        mkUnfolding, mkCoreUnfolding,
+       mkTopUnfolding, mkSimpleUnfolding,
+       mkInlineUnfolding, mkInlinableUnfolding, mkWwInlineRule,
        mkCompulsoryUnfolding, mkDFunUnfolding,
 
        interestingArg, ArgSummary(..),
@@ -40,9 +41,11 @@ import StaticFlags
 import DynFlags
 import CoreSyn
 import PprCore         ()      -- Instances
+import TcType          ( tcSplitSigmaTy, tcSplitDFunHead )
 import OccurAnal
 import CoreSubst hiding( substTy )
 import CoreFVs         ( exprFreeVars )
+import CoreArity       ( manifestArity, exprBotStrictness_maybe )
 import CoreUtils
 import Id
 import DataCon
@@ -61,7 +64,7 @@ import Util
 import FastTypes
 import FastString
 import Outputable
-
+import Data.Maybe
 \end{code}
 
 
@@ -72,12 +75,12 @@ import Outputable
 %************************************************************************
 
 \begin{code}
-mkTopUnfolding :: CoreExpr -> Unfolding
-mkTopUnfolding expr = mkUnfolding True {- Top level -} expr
+mkTopUnfolding :: Bool -> CoreExpr -> Unfolding
+mkTopUnfolding = mkUnfolding InlineRhs True {- Top level -}
 
 mkImplicitUnfolding :: CoreExpr -> Unfolding
 -- For implicit Ids, do a tiny bit of optimising first
-mkImplicitUnfolding expr = mkTopUnfolding (simpleOptExpr expr)
+mkImplicitUnfolding expr = mkTopUnfolding False (simpleOptExpr expr) 
 
 -- Note [Top-level flag on inline rules]
 -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -85,73 +88,103 @@ mkImplicitUnfolding expr = mkTopUnfolding (simpleOptExpr expr)
 -- top-level flag to True.  It gets set more accurately by the simplifier
 -- Simplify.simplUnfolding.
 
-mkUnfolding :: Bool -> CoreExpr -> Unfolding
-mkUnfolding top_lvl expr
-  = CoreUnfolding { uf_tmpl      = occurAnalyseExpr expr,
-                   uf_src        = InlineRhs,
-                   uf_arity      = arity,
-                   uf_is_top     = top_lvl,
-                   uf_is_value   = exprIsHNF        expr,
-                    uf_is_conlike = exprIsConLike    expr,
-                   uf_expandable = exprIsExpandable expr,
-                   uf_is_cheap   = is_cheap,
-                   uf_guidance   = guidance }
-  where
-    is_cheap = exprIsCheap expr
-    (arity, guidance) = calcUnfoldingGuidance is_cheap opt_UF_CreationThreshold expr
-       -- Sometimes during simplification, there's a large let-bound thing     
-       -- which has been substituted, and so is now dead; so 'expr' contains
-       -- two copies of the thing while the occurrence-analysed expression doesn't
-       -- Nevertheless, we *don't* occ-analyse before computing the size because the
-       -- size computation bales out after a while, whereas occurrence analysis does not.
-       --
-       -- This can occasionally mean that the guidance is very pessimistic;
-       -- it gets fixed up next round.  And it should be rare, because large
-       -- let-bound things that are dead are usually caught by preInlineUnconditionally
+mkSimpleUnfolding :: CoreExpr -> Unfolding
+mkSimpleUnfolding = mkUnfolding InlineRhs False False
 
-mkCoreUnfolding :: Bool -> UnfoldingSource -> CoreExpr
-                -> Arity -> UnfoldingGuidance -> Unfolding
--- Occurrence-analyses the expression before capturing it
-mkCoreUnfolding top_lvl src expr arity guidance 
-  = CoreUnfolding { uf_tmpl      = occurAnalyseExpr expr,
-                   uf_src        = src,
-                   uf_arity      = arity,
-                   uf_is_top     = top_lvl,
-                   uf_is_value   = exprIsHNF        expr,
-                    uf_is_conlike = exprIsConLike    expr,
-                   uf_is_cheap   = exprIsCheap      expr,
-                   uf_expandable = exprIsExpandable expr,
-                   uf_guidance   = guidance }
-
-mkDFunUnfolding :: DataCon -> [Id] -> Unfolding
-mkDFunUnfolding con ops = DFunUnfolding con (map Var ops)
+mkDFunUnfolding :: Type -> [CoreExpr] -> Unfolding
+mkDFunUnfolding dfun_ty ops 
+  = DFunUnfolding dfun_nargs data_con ops
+  where
+    (tvs, theta, head_ty) = tcSplitSigmaTy dfun_ty
+         -- NB: tcSplitSigmaTy: do not look through a newtype
+         --     when the dictionary type is a newtype
+    (cls, _)   = tcSplitDFunHead head_ty
+    dfun_nargs = length tvs + length theta
+    data_con   = classDataCon cls
 
 mkWwInlineRule :: Id -> CoreExpr -> Arity -> Unfolding
 mkWwInlineRule id expr arity
-  = mkCoreUnfolding True (InlineWrapper id) 
+  = mkCoreUnfolding (InlineWrapper id) True
                    (simpleOptExpr expr) arity
                    (UnfWhen unSaturatedOk boringCxtNotOk)
 
 mkCompulsoryUnfolding :: CoreExpr -> Unfolding
 mkCompulsoryUnfolding expr        -- Used for things that absolutely must be unfolded
-  = mkCoreUnfolding True InlineCompulsory
+  = mkCoreUnfolding InlineCompulsory True
                     expr 0    -- Arity of unfolding doesn't matter
                     (UnfWhen unSaturatedOk boringCxtOk)
 
-mkInlineRule :: Bool -> CoreExpr -> Arity -> Unfolding
-mkInlineRule unsat_ok expr arity 
-  = mkCoreUnfolding True InlineRule     -- Note [Top-level flag on inline rules]
-                   expr' arity 
+mkInlineUnfolding :: Maybe Arity -> CoreExpr -> Unfolding
+mkInlineUnfolding mb_arity expr 
+  = mkCoreUnfolding InlineStable
+                   True         -- Note [Top-level flag on inline rules]
+                    expr' arity 
                    (UnfWhen unsat_ok boring_ok)
   where
     expr' = simpleOptExpr expr
+    (unsat_ok, arity) = case mb_arity of
+                          Nothing -> (unSaturatedOk, manifestArity expr')
+                          Just ar -> (needSaturated, ar)
+              
     boring_ok = case calcUnfoldingGuidance True    -- Treat as cheap
+                                          False   -- But not bottoming
                                            (arity+1) expr' of
                  (_, UnfWhen _ boring_ok) -> boring_ok
                  _other                   -> boringCxtNotOk
      -- See Note [INLINE for small functions]
+
+mkInlinableUnfolding :: CoreExpr -> Unfolding
+mkInlinableUnfolding expr
+  = mkUnfolding InlineStable True is_bot expr'
+  where
+    expr' = simpleOptExpr expr
+    is_bot = isJust (exprBotStrictness_maybe expr')
 \end{code}
 
+Internal functions
+
+\begin{code}
+mkCoreUnfolding :: UnfoldingSource -> Bool -> CoreExpr
+                -> Arity -> UnfoldingGuidance -> Unfolding
+-- Occurrence-analyses the expression before capturing it
+mkCoreUnfolding src top_lvl expr arity guidance 
+  = CoreUnfolding { uf_tmpl      = occurAnalyseExpr expr,
+                   uf_src        = src,
+                   uf_arity      = arity,
+                   uf_is_top     = top_lvl,
+                   uf_is_value   = exprIsHNF        expr,
+                    uf_is_conlike = exprIsConLike    expr,
+                   uf_is_cheap   = exprIsCheap      expr,
+                   uf_expandable = exprIsExpandable expr,
+                   uf_guidance   = guidance }
+
+mkUnfolding :: UnfoldingSource -> Bool -> Bool -> CoreExpr -> Unfolding
+-- Calculates unfolding guidance
+-- Occurrence-analyses the expression before capturing it
+mkUnfolding src top_lvl is_bottoming expr
+  = CoreUnfolding { uf_tmpl      = occurAnalyseExpr expr,
+                   uf_src        = src,
+                   uf_arity      = arity,
+                   uf_is_top     = top_lvl,
+                   uf_is_value   = exprIsHNF        expr,
+                    uf_is_conlike = exprIsConLike    expr,
+                   uf_expandable = exprIsExpandable expr,
+                   uf_is_cheap   = is_cheap,
+                   uf_guidance   = guidance }
+  where
+    is_cheap = exprIsCheap expr
+    (arity, guidance) = calcUnfoldingGuidance is_cheap (top_lvl && is_bottoming) 
+                                              opt_UF_CreationThreshold expr
+       -- Sometimes during simplification, there's a large let-bound thing     
+       -- which has been substituted, and so is now dead; so 'expr' contains
+       -- two copies of the thing while the occurrence-analysed expression doesn't
+       -- Nevertheless, we *don't* occ-analyse before computing the size because the
+       -- size computation bales out after a while, whereas occurrence analysis does not.
+       --
+       -- This can occasionally mean that the guidance is very pessimistic;
+       -- it gets fixed up next round.  And it should be rare, because large
+       -- let-bound things that are dead are usually caught by preInlineUnconditionally
+\end{code}
 
 %************************************************************************
 %*                                                                     *
@@ -163,10 +196,12 @@ mkInlineRule unsat_ok expr arity
 calcUnfoldingGuidance
        :: Bool         -- True <=> the rhs is cheap, or we want to treat it
                        --          as cheap (INLINE things)     
+        -> Bool                -- True <=> this is a top-level unfolding for a
+                       --          diverging function; don't inline this
         -> Int         -- Bomb out if size gets bigger than this
        -> CoreExpr     -- Expression to look at
        -> (Arity, UnfoldingGuidance)
-calcUnfoldingGuidance expr_is_cheap bOMB_OUT_SIZE expr
+calcUnfoldingGuidance expr_is_cheap top_bot bOMB_OUT_SIZE expr
   = case collectBinders expr of { (bndrs, body) ->
     let
         val_bndrs   = filter isId bndrs
@@ -176,8 +211,11 @@ calcUnfoldingGuidance expr_is_cheap bOMB_OUT_SIZE expr
           = case (sizeExpr (iUnbox bOMB_OUT_SIZE) val_bndrs body) of
              TooBig -> UnfNever
              SizeIs size cased_bndrs scrut_discount
-               | uncondInline n_val_bndrs (iBox size) && expr_is_cheap
-               -> UnfWhen needSaturated boringCxtOk
+               | uncondInline n_val_bndrs (iBox size)
+                , expr_is_cheap
+               -> UnfWhen unSaturatedOk boringCxtOk   -- Note [INLINE for small functions]
+               | top_bot  -- See Note [Do not inline top-level bottoming functions]
+               -> UnfNever
 
                | otherwise
                -> UnfIfGoodArgs { ug_args  = map (discount cased_bndrs) val_bndrs
@@ -222,24 +260,61 @@ Notice that 'x' counts 0, while (f x) counts 2.  That's deliberate: there's
 a function call to account for.  Notice also that constructor applications 
 are very cheap, because exposing them to a caller is so valuable.
 
-Note [Unconditional inlining]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-We inline *unconditionally* if inlined thing is smaller (using sizeExpr)
-than the thing it's replacing.  Notice that
+
+Note [Do not inline top-level bottoming functions]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The FloatOut pass has gone to some trouble to float out calls to 'error' 
+and similar friends.  See Note [Bottoming floats] in SetLevels.
+Do not re-inline them!  But we *do* still inline if they are very small
+(the uncondInline stuff).
+
+
+Note [INLINE for small functions]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Consider       {-# INLINE f #-}
+                f x = Just x
+                g y = f y
+Then f's RHS is no larger than its LHS, so we should inline it into
+even the most boring context.  In general, f the function is
+sufficiently small that its body is as small as the call itself, the
+inline unconditionally, regardless of how boring the context is.
+
+Things to note:
+
+ * We inline *unconditionally* if inlined thing is smaller (using sizeExpr)
+   than the thing it's replacing.  Notice that
       (f x) --> (g 3)            -- YES, unconditionally
       (f x) --> x : []           -- YES, *even though* there are two
                                  --      arguments to the cons
       x     --> g 3              -- NO
       x            --> Just v            -- NO
 
-It's very important not to unconditionally replace a variable by
-a non-atomic term.
+  It's very important not to unconditionally replace a variable by
+  a non-atomic term.
+
+* We do this even if the thing isn't saturated, else we end up with the
+  silly situation that
+     f x y = x
+     ...map (f 3)...
+  doesn't inline.  Even in a boring context, inlining without being
+  saturated will give a lambda instead of a PAP, and will be more
+  efficient at runtime.
+
+* However, when the function's arity > 0, we do insist that it 
+  has at least one value argument at the call site.  Otherwise we find this:
+       f = /\a \x:a. x
+       d = /\b. MkD (f b)
+  If we inline f here we get
+       d = /\b. MkD (\x:b. x)
+  and then prepareRhs floats out the argument, abstracting the type
+  variables, so we end up with the original again!
+
 
 \begin{code}
 uncondInline :: Arity -> Int -> Bool
 -- Inline unconditionally if there no size increase
 -- Size of call is arity (+1 for the function)
--- See Note [Unconditional inlining]
+-- See Note [INLINE for small functions]
 uncondInline arity size 
   | arity == 0 = size == 0
   | otherwise  = size <= arity + 1
@@ -266,29 +341,27 @@ sizeExpr bOMB_OUT_SIZE top_args expr
                                            -- discounts even on nullary constructors
 
     size_up (App fun (Type _)) = size_up fun
-    size_up (App fun arg)      = size_up_app fun [arg]
-                                 `addSize` nukeScrutDiscount (size_up arg)
+    size_up (App fun arg)      = size_up arg  `addSizeNSD`
+                                 size_up_app fun [arg]
 
     size_up (Lam b e) | isId b    = lamScrutDiscount (size_up e `addSizeN` 1)
                      | otherwise = size_up e
 
     size_up (Let (NonRec binder rhs) body)
-      = nukeScrutDiscount (size_up rhs)                `addSize`
-       size_up body                            `addSizeN`
+      = size_up rhs            `addSizeNSD`
+       size_up body            `addSizeN`
        (if isUnLiftedType (idType binder) then 0 else 1)
                -- For the allocation
                -- If the binder has an unlifted type there is no allocation
 
     size_up (Let (Rec pairs) body)
-      = nukeScrutDiscount rhs_size             `addSize`
-       size_up body                            `addSizeN`
-       length pairs            -- For the allocation
-      where
-       rhs_size = foldr (addSize . size_up . snd) sizeZero pairs
+      = foldr (addSizeNSD . size_up . snd) 
+              (size_up body `addSizeN` length pairs)   -- (length pairs) for the allocation
+              pairs
 
     size_up (Case (Var v) _ _ alts) 
        | v `elem` top_args             -- We are scrutinising an argument variable
-       = alts_size (foldr1 addSize alt_sizes)  -- The 1 is for the case itself
+       = alts_size (foldr1 addAltSize alt_sizes)
                    (foldr1 maxSize alt_sizes)
                -- Good to inline if an arg is scrutinised, because
                -- that may eliminate allocation in the caller
@@ -298,9 +371,9 @@ sizeExpr bOMB_OUT_SIZE top_args expr
 
                -- alts_size tries to compute a good discount for
                -- the case when we are scrutinising an argument variable
-         alts_size (SizeIs tot tot_disc _tot_scrut)           -- Size of all alternatives
-                   (SizeIs max _max_disc  max_scrut)           -- Size of biggest alternative
-               = SizeIs tot (unitBag (v, iBox (_ILIT(2) +# tot -# max)) `unionBags` tot_disc) max_scrut
+         alts_size (SizeIs tot tot_disc tot_scrut)  -- Size of all alternatives
+                   (SizeIs max _        _)          -- Size of biggest alternative
+               = SizeIs tot (unitBag (v, iBox (_ILIT(2) +# tot -# max)) `unionBags` tot_disc) tot_scrut
                        -- If the variable is known, we produce a discount that
                        -- will take us back to 'max', the size of the largest alternative
                        -- The 1+ is a little discount for reduced allocation in the caller
@@ -310,9 +383,8 @@ sizeExpr bOMB_OUT_SIZE top_args expr
 
          alts_size tot_size _ = tot_size
 
-    size_up (Case e _ _ alts) = foldr (addSize . size_up_alt) 
-                                     (nukeScrutDiscount (size_up e))
-                                     alts
+    size_up (Case e _ _ alts) = size_up e  `addSizeNSD` 
+                                foldr (addAltSize . size_up_alt) sizeZero alts
                -- We don't charge for the case itself
                -- It's a strict thing, and the price of the call
                -- is paid by scrut.  Also consider
@@ -325,8 +397,8 @@ sizeExpr bOMB_OUT_SIZE top_args expr
     -- size_up_app is used when there's ONE OR MORE value args
     size_up_app (App fun arg) args 
        | isTypeArg arg            = size_up_app fun args
-       | otherwise                = size_up_app fun (arg:args)
-                                    `addSize` nukeScrutDiscount (size_up arg)
+       | otherwise                = size_up arg  `addSizeNSD`
+                                     size_up_app fun (arg:args)
     size_up_app (Var fun)     args = size_up_call fun args
     size_up_app other         args = size_up other `addSizeN` length args
 
@@ -355,10 +427,22 @@ sizeExpr bOMB_OUT_SIZE top_args expr
     addSizeN TooBig          _  = TooBig
     addSizeN (SizeIs n xs d) m         = mkSizeIs bOMB_OUT_SIZE (n +# iUnbox m) xs d
     
-    addSize TooBig           _                 = TooBig
-    addSize _                TooBig            = TooBig
-    addSize (SizeIs n1 xs d1) (SizeIs n2 ys d2) 
-       = mkSizeIs bOMB_OUT_SIZE (n1 +# n2) (xs `unionBags` ys) (d1 +# d2)
+        -- addAltSize is used to add the sizes of case alternatives
+    addAltSize TooBig           _      = TooBig
+    addAltSize _                TooBig = TooBig
+    addAltSize (SizeIs n1 xs d1) (SizeIs n2 ys d2) 
+       = mkSizeIs bOMB_OUT_SIZE (n1 +# n2) 
+                                 (xs `unionBags` ys) 
+                                 (d1 +# d2)   -- Note [addAltSize result discounts]
+
+        -- This variant ignores the result discount from its LEFT argument
+       -- It's used when the second argument isn't part of the result
+    addSizeNSD TooBig           _      = TooBig
+    addSizeNSD _                TooBig = TooBig
+    addSizeNSD (SizeIs n1 xs _) (SizeIs n2 ys d2) 
+       = mkSizeIs bOMB_OUT_SIZE (n1 +# n2) 
+                                 (xs `unionBags` ys) 
+                                 d2  -- Ignore d1
 \end{code}
 
 \begin{code}
@@ -418,21 +502,44 @@ funSize top_args fun n_val_args
 
 conSize :: DataCon -> Int -> ExprSize
 conSize dc n_val_args
-  | n_val_args == 0      = SizeIs (_ILIT(0)) emptyBag (_ILIT(1))       -- Like variables
+  | n_val_args == 0 = SizeIs (_ILIT(0)) emptyBag (_ILIT(1))    -- Like variables
+
+-- See Note [Constructor size]
   | isUnboxedTupleCon dc = SizeIs (_ILIT(0)) emptyBag (iUnbox n_val_args +# _ILIT(1))
-  | otherwise           = SizeIs (_ILIT(1)) emptyBag (iUnbox n_val_args +# _ILIT(1))
-       -- Treat a constructors application as size 1, regardless of how
-       -- many arguments it has; we are keen to expose them
-       -- (and we charge separately for their args).  We can't treat
-       -- them as size zero, else we find that (Just x) has size 0,
-       -- which is the same as a lone variable; and hence 'v' will 
-       -- always be replaced by (Just x), where v is bound to Just x.
-       --
-       -- However, unboxed tuples count as size zero
-       -- I found occasions where we had 
-       --      f x y z = case op# x y z of { s -> (# s, () #) }
-       -- and f wasn't getting inlined
 
+-- See Note [Unboxed tuple result discount]
+--  | isUnboxedTupleCon dc = SizeIs (_ILIT(0)) emptyBag (_ILIT(0))
+
+-- See Note [Constructor size]
+  | otherwise = SizeIs (_ILIT(1)) emptyBag (iUnbox n_val_args +# _ILIT(1))
+\end{code}
+
+Note [Constructor size]
+~~~~~~~~~~~~~~~~~~~~~~~
+Treat a constructors application as size 1, regardless of how many
+arguments it has; we are keen to expose them (and we charge separately
+for their args).  We can't treat them as size zero, else we find that
+(Just x) has size 0, which is the same as a lone variable; and hence
+'v' will always be replaced by (Just x), where v is bound to Just x.
+
+However, unboxed tuples count as size zero. I found occasions where we had 
+       f x y z = case op# x y z of { s -> (# s, () #) }
+and f wasn't getting inlined.
+
+Note [Unboxed tuple result discount]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+I tried giving unboxed tuples a *result discount* of zero (see the
+commented-out line).  Why?  When returned as a result they do not
+allocate, so maybe we don't want to charge so much for them If you
+have a non-zero discount here, we find that workers often get inlined
+back into wrappers, because it look like
+    f x = case $wf x of (# a,b #) -> (a,b)
+and we are keener because of the case.  However while this change
+shrank binary sizes by 0.5% it also made spectral/boyer allocate 5%
+more. All other changes were very small. So it's not a big deal but I
+didn't adopt the idea.
+
+\begin{code}
 primOpSize :: PrimOp -> Int -> ExprSize
 primOpSize op n_val_args
  | not (primOpIsDupable op) = sizeN opt_UF_DearOp
@@ -464,16 +571,21 @@ augmentSize = SizeIs (_ILIT(0)) emptyBag (_ILIT(4))
        -- Ditto (augment t (\cn -> e) ys) should cost only the cost of
        -- e plus ys. The -2 accounts for the \cn 
 
-nukeScrutDiscount :: ExprSize -> ExprSize
-nukeScrutDiscount (SizeIs n vs _) = SizeIs n vs (_ILIT(0))
-nukeScrutDiscount TooBig          = TooBig
-
 -- When we return a lambda, give a discount if it's used (applied)
 lamScrutDiscount :: ExprSize -> ExprSize
 lamScrutDiscount (SizeIs n vs _) = SizeIs n vs (iUnbox opt_UF_FunAppDiscount)
 lamScrutDiscount TooBig          = TooBig
 \end{code}
 
+Note [addAltSize result discounts]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+When adding the size of alternatives, we *add* the result discounts
+too, rather than take the *maximum*.  For a multi-branch case, this
+gives a discount for each branch that returns a constructor, making us
+keener to inline.  I did try using 'max' instead, but it makes nofib 
+'rewrite' and 'puzzle' allocate significantly more, and didn't make
+binary sizes shrink significantly either.
+
 Note [Discounts and thresholds]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Constants for discounts and thesholds are defined in main/StaticFlags,
@@ -566,9 +678,11 @@ actual arguments.
 \begin{code}
 couldBeSmallEnoughToInline :: Int -> CoreExpr -> Bool
 couldBeSmallEnoughToInline threshold rhs 
-  = case calcUnfoldingGuidance False threshold rhs of
-       (_, UnfNever) -> False
-       _             -> True
+  = case sizeExpr (iUnbox threshold) [] body of
+       TooBig -> False
+       _      -> True
+  where
+    (_, body) = collectBinders rhs
 
 ----------------
 smallEnoughToInline :: Unfolding -> Bool
@@ -656,7 +770,7 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
        NoUnfolding      -> Nothing ;
        OtherCon _       -> Nothing ;
        DFunUnfolding {} -> Nothing ;   -- Never unfold a DFun
-       CoreUnfolding { uf_tmpl = unf_template, uf_is_top = is_top, uf_is_value = is_value,
+       CoreUnfolding { uf_tmpl = unf_template, uf_is_top = is_top, 
                        uf_is_cheap = is_cheap, uf_arity = uf_arity, uf_guidance = guidance } ->
                        -- uf_arity will typically be equal to (idArity id), 
                        -- but may be less for InlineRules
@@ -686,19 +800,19 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
 
        interesting_saturated_call 
          = case cont_info of
-             BoringCtxt -> not is_top && uf_arity > 0          -- Note [Nested functions]
-             CaseCtxt   -> not (lone_variable && is_value)     -- Note [Lone variables]
-             ArgCtxt {} -> uf_arity > 0                        -- Note [Inlining in ArgCtxt]
-             ValAppCtxt -> True                                -- Note [Cast then apply]
+             BoringCtxt -> not is_top && uf_arity > 0        -- Note [Nested functions]
+             CaseCtxt   -> not (lone_variable && is_cheap)   -- Note [Lone variables]
+             ArgCtxt {} -> uf_arity > 0                      -- Note [Inlining in ArgCtxt]
+             ValAppCtxt -> True                              -- Note [Cast then apply]
 
        (yes_or_no, extra_doc)
          = case guidance of
              UnfNever -> (False, empty)
 
-             UnfWhen unsat_ok boring_ok -> ( (unsat_ok  || saturated)
-                                           && (boring_ok || some_benefit)
-                                            , empty )
-                  -- For the boring_ok part see Note [INLINE for small functions]
+             UnfWhen unsat_ok boring_ok 
+                 -> (enough_args && (boring_ok || some_benefit), empty )
+                 where      -- See Note [INLINE for small functions]
+                   enough_args = saturated || (unsat_ok && n_val_args > 0)
 
              UnfIfGoodArgs { ug_args = arg_discounts, ug_res = res_discount, ug_size = size }
                 -> ( is_cheap && some_benefit && small_enough
@@ -710,13 +824,12 @@ callSiteInline dflags id unfolding lone_variable arg_infos cont_info
                                              res_discount arg_infos cont_info
                
     in    
-    if dopt Opt_D_dump_inlinings dflags then
+    if (dopt Opt_D_dump_inlinings dflags && dopt Opt_D_verbose_core2core dflags) then
        pprTrace ("Considering inlining: " ++ showSDoc (ppr id))
                 (vcat [text "arg infos" <+> ppr arg_infos,
                        text "uf arity" <+> ppr uf_arity,
                        text "interesting continuation" <+> ppr cont_info,
                        text "some_benefit" <+> ppr some_benefit,
-                       text "is value:" <+> ppr is_value,
                         text "is cheap:" <+> ppr is_cheap,
                        text "guidance" <+> ppr guidance,
                        extra_doc,
@@ -758,22 +871,12 @@ But the defn of GHC.Classes.$dmmin is:
     {- Arity: 3, HasNoCafRefs, Strictness: SLL,
        Unfolding: (\ @ a $dOrd :: GHC.Classes.Ord a x :: a y :: a ->
                    case @ a GHC.Classes.<= @ a $dOrd x y of wild {
-                     GHC.Bool.False -> y GHC.Bool.True -> x }) -}
+                     GHC.Types.False -> y GHC.Types.True -> x }) -}
 
 We *really* want to inline $dmmin, even though it has arity 3, in
 order to unravel the recursion.
 
 
-Note [INLINE for small functions]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Consider       {-# INLINE f #-}
-                f x = Just x
-                g y = f y
-Then f's RHS is no larger than its LHS, so we should inline it
-into even the most boring context.  (We do so if there is no INLINE
-pragma!)  
-
-
 Note [Things to watch]
 ~~~~~~~~~~~~~~~~~~~~~~
 *   { y = I# 3; x = y `cast` co; ...case (x `cast` co) of ... }
@@ -840,8 +943,8 @@ call is at least CONLIKE.  At least for the cases where we use ArgCtxt
 for the RHS of a 'let', we only profit from the inlining if we get a 
 CONLIKE thing (modulo lets).
 
-Note [Lone variables]
-~~~~~~~~~~~~~~~~~~~~~
+Note [Lone variables]  See also Note [Interaction of exprIsCheap and lone variables]
+~~~~~~~~~~~~~~~~~~~~~   which appears below
 The "lone-variable" case is important.  I spent ages messing about
 with unsatisfactory varaints, but this is nice.  The idea is that if a
 variable appears all alone
@@ -850,7 +953,7 @@ variable appears all alone
        as scrutinee of a case          CaseCtxt
        as arg of a fn                  ArgCtxt
 AND
-       it is bound to a value
+       it is bound to a cheap expression
 
 then we should not inline it (unless there is some other reason,
 e.g. is is the sole occurrence).  That is what is happening at 
@@ -902,6 +1005,27 @@ However, watch out:
    There's no advantage in inlining f here, and perhaps
    a significant disadvantage.  Hence some_val_args in the Stop case
 
+Note [Interaction of exprIsCheap and lone variables]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The lone-variable test says "don't inline if a case expression
+scrutines a lone variable whose unfolding is cheap".  It's very 
+important that, under these circumstances, exprIsConApp_maybe
+can spot a constructor application. So, for example, we don't
+consider
+       let x = e in (x,x)
+to be cheap, and that's good because exprIsConApp_maybe doesn't
+think that expression is a constructor application.
+
+I used to test is_value rather than is_cheap, which was utterly
+wrong, because the above expression responds True to exprIsHNF.
+
+This kind of thing can occur if you have
+
+       {-# INLINE foo #-}
+       foo = let x = e in (x,x)
+
+which Roman did.
+
 \begin{code}
 computeDiscount :: Int -> [Int] -> Int -> [ArgSummary] -> CallCtxt -> Int
 computeDiscount n_vals_wanted arg_discounts res_discount arg_infos cont_info
@@ -1004,7 +1128,7 @@ interestingArg e = go e 0
     go (Note _ a)       n = go a n
     go (Cast e _)       n = go e n
     go (Lam v e)        n 
-       | isTyVar v        = go e n
+       | isTyCoVar v      = go e n
        | n>0              = go e (n-1)
        | otherwise        = ValueArg
     go (Let _ e)        n = case go e n of { ValueArg -> ValueArg; _ -> NonTrivArg }
@@ -1039,13 +1163,14 @@ However e might not *look* as if
 -- where t1..tk are the *universally-qantified* type args of 'dc'
 exprIsConApp_maybe :: IdUnfoldingFun -> CoreExpr -> Maybe (DataCon, [Type], [CoreExpr])
 
-exprIsConApp_maybe id_unf (Note _ expr)
+exprIsConApp_maybe id_unf (Note note expr)
+  | notSccNote note
   = exprIsConApp_maybe id_unf expr
-       -- We ignore all notes.  For example,
+       -- We ignore all notes except SCCs.  For example,
        --      case _scc_ "foo" (C a b) of
        --                      C a b -> e
-       -- should be optimised away, but it will be only if we look
-       -- through the SCC note.
+       -- should not be optimised away, because we'll lose the
+       -- entry count on 'foo'; see Trac #4414
 
 exprIsConApp_maybe id_unf (Cast expr co)
   =     -- Here we do the KPush reduction rule as described in the FC paper
@@ -1124,13 +1249,15 @@ exprIsConApp_maybe id_unf expr
 
     analyse (Var fun) args
        | Just con <- isDataConWorkId_maybe fun
-        , is_saturated
+        , count isValArg args == idArity fun
        , let (univ_ty_args, rest_args) = splitAtList (dataConUnivTyVars con) args
        = Just (con, stripTypeArgs univ_ty_args, rest_args)
 
        -- Look through dictionary functions; see Note [Unfolding DFuns]
-        | DFunUnfolding con ops <- unfolding
-        , is_saturated
+        | DFunUnfolding dfun_nargs con ops <- unfolding
+        , let sat = length args == dfun_nargs    -- See Note [DFun arity check]
+          in if sat then True else 
+             pprTrace "Unsaturated dfun" (ppr fun <+> int dfun_nargs $$ ppr args) False   
         , let (dfun_tvs, _cls, dfun_res_tys) = tcSplitDFunTy (idType fun)
              subst = zipOpenTvSubst dfun_tvs (stripTypeArgs (takeList dfun_tvs args))
         = Just (con, substTys subst dfun_res_tys, 
@@ -1138,21 +1265,15 @@ exprIsConApp_maybe id_unf expr
 
        -- Look through unfoldings, but only cheap ones, because
        -- we are effectively duplicating the unfolding
-       | CoreUnfolding { uf_expandable = expand_me, uf_tmpl = rhs } <- unfolding
-       , expand_me = -- pprTrace "expanding" (ppr fun $$ ppr rhs) $
-                      analyse rhs args
+       | Just rhs <- expandUnfolding_maybe unfolding
+       = -- pprTrace "expanding" (ppr fun $$ ppr rhs) $
+          analyse rhs args
         where
-         is_saturated = count isValArg args == idArity fun
-          unfolding = id_unf fun    -- Does not look through loop breakers
-                   -- ToDo: we *may* look through variables that are NOINLINE
-                   --       in this phase, and that is really not right
+         unfolding = id_unf fun
 
     analyse _ _ = Nothing
 
     -----------
-    in_scope = mkInScopeSet (exprFreeVars expr)
-
-    -----------
     beta (Lam v body) pairs (arg : args) 
         | isTypeArg arg
         = beta body ((v,arg):pairs) args 
@@ -1161,13 +1282,9 @@ exprIsConApp_maybe id_unf expr
        = Nothing
 
     beta fun pairs args
-        = case analyse (substExpr subst fun) args of
-           Nothing  -> -- pprTrace "Bale out! exprIsConApp_maybe" doc $
-                       Nothing
-           Just ans -> -- pprTrace "Woo-hoo! exprIsConApp_maybe" doc $
-                        Just ans
+        = analyse (substExpr (text "subst-expr-is-con-app") subst fun) args
         where
-          subst = mkOpenSubst in_scope pairs
+          subst = mkOpenSubst (mkInScopeSet (exprFreeVars fun)) pairs
          -- doc = vcat [ppr fun, ppr expr, ppr pairs, ppr args]
 
 
@@ -1188,3 +1305,8 @@ So to split it up we just need to apply the ops $c1, $c2 etc
 to the very same args as the dfun.  It takes a little more work
 to compute the type arguments to the dictionary constructor.
 
+Note [DFun arity check]
+~~~~~~~~~~~~~~~~~~~~~~~
+Here we check that the total number of supplied arguments (inclding 
+type args) matches what the dfun is expecting.  This may be *less*
+than the ordinary arity of the dfun: see Note [DFun unfoldings] in CoreSyn