[Git][ghc/ghc][wip/T22317] Simplifier: Consider `seq` as a `BoringCtxt` (#22317)

Sebastian Graf (@sgraf812) gitlab at gitlab.haskell.org
Thu Nov 17 12:00:23 UTC 2022



Sebastian Graf pushed to branch wip/T22317 at Glasgow Haskell Compiler / GHC


Commits:
026e2fc0 by Sebastian Graf at 2022-11-17T13:00:03+01:00
Simplifier: Consider `seq` as a `BoringCtxt` (#22317)

See `Note [Seq is boring]` for the rationale.

Fixes #22317.

- - - - -


6 changed files:

- compiler/GHC/Core/Opt/Simplify/Utils.hs
- compiler/GHC/Core/Unfold.hs
- + testsuite/tests/simplCore/should_compile/T22317.hs
- + testsuite/tests/simplCore/should_compile/T22317.stderr
- testsuite/tests/simplCore/should_compile/T22375.stderr
- testsuite/tests/simplCore/should_compile/all.T


Changes:

=====================================
compiler/GHC/Core/Opt/Simplify/Utils.hs
=====================================
@@ -740,8 +740,8 @@ Note [Interesting call context]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 We want to avoid inlining an expression where there can't possibly be
 any gain, such as in an argument position.  Hence, if the continuation
-is interesting (eg. a case scrutinee, application etc.) then we
-inline, otherwise we don't.
+is interesting (eg. a case scrutinee that isn't just a seq, application etc.)
+then we inline, otherwise we don't.
 
 Previously some_benefit used to return True only if the variable was
 applied to some value arguments.  This didn't work:
@@ -781,6 +781,53 @@ expression into the branches of any case in f's unfolding.  So, to
 reduce unnecessary code expansion, we just make the context look boring.
 This made a small compile-time perf improvement in perf/compiler/T6048,
 and it looks plausible to me.
+
+Note [Seq is boring]
+~~~~~~~~~~~~~~~~~~~~
+Suppose
+  f x = case v of
+          True  -> Just x
+          False -> Just (x-1)
+
+Now consider these cases:
+
+1. case f x of b{-dead-} { DEFAULT -> blah[no b] }
+     Inlining (f x) will allow us to avoid ever allocating (Just x),
+     since the case binder `b` is dead.  We will end up with a
+     join point for blah, thus
+         join j = blah in
+         case v of { True -> j; False -> j }
+     which will turn into (case v of DEFAULT -> blah
+     All good
+
+2. case f x of b { DEFAULT -> blah[b] }
+     Inlining (f x) will still mean we allocate (Just x). We'd get:
+         join j b = blah[b]
+         case v of { True -> j (Just x); False -> j (Just (x-1)) }
+     No new optimisations are revealed. Nothing is gained.
+     (This is the situation in T22317.)
+
+2a. case g x of b { (x{-dead-}, x{-dead-}) -> blah[b, no x, no y] }
+      Instead of DEFAULT we have a single constructor alternative
+      with all dead binders.  This is just a variant of (2); no
+      gain from inlining (f x)
+
+3. case f x of b { Just y -> blah[y,b] }
+     Inlining (f x) will mean we still allocate (Just x),
+     but we also get to bind `y` without fetching it out of the Just, thus
+         join j y b = blah[y,b]
+         case v of { True -> j x (Just x)
+                   ; False -> let y = x-1 in j y (Just y) }
+   Inlining (f x) has a small benefit, perhaps.
+   (To T14955 it makes a surprisingly large difference of ~30% to inline here.)
+
+
+Conclusion: if the case expression
+  * Has a non-dead case-binder
+  * Has one alternative
+  * All the binders in the alternative are dead
+then the `case` is just a strict let-binding, and the scrutinee is
+BoringCtxt (don't inline).  Otherwise CaseCtxt.
 -}
 
 lazyArgContext :: ArgInfo -> CallCtxt
@@ -811,10 +858,13 @@ interestingCallContext :: SimplEnv -> SimplCont -> CallCtxt
 interestingCallContext env cont
   = interesting cont
   where
-    interesting (Select {})
-       | seCaseCase env = CaseCtxt
-       | otherwise      = BoringCtxt
-       -- See Note [No case of case is boring]
+    interesting (Select {sc_alts=alts, sc_bndr=case_bndr})
+      | not (seCaseCase env)         = BoringCtxt -- See Note [No case of case is boring]
+      | [Alt _ bs _] <- alts
+      , all isDeadBinder bs
+      , not (isDeadBinder case_bndr) = BoringCtxt -- See Note [Seq is boring]
+      | otherwise                    = CaseCtxt
+
 
     interesting (ApplyToVal {}) = ValAppCtxt
         -- Can happen if we have (f Int |> co) y


=====================================
compiler/GHC/Core/Unfold.hs
=====================================
@@ -1347,8 +1347,11 @@ 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]   See also Note [Interaction of exprIsWorkFree and lone variables]
-~~~~~~~~~~~~~~~~~~~~~   which appears below
+Note [Lone variables]
+~~~~~~~~~~~~~~~~~~~~~
+See also Note [Interaction of exprIsWorkFree and lone variables]
+which appears below
+
 The "lone-variable" case is important.  I spent ages messing about
 with unsatisfactory variants, but this is nice.  The idea is that if a
 variable appears all alone


=====================================
testsuite/tests/simplCore/should_compile/T22317.hs
=====================================
@@ -0,0 +1,20 @@
+{-# LANGUAGE BangPatterns #-}
+
+module T22317 where
+
+data T = T (Maybe Bool) (Maybe Bool) (Maybe Bool) (Maybe Bool)
+
+
+m :: Maybe a -> Maybe a -> Maybe a
+m (Just v1) Nothing = Just v1
+m _         mb      = mb
+{-# INLINE m #-}
+
+f :: T -> T -> T
+f (T a1 b1 c1 d1) (T a2 b2 c2 d2)
+  = let j1 !a = let j2 !b = let j3 !c = let j4 !d = T a b c d
+                                        in j4 (m d1 d2)
+                            in j3 (m c1 c2)
+                in j2 (m b1 b2)
+    in j1 (m a1 a2)
+{-# OPAQUE f #-}


=====================================
testsuite/tests/simplCore/should_compile/T22317.stderr
=====================================
@@ -0,0 +1,256 @@
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO
+Considering inlining: m
+  arg infos [TrivArg, TrivArg]
+  interesting continuation BoringCtxt
+  some_benefit False
+  is exp: True
+  is work-free: True
+  guidance ALWAYS_IF(arity=2,unsat_ok=False,boring_ok=False)
+  ANSWER = NO


=====================================
testsuite/tests/simplCore/should_compile/T22375.stderr
=====================================
@@ -1,7 +1,7 @@
 
 ==================== Tidy Core ====================
 Result size of Tidy Core
-  = {terms: 71, types: 31, coercions: 0, joins: 0/0}
+  = {terms: 76, types: 37, coercions: 0, joins: 0/0}
 
 -- RHS size: {terms: 14, types: 7, coercions: 0, joins: 0/0}
 T22375.$fEqX_$c== :: X -> X -> Bool
@@ -46,7 +46,24 @@ T22375.$fEqX [InlPrag=CONLIKE] :: Eq X
 T22375.$fEqX
   = GHC.Classes.C:Eq @X T22375.$fEqX_$c== T22375.$fEqX_$c/=
 
--- RHS size: {terms: 32, types: 5, coercions: 0, joins: 0/0}
+-- RHS size: {terms: 24, types: 3, coercions: 0, joins: 0/0}
+T22375.$wf [InlPrag=[2]] :: X -> GHC.Prim.Int# -> GHC.Prim.Int#
+[GblId[StrictWorker([!])],
+ Arity=2,
+ Str=<1L><L>,
+ Unf=Unf{Src=<vanilla>, TopLvl=True, Value=True, ConLike=True,
+         WorkFree=True, Expandable=True, Guidance=IF_ARGS [64 0] 55 0}]
+T22375.$wf
+  = \ (x :: X) (ww :: GHC.Prim.Int#) ->
+      case x of {
+        A -> GHC.Prim.+# 1# ww;
+        B -> GHC.Prim.+# 2# ww;
+        C -> GHC.Prim.+# 3# ww;
+        D -> GHC.Prim.+# 4# ww;
+        E -> GHC.Prim.+# 5# ww
+      }
+
+-- RHS size: {terms: 12, types: 5, coercions: 0, joins: 0/0}
 f [InlPrag=[2]] :: X -> Int -> Int
 [GblId,
  Arity=2,
@@ -57,13 +74,7 @@ f [InlPrag=[2]] :: X -> Int -> Int
          Guidance=ALWAYS_IF(arity=2,unsat_ok=True,boring_ok=False)}]
 f = \ (x :: X) (v :: Int) ->
       case v of { GHC.Types.I# ww ->
-      case x of {
-        A -> GHC.Types.I# (GHC.Prim.+# 1# ww);
-        B -> GHC.Types.I# (GHC.Prim.+# 2# ww);
-        C -> GHC.Types.I# (GHC.Prim.+# 3# ww);
-        D -> GHC.Types.I# (GHC.Prim.+# 4# ww);
-        E -> GHC.Types.I# (GHC.Prim.+# 5# ww)
-      }
+      case T22375.$wf x ww of ww1 { __DEFAULT -> GHC.Types.I# ww1 }
       }
 
 


=====================================
testsuite/tests/simplCore/should_compile/all.T
=====================================
@@ -449,3 +449,5 @@ test('T22375', normal, compile, ['-O -ddump-simpl -dsuppress-uniques -dno-typeab
 # One module, T21851_2.hs, has OPTIONS_GHC -ddump-simpl
 # Expecting to see $s$wwombat
 test('T21851_2', [grep_errmsg(r'wwombat') ], multimod_compile, ['T21851_2', '-O -dno-typeable-binds -dsuppress-uniques'])
+# Should not inline m, so there shouldn't be a single YES
+test('T22317', [grep_errmsg(r'ANSWER = YES') ], compile, ['-O -dinline-check m -ddebug-output'])



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/026e2fc04a2bfb669ae9851ad0f48d1ae48d6790

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/026e2fc04a2bfb669ae9851ad0f48d1ae48d6790
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/20221117/43d6db6e/attachment-0001.html>


More information about the ghc-commits mailing list