Demand analysis: simplify the demand for a RHS

Ticket #17932 showed that we were using a stupid demand for the RHS
of a let-binding, when the result is a product.  This was the result
of a "fix" in 2013, which (happily) turns out to no longer be
necessary.

So I just deleted the code, which simplifies the demand analyser,
and fixes #17932. That in turn uncovered that the anticipation
of worker/wrapper in CPR analysis was inaccurate, hence the logic
that decides whether to unbox an argument in WW was extracted into
a function `wantToUnbox`, now consulted by CPR analysis.

I tried nofib, and got 0.0% perf changes.

All this came up when messing about with !2873 (ticket #17917),
but is idependent of it.

Unfortunately, this patch regresses #4267 and realised that it is now
blocked on #16335.

3 files changed, 117 insertions, 96 deletions

diff --git a/compiler/GHC/Core/Op/CprAnal.hs b/compiler/GHC/Core/Op/CprAnal.hs
index 8016c2c13d..022ce0b7f1 100644
--- a/compiler/GHC/Core/Op/CprAnal.hs
+++ b/compiler/GHC/Core/Op/CprAnal.hs
@@ -13,7 +13,6 @@ module GHC.Core.Op.CprAnal ( cprAnalProgram ) where
 
 import GhcPrelude
 
-import GHC.Core.Op.WorkWrap.Lib ( deepSplitProductType_maybe )
 import GHC.Driver.Session
 import GHC.Types.Demand
 import GHC.Types.Cpr
@@ -30,6 +29,7 @@ import GHC.Core.Utils   ( exprIsHNF, dumpIdInfoOfProgram )
 import GHC.Core.TyCon
 import GHC.Core.Type
 import GHC.Core.FamInstEnv
+import GHC.Core.Op.WorkWrap.Lib
 import Util
 import ErrUtils         ( dumpIfSet_dyn, DumpFormat (..) )
 import Maybes           ( isJust, isNothing )
@@ -88,7 +88,8 @@ Ideally, we would want the following pipeline:
 4. worker/wrapper (for CPR)
 
 Currently, we omit 2. and anticipate the results of worker/wrapper.
-See Note [CPR in a DataAlt case alternative] and Note [CPR for strict binders].
+See Note [CPR in a DataAlt case alternative]
+and Note [CPR for binders that will be unboxed].
 An additional w/w pass would simplify things, but probably add slight overhead.
 So currently we have
 
@@ -175,7 +176,7 @@ cprAnal' env (Lam var body)
   | otherwise
   = (lam_ty, Lam var body')
   where
-    env'             = extendSigsWithLam env var
+    env'             = extendAnalEnvForDemand env var (idDemandInfo var)
     (body_ty, body') = cprAnal env' body
     lam_ty           = abstractCprTy body_ty
 
@@ -392,15 +393,25 @@ lookupSigEnv env id = lookupVarEnv (ae_sigs env) id
 nonVirgin :: AnalEnv -> AnalEnv
 nonVirgin env = env { ae_virgin = False }
 
-extendSigsWithLam :: AnalEnv -> Id -> AnalEnv
--- Extend the AnalEnv when we meet a lambda binder
-extendSigsWithLam env id
+-- | A version of 'extendAnalEnv' for a binder of which we don't see the RHS
+-- needed to compute a 'CprSig' (e.g. lambdas and DataAlt field binders).
+-- In this case, we can still look at their demand to attach CPR signatures
+-- anticipating the unboxing done by worker/wrapper.
+-- See Note [CPR for binders that will be unboxed].
+extendAnalEnvForDemand :: AnalEnv -> Id -> Demand -> AnalEnv
+extendAnalEnvForDemand env id dmd
   | isId id
-  , isStrictDmd (idDemandInfo id) -- See Note [CPR for strict binders]
-  , Just (dc,_,_,_) <- deepSplitProductType_maybe (ae_fam_envs env) $ idType id
+  , Just (_, DataConAppContext { dcac_dc = dc })
+      <- wantToUnbox (ae_fam_envs env) has_inlineable_prag (idType id) dmd
   = extendAnalEnv env id (CprSig (conCprType (dataConTag dc)))
   | otherwise
   = env
+  where
+    -- Rather than maintaining in AnalEnv whether we are in an INLINEABLE
+    -- function, we just assume that we aren't. That flag is only relevant
+    -- to Note [Do not unpack class dictionaries], the few unboxing
+    -- opportunities on dicts it prohibits are probably irrelevant to CPR.
+    has_inlineable_prag = False
 
 extendEnvForDataAlt :: AnalEnv -> CoreExpr -> Id -> DataCon -> [Var] -> AnalEnv
 -- See Note [CPR in a DataAlt case alternative]
@@ -425,18 +436,16 @@ extendEnvForDataAlt env scrut case_bndr dc bndrs
     -- propagate available unboxed things from the scrutinee, getting rid of
     -- the is_var_scrut heuristic. See Note [CPR in a DataAlt case alternative].
     -- Giving strict binders the CPR property only makes sense for products, as
-    -- the arguments in Note [CPR for strict binders] don't apply to sums (yet);
-    -- we lack WW for strict binders of sum type.
+    -- the arguments in Note [CPR for binders that will be unboxed] don't apply
+    -- to sums (yet); we lack WW for strict binders of sum type.
     do_con_arg env (id, str)
-       | let is_strict = isStrictDmd (idDemandInfo id) || isMarkedStrict str
-       , is_var_scrut && is_strict
-       , let fam_envs = ae_fam_envs env
-       , Just (dc,_,_,_) <- deepSplitProductType_maybe fam_envs $ idType id
-       = extendAnalEnv env id (CprSig (conCprType (dataConTag dc)))
+       | is_var scrut
+       -- See Note [Add demands for strict constructors] in WorkWrap.Lib
+       , let dmd = applyWhen (isMarkedStrict str) strictifyDmd (idDemandInfo id)
+       = extendAnalEnvForDemand env id dmd
        | otherwise
        = env
 
-    is_var_scrut = is_var scrut
     is_var (Cast e _) = is_var e
     is_var (Var v)    = isLocalId v
     is_var _          = False
@@ -472,7 +481,8 @@ Specifically
    box.  If the wrapper doesn't cancel with its caller, we'll end up
    re-boxing something that we did have available in boxed form.
 
- * Any strict binders with product type, can use Note [CPR for strict binders]
+ * Any strict binders with product type, can use
+   Note [CPR for binders that will be unboxed]
    to anticipate worker/wrappering for strictness info.
    But we can go a little further. Consider
 
@@ -499,11 +509,11 @@ Specifically
    sub-component thereof.  But it's simple, and nothing terrible
    happens if we get it wrong.  e.g. Trac #10694.
 
-Note [CPR for strict binders]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-If a lambda-bound variable is marked demanded with a strict demand, then give it
-a CPR signature, anticipating the results of worker/wrapper. Here's a concrete
-example ('f1' in test T10482a), assuming h is strict:
+Note [CPR for binders that will be unboxed]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+If a lambda-bound variable will be unboxed by worker/wrapper (so it must be
+demanded strictly), then give it a CPR signature. Here's a concrete example
+('f1' in test T10482a), assuming h is strict:
 
   f1 :: Int -> Int
   f1 x = case h x of
@@ -527,6 +537,9 @@ Note that
     has product type, else we may get over-optimistic CPR results
     (e.g. from \x -> x!).
 
+  * This also (approximately) applies to DataAlt field binders;
+    See Note [CPR in a DataAlt case alternative].
+
   * See Note [CPR examples]
 
 Note [CPR for sum types]
@@ -628,21 +641,6 @@ point: all of these functions can have the CPR property.
             True  -> x
             False -> f1 (x-1)
 
-
-    ------- f2 -----------
-    -- x is a strict field of MkT2, so we'll pass it unboxed
-    -- to $wf2, so it's available unboxed.  This depends on
-    -- the case expression analysing (a subcomponent of) one
-    -- of the original arguments to the function, so it's
-    -- a bit more delicate.
-
-    data T2 = MkT2 !Int Int
-
-    f2 :: T2 -> Int
-    f2 (MkT2 x y) | y>0       = f2 (MkT2 x (y-1))
-                  | otherwise = x
-
-
     ------- f3 -----------
     -- h is strict in x, so x will be unboxed before it
     -- is rerturned in the otherwise case.
@@ -652,18 +650,4 @@ point: all of these functions can have the CPR property.
     f1 :: T3 -> Int
     f1 (MkT3 x y) | h x y     = f3 (MkT3 x (y-1))
                   | otherwise = x
-
-
-    ------- f4 -----------
-    -- Just like f2, but MkT4 can't unbox its strict
-    -- argument automatically, as f2 can
-
-    data family Foo a
-    newtype instance Foo Int = Foo Int
-
-    data T4 a = MkT4 !(Foo a) Int
-
-    f4 :: T4 Int -> Int
-    f4 (MkT4 x@(Foo v) y) | y>0       = f4 (MkT4 x (y-1))
-                          | otherwise = v
 -}
diff --git a/compiler/GHC/Core/Op/DmdAnal.hs b/compiler/GHC/Core/Op/DmdAnal.hs
index 88e96773ac..08d244a36a 100644
--- a/compiler/GHC/Core/Op/DmdAnal.hs
+++ b/compiler/GHC/Core/Op/DmdAnal.hs
@@ -617,16 +617,11 @@ dmdAnalRhsLetDown rec_flag env let_dmd id rhs
     is_thunk = not (exprIsHNF rhs) && not (isJoinId id)
 
 -- | @mkRhsDmd env rhs_arity rhs@ creates a 'CleanDemand' for
--- unleashing on the given function's @rhs@, by creating a call demand of
--- @rhs_arity@ with a body demand appropriate for possible product types.
--- See Note [Product demands for function body].
--- For example, a call of the form @mkRhsDmd _ 2 (\x y -> (x, y))@ returns a
--- clean usage demand of @C1(C1(U(U,U)))@.
+-- unleashing on the given function's @rhs@, by creating
+-- a call demand of @rhs_arity@
+-- See Historical Note [Product demands for function body]
 mkRhsDmd :: AnalEnv -> Arity -> CoreExpr -> CleanDemand
-mkRhsDmd env rhs_arity rhs =
-  case peelTsFuns rhs_arity (findTypeShape (ae_fam_envs env) (exprType rhs)) of
-    Just (TsProd tss) -> mkCallDmds rhs_arity (cleanEvalProdDmd (length tss))
-    _                 -> mkCallDmds rhs_arity cleanEvalDmd
+mkRhsDmd _env rhs_arity _rhs = mkCallDmds rhs_arity cleanEvalDmd
 
 -- | If given the let-bound 'Id', 'useLetUp' determines whether we should
 -- process the binding up (body before rhs) or down (rhs before body).
@@ -857,9 +852,9 @@ forward plusInt's demand signature, and all is well (see Note [Newtype arity] in
 GHC.Core.Arity)! A small example is the test case NewtypeArity.
 
 
-Note [Product demands for function body]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-This example comes from shootout/binary_trees:
+Historical Note [Product demands for function body]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+In 2013 I spotted this example, in shootout/binary_trees:
 
     Main.check' = \ b z ds. case z of z' { I# ip ->
                                 case ds_d13s of
@@ -878,8 +873,12 @@ Here we *really* want to unbox z, even though it appears to be used boxed in
 the Nil case.  Partly the Nil case is not a hot path.  But more specifically,
 the whole function gets the CPR property if we do.
 
-So for the demand on the body of a RHS we use a product demand if it's
-a product type.
+That motivated using a demand of C(C(C(S(L,L)))) for the RHS, where
+(solely because the result was a product) we used a product demand
+(albeit with lazy components) for the body. But that gives very silly
+behaviour -- see #17932.   Happily it turns out now to be entirely
+unnecessary: we get good results with C(C(C(S))).   So I simply
+deleted the special case.
 
 ************************************************************************
 *                                                                      *
diff --git a/compiler/GHC/Core/Op/WorkWrap/Lib.hs b/compiler/GHC/Core/Op/WorkWrap/Lib.hs
index 6245bb9099..684c807d07 100644
--- a/compiler/GHC/Core/Op/WorkWrap/Lib.hs
+++ b/compiler/GHC/Core/Op/WorkWrap/Lib.hs
@@ -8,7 +8,8 @@ A library for the ``worker\/wrapper'' back-end to the strictness analyser
 
 module GHC.Core.Op.WorkWrap.Lib
    ( mkWwBodies, mkWWstr, mkWorkerArgs
-   , deepSplitProductType_maybe, findTypeShape
+   , DataConAppContext(..), deepSplitProductType_maybe, wantToUnbox
+   , findTypeShape
    , isWorkerSmallEnough
    )
 where
@@ -588,21 +589,8 @@ mkWWstr_one dflags fam_envs has_inlineable_prag arg
      -- (that's what mk_absent_let does)
   = return (True, [], nop_fn, work_fn)
 
-  | isStrictDmd dmd
-  , Just cs <- splitProdDmd_maybe dmd
-      -- See Note [Unpacking arguments with product and polymorphic demands]
-  , not (has_inlineable_prag && isClassPred arg_ty)
-      -- See Note [Do not unpack class dictionaries]
-  , Just stuff@(_, _, inst_con_arg_tys, _) <- deepSplitProductType_maybe fam_envs arg_ty
-  , cs `equalLength` inst_con_arg_tys
-      -- See Note [mkWWstr and unsafeCoerce]
-  = unbox_one dflags fam_envs arg cs stuff
-
-  | isSeqDmd dmd   -- For seqDmd, splitProdDmd_maybe will return Nothing, but
-                   -- it should behave like <S, U(AAAA)>, for some suitable arity
-  , Just stuff@(_, _, inst_con_arg_tys, _) <- deepSplitProductType_maybe fam_envs arg_ty
-  , let abs_dmds = map (const absDmd) inst_con_arg_tys
-  = unbox_one dflags fam_envs arg abs_dmds stuff
+  | Just (cs, acdc) <- wantToUnbox fam_envs has_inlineable_prag arg_ty dmd
+  = unbox_one dflags fam_envs arg cs acdc
 
   | otherwise   -- Other cases
   = return (False, [arg], nop_fn, nop_fn)
@@ -611,12 +599,36 @@ mkWWstr_one dflags fam_envs has_inlineable_prag arg
     arg_ty = idType arg
     dmd    = idDemandInfo arg
 
+wantToUnbox :: FamInstEnvs -> Bool -> Type -> Demand -> Maybe ([Demand], DataConAppContext)
+wantToUnbox fam_envs has_inlineable_prag ty dmd =
+  case deepSplitProductType_maybe fam_envs ty of
+    Just dcac@DataConAppContext{ dcac_arg_tys = con_arg_tys }
+      | isStrictDmd dmd
+      -- See Note [Unpacking arguments with product and polymorphic demands]
+      , Just cs <- split_prod_dmd_arity dmd (length con_arg_tys)
+      -- See Note [Do not unpack class dictionaries]
+      , not (has_inlineable_prag && isClassPred ty)
+      -- See Note [mkWWstr and unsafeCoerce]
+      , cs `equalLength` con_arg_tys
+      -> Just (cs, dcac)
+    _ -> Nothing
+  where
+    split_prod_dmd_arity dmd arty
+      -- For seqDmd, splitProdDmd_maybe will return Nothing (because how would
+      -- it know the arity?), but it should behave like <S, U(AAAA)>, for some
+      -- suitable arity
+      | isSeqDmd dmd = Just (replicate arty absDmd)
+      -- Otherwise splitProdDmd_maybe does the job
+      | otherwise    = splitProdDmd_maybe dmd
+
 unbox_one :: DynFlags -> FamInstEnvs -> Var
           -> [Demand]
-          -> (DataCon, [Type], [(Type, StrictnessMark)], Coercion)
+          -> DataConAppContext
           -> UniqSM (Bool, [Var], CoreExpr -> CoreExpr, CoreExpr -> CoreExpr)
 unbox_one dflags fam_envs arg cs
-          (data_con, inst_tys, inst_con_arg_tys, co)
+          DataConAppContext { dcac_dc = data_con, dcac_tys = inst_tys
+                            , dcac_arg_tys = inst_con_arg_tys
+                            , dcac_co = co }
   = do { (uniq1:uniqs) <- getUniquesM
         ; let   -- See Note [Add demands for strict constructors]
                 cs'       = addDataConStrictness data_con cs
@@ -898,8 +910,8 @@ If we have
    f :: Ord a => [a] -> Int -> a
    {-# INLINABLE f #-}
 and we worker/wrapper f, we'll get a worker with an INLINABLE pragma
-(see Note [Worker-wrapper for INLINABLE functions] in GHC.Core.Op.WorkWrap), which
-can still be specialised by the type-class specialiser, something like
+(see Note [Worker-wrapper for INLINABLE functions] in GHC.Core.Op.WorkWrap),
+which can still be specialised by the type-class specialiser, something like
    fw :: Ord a => [a] -> Int# -> a
 
 BUT if f is strict in the Ord dictionary, we might unpack it, to get
@@ -915,9 +927,29 @@ Historical note: #14955 describes how I got this fix wrong
 the first time.
 -}
 
-deepSplitProductType_maybe
-    :: FamInstEnvs -> Type
-    -> Maybe (DataCon, [Type], [(Type, StrictnessMark)], Coercion)
+-- | Context for a 'DataCon' application with a hole for every field, including
+-- surrounding coercions.
+-- The result of 'deepSplitProductType_maybe' and 'deepSplitCprType_maybe'.
+--
+-- Example:
+--
+-- > DataConAppContext Just [Int] [(Lazy, Int)] (co :: Maybe Int ~ First Int)
+--
+-- represents
+--
+-- > Just @Int (_1 :: Int) |> co :: First Int
+--
+-- where _1 is a hole for the first argument. The number of arguments is
+-- determined by the length of @arg_tys@.
+data DataConAppContext
+  = DataConAppContext
+  { dcac_dc      :: !DataCon
+  , dcac_tys     :: ![Type]
+  , dcac_arg_tys :: ![(Type, StrictnessMark)]
+  , dcac_co      :: !Coercion
+  }
+
+deepSplitProductType_maybe :: FamInstEnvs -> Type -> Maybe DataConAppContext
 -- If    deepSplitProductType_maybe ty = Just (dc, tys, arg_tys, co)
 -- then  dc @ tys (args::arg_tys) :: rep_ty
 --       co :: ty ~ rep_ty
@@ -930,12 +962,14 @@ deepSplitProductType_maybe fam_envs ty
   , Just con <- isDataProductTyCon_maybe tc
   , let arg_tys = dataConInstArgTys con tc_args
         strict_marks = dataConRepStrictness con
-  = Just (con, tc_args, zipEqual "dspt" arg_tys strict_marks, co)
+  = Just DataConAppContext { dcac_dc = con
+                           , dcac_tys = tc_args
+                           , dcac_arg_tys = zipEqual "dspt" arg_tys strict_marks
+                           , dcac_co = co }
 deepSplitProductType_maybe _ _ = Nothing
 
 deepSplitCprType_maybe
-    :: FamInstEnvs -> ConTag -> Type
-    -> Maybe (DataCon, [Type], [(Type, StrictnessMark)], Coercion)
+  :: FamInstEnvs -> ConTag -> Type -> Maybe DataConAppContext
 -- If    deepSplitCprType_maybe n ty = Just (dc, tys, arg_tys, co)
 -- then  dc @ tys (args::arg_tys) :: rep_ty
 --       co :: ty ~ rep_ty
@@ -952,7 +986,10 @@ deepSplitCprType_maybe fam_envs con_tag ty
   , let con = cons `getNth` (con_tag - fIRST_TAG)
         arg_tys = dataConInstArgTys con tc_args
         strict_marks = dataConRepStrictness con
-  = Just (con, tc_args, zipEqual "dsct" arg_tys strict_marks, co)
+  = Just DataConAppContext { dcac_dc = con
+                           , dcac_tys = tc_args
+                           , dcac_arg_tys = zipEqual "dspt" arg_tys strict_marks
+                           , dcac_co = co }
 deepSplitCprType_maybe _ _ _ = Nothing
 
 findTypeShape :: FamInstEnvs -> Type -> TypeShape
@@ -1009,17 +1046,18 @@ mkWWcpr opt_CprAnal fam_envs body_ty cpr
   | otherwise
   = case asConCpr cpr of
        Nothing      -> return (False, id, id, body_ty)  -- No CPR info
-       Just con_tag | Just stuff <- deepSplitCprType_maybe fam_envs con_tag body_ty
-                    -> mkWWcpr_help stuff
+       Just con_tag | Just dcac <- deepSplitCprType_maybe fam_envs con_tag body_ty
+                    -> mkWWcpr_help dcac
                     |  otherwise
                        -- See Note [non-algebraic or open body type warning]
                     -> WARN( True, text "mkWWcpr: non-algebraic or open body type" <+> ppr body_ty )
                        return (False, id, id, body_ty)
 
-mkWWcpr_help :: (DataCon, [Type], [(Type,StrictnessMark)], Coercion)
+mkWWcpr_help :: DataConAppContext
              -> UniqSM (Bool, CoreExpr -> CoreExpr, CoreExpr -> CoreExpr, Type)
 
-mkWWcpr_help (data_con, inst_tys, arg_tys, co)
+mkWWcpr_help (DataConAppContext { dcac_dc = data_con, dcac_tys = inst_tys
+                                , dcac_arg_tys = arg_tys, dcac_co = co })
   | [arg1@(arg_ty1, _)] <- arg_tys
   , isUnliftedType arg_ty1
         -- Special case when there is a single result of unlifted type