[Git][ghc/ghc][wip/spj-unf-size] Minor refactoring...

Simon Peyton Jones (@simonpj) gitlab at gitlab.haskell.org
Sat Nov 4 00:39:09 UTC 2023



Simon Peyton Jones pushed to branch wip/spj-unf-size at Glasgow Haskell Compiler / GHC


Commits:
55dc1a59 by Simon Peyton Jones at 2023-11-04T00:38:11+00:00
Minor refactoring...

Plus: don't be so eager to inline when argument is a non-value,
but has some struture.

We want *some* incentive though.

- - - - -


3 changed files:

- compiler/GHC/Core.hs
- compiler/GHC/Core/Opt/Simplify/Inline.hs
- compiler/GHC/Core/Unfold.hs


Changes:

=====================================
compiler/GHC/Core.hs
=====================================
@@ -1416,6 +1416,8 @@ data CaseTree
                          -- nothing relies on non-empty-ness
 
   | ScrutOf Id Discount  -- If this Id is bound to a value, apply this discount
+                         -- All size info is accounted for elsewhere;
+                         -- ScrutOf just records a discount
 
 data AltTree  = AltTree AltCon
                         [Id]      -- Term variables only


=====================================
compiler/GHC/Core/Opt/Simplify/Inline.hs
=====================================
@@ -631,8 +631,8 @@ exprSummary env e = go env e []
       where
         env' = modifyInScope env b  -- Tricky corner here
 
-    go _ _ _ = ArgIsNot []    -- Some structure; not all boring
-                              -- Example of improvement: base/tests/T9848
+    go _ _ _ = ArgNonTriv  -- Some structure; not all boring
+                           -- Example of improvement: base/tests/T9848
 
     go_var :: SimplEnv -> Id
            -> [CoreExpr]   -- Value args only
@@ -642,7 +642,7 @@ exprSummary env e = go env e []
       = ArgIsCon (DataAlt con) (map (exprSummary env) val_args)
 
       | DFunUnfolding {} <- unfolding
-      = ArgIsNot []  -- Says "this is a data con" without saying which
+      = ArgNonTriv   -- Says "this is a data con" without saying which
                      -- Will also return this for ($df d1 .. dn)
 
       | Just rhs <- expandUnfolding_maybe unfolding
@@ -655,9 +655,9 @@ exprSummary env e = go env e []
       = ArgIsLam
 
       | not (null val_args)
-      = ArgIsNot []   -- Use ArgIsNot [] for args with some structure e.g. (f xs)
-                      -- This makes the call not totally-boring, and hence makes
-                      -- INLINE things inline (which they won't if all args are boring)
+      = ArgNonTriv  -- Use ArgNonTriv args with some structure e.g. (f xs)
+                    -- This makes the call not totally-boring, and hence makes
+                    -- INLINE things inline (which they won't if all args are boring)
 
       | otherwise
       = ArgNoInfo


=====================================
compiler/GHC/Core/Unfold.hs
=====================================
@@ -67,6 +67,11 @@ import qualified Data.ByteString as BS
 
 {- Note [Overview of inlining heuristics]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+These inlining heurstics all concern callSiteInline; that is, the
+decision about whether or not to inline a let-binding.  It does not
+concern inlining used-once things, or things with a trivial RHS, which
+kills the let-binding altogether.
+
 Key examples
 ------------
 Example 1:
@@ -79,7 +84,7 @@ Example 1:
 Even though f's entire RHS is big, it collapses to something small when applied
 to A.  We'd like to spot this.
 
-Example 1:
+Example 2:
 
    let f x = case x of
                (p,q) -> case p of
@@ -105,9 +110,35 @@ when `y` is a free variable.
 
 This kind of thing can occur a lot with join points.
 
+Example 4: result discounts:
+
+   let f x = case x of
+               A -> (e1,e2)
+               B -> (e3,e4)
+               C -> e5
+    in \y -> ...case f y of { (a,b) -> blah }
+
+Here there is nothing interesting about f's /argument/, but:
+  * Many of f's cases return a data constructur (True or False)
+  * The call of `f` scrutinises its result
+
+If we inline `f`, the 'case' will cancel with pair constrution, we should be keener
+to inline `f` than if it was called in a boring context. We say that `f`  has a
+/result discount/ meaning that we should apply a discount if `f` is called in
+a case context.
+
+Example 5: totally boring
+
+   let f x = not (g x x)
+   in ....(\y. f y)...
+
+Here,there is /nothing/ interesting about either the arguments or the result
+coninuation of the call (f y).  There is no point in inlining, even if f's RHS
+is small, as it is here.
+
 Design overview
 ---------------
-The question is whethe or not to inline f = rhs.
+The question is whether or not to inline f = rhs.
 The key idea is to abstract `rhs` to an ExprTree, which gives a measure of
 size, but records structure for case-expressions.
 
@@ -149,7 +180,7 @@ data UnfoldingOpts = UnfoldingOpts
    , unfoldingVeryAggressive :: !Bool
       -- ^ Force inlining in many more cases
 
-   , unfoldingCaseThreshold :: !Size
+   , unfoldingCaseThreshold :: !Int
       -- ^ Don't consider depth up to x
 
    , unfoldingCaseScaling :: !Int
@@ -467,6 +498,11 @@ uncondInline rhs arity size
 
 {- Note [Constructing an ExprTree]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+We construct an ExprTree as an abstaction or "digest" of the body
+of a function    f = \x1...xn. body
+
+The arguments are x1..xn; we apply `exprTree` to the `body`.
+
 We maintain:
 * avs: argument variables, or variables bound by a case on an
        argument variable.
@@ -527,7 +563,7 @@ exprTree opts args expr
     et_add_alt = metAddAlt bOMB_OUT_SIZE
 
     go :: Int -> ETVars -> CoreExpr -> Maybe ExprTree
-          -- rcd is the /unused/ case depth; decreases toward zero
+          -- rcd is the /remaining/ case depth; decreases toward zero
           -- (avs,lvs): see Note [Constructing an ExprTree]
     go rcd vs (Cast e _)      = go rcd vs e
     go rcd vs (Tick _ e)      = go rcd vs e
@@ -640,7 +676,8 @@ exprTree opts args expr
 
     -- Don't record a CaseOf
     go_case rcd vs scrut b alts    -- alts is non-empty
-      = caseSize scrut alts  `metAddN`   -- A bit odd that this is only in one branch
+      = caseSize scrut alts  `metAddN`   -- A bit odd that this `caseSize` business is only
+                                         -- applied in this equation, not in the previous ones
         go rcd vs scrut      `et_add`
         go_alts (rcd-1) vs b alts
 
@@ -783,7 +820,7 @@ funSize :: UnfoldingOpts -> ETVars -> Id -> Int -> Int -> ExprTree
 -- Size for function calls that are not constructors or primops
 -- Note [Function applications]
 funSize opts (avs,_) fun n_val_args voids
-  | fun `hasKey` buildIdKey   = etZero  -- Wwant to inline applications of build/augment
+  | fun `hasKey` buildIdKey   = etZero  -- We want to inline applications of build/augment
   | fun `hasKey` augmentIdKey = etZero  -- so we give size zero to the whole call
   | otherwise = ExprTree { et_wc_tot = size, et_size  = size
                          , et_cases = cases
@@ -1124,10 +1161,14 @@ data InlineContext
                                         --          so apply result discount
      }
 
-data ArgSummary = ArgNoInfo
-                | ArgIsCon AltCon [ArgSummary]  -- Value args only
-                | ArgIsNot [AltCon]
-                | ArgIsLam
+data ArgSummary
+  = ArgNoInfo
+  | ArgIsCon AltCon [ArgSummary]  -- Arg is a data-con application;
+                                  --   the [ArgSummary] is for value args only
+  | ArgIsNot [AltCon]             -- Arg is a value, not headed by these construtors
+  | ArgIsLam                      -- Arg is a lambda
+  | ArgNonTriv                    -- The arg has some struture, but is not a value
+                                  --   e.g. it might be a call (f x)
 
 hasArgInfo :: ArgSummary -> Bool
 hasArgInfo ArgNoInfo = False
@@ -1136,6 +1177,7 @@ hasArgInfo _         = True
 instance Outputable ArgSummary where
   ppr ArgNoInfo       = text "ArgNoInfo"
   ppr ArgIsLam        = text "ArgIsLam"
+  ppr ArgNonTriv      = text "ArgNonTriv"
   ppr (ArgIsCon c as) = ppr c <> ppr as
   ppr (ArgIsNot cs)   = text "ArgIsNot" <> ppr cs
 
@@ -1163,14 +1205,18 @@ caseTreeSize :: InlineContext -> CaseTree -> Size
 caseTreeSize ic (ScrutOf bndr disc)
   = case lookupBndr ic bndr of
       ArgNoInfo   -> 0
-      ArgIsNot {} -> -disc  -- E.g. bndr is a DFun application
+      ArgNonTriv  -> -10    -- E.g. bndr is a DFun application
                             --      T8732 need to inline mapM_
+
+      -- Apply full discount for values
       ArgIsLam    -> -disc  -- Apply discount
+      ArgIsNot {} -> -disc
       ArgIsCon {} -> -disc  -- Apply discount
 
 caseTreeSize ic (CaseOf scrut_var case_bndr alts)
   = case lookupBndr ic scrut_var of
-      ArgNoInfo     -> altsSize ic case_bndr alts + case_size
+      ArgNoInfo  -> altsSize ic case_bndr alts + case_size
+      ArgNonTriv -> altsSize ic case_bndr alts + case_size
 
       ArgIsNot cons -> altsSize ic case_bndr (trim_alts cons alts)
          -- The case-expression may not disappear, but it scrutinises



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/55dc1a5972a40610c416a428805b5294b3160f71

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/55dc1a5972a40610c416a428805b5294b3160f71
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20231103/730538ab/attachment-0001.html>


More information about the ghc-commits mailing list