[commit: ghc] master: Fix the lone-variable case in callSiteInline (0636689)
git at git.haskell.org
git at git.haskell.org
Thu Jan 25 17:20:48 UTC 2018
Repository : ssh://git@git.haskell.org/ghc
On branch : master
Link : http://ghc.haskell.org/trac/ghc/changeset/06366890ba77c20198d7fccc870083b0bbfb1b11/ghc
>---------------------------------------------------------------
commit 06366890ba77c20198d7fccc870083b0bbfb1b11
Author: Simon Peyton Jones <simonpj at microsoft.com>
Date: Thu Jan 25 10:32:46 2018 +0000
Fix the lone-variable case in callSiteInline
See Note [Lone variables] in CoreUnfold and
Note [exprIsExpandable] in CoreUtils.
Helpfully pointed out by Matthew Pickering in Trac #14688
Nofib results are good:
--------------------------------------------------------------------------------
Program Size Allocs Runtime Elapsed TotalMem
--------------------------------------------------------------------------------
anna +0.1% +0.3% 0.151 0.151 0.0%
awards +0.0% -0.2% 0.001 0.001 0.0%
compress2 +0.6% -0.7% -4.8% -5.0% -4.0%
eliza +0.0% -2.4% 0.001 0.001 0.0%
fulsom +0.4% -13.3% -7.6% -7.6% +190.0%
gamteb +0.0% -0.6% 0.062 0.062 0.0%
gg +0.1% -0.4% 0.016 0.016 0.0%
ida +0.1% +0.3% 0.110 0.110 0.0%
kahan +0.0% -0.7% -0.9% -0.9% 0.0%
mate +0.1% -5.2% -4.9% -4.9% 0.0%
n-body +0.0% -0.2% -0.3% -3.0% 0.0%
pretty +0.0% -2.8% 0.000 0.000 0.0%
scs +0.0% -0.2% +1.6% +2.4% 0.0%
simple +0.4% -0.2% -2.3% -2.3% -3.4%
veritas +0.4% -1.0% 0.003 0.003 0.0%
wang +0.0% -1.6% 0.165 0.165 0.0%
--------------------------------------------------------------------------------
Min -0.0% -13.3% -16.2% -18.8% -4.0%
Max +0.6% +0.3% +4.9% +4.9% +190.0%
Geometric Mean +0.1% -0.3% -1.7% -2.4% +0.9%
>---------------------------------------------------------------
06366890ba77c20198d7fccc870083b0bbfb1b11
compiler/coreSyn/CoreUnfold.hs | 9 ++--
compiler/coreSyn/CoreUtils.hs | 102 +++++++++++++++++++++++++++++------------
2 files changed, 77 insertions(+), 34 deletions(-)
diff --git a/compiler/coreSyn/CoreUnfold.hs b/compiler/coreSyn/CoreUnfold.hs
index c459fd2..2e2b7a3 100644
--- a/compiler/coreSyn/CoreUnfold.hs
+++ b/compiler/coreSyn/CoreUnfold.hs
@@ -1241,8 +1241,8 @@ tryUnfolding dflags id lone_variable
= True
| otherwise
= case cont_info of
- CaseCtxt -> not (lone_variable && is_wf) -- Note [Lone variables]
- ValAppCtxt -> True -- Note [Cast then apply]
+ CaseCtxt -> not (lone_variable && is_exp) -- Note [Lone variables]
+ ValAppCtxt -> True -- Note [Cast then apply]
RuleArgCtxt -> uf_arity > 0 -- See Note [Unfold info lazy contexts]
DiscArgCtxt -> uf_arity > 0 --
RhsCtxt -> uf_arity > 0 --
@@ -1388,9 +1388,10 @@ because the latter is strict.
s = "foo"
f = \x -> ...(error s)...
-Fundamentally such contexts should not encourage inlining because the
+Fundamentally such contexts should not encourage inlining because, provided
+the RHS is "expandable" (see Note [exprIsExpandable] in CoreUtils) the
context can ``see'' the unfolding of the variable (e.g. case or a
-RULE) so there's no gain. If the thing is bound to a value.
+RULE) so there's no gain.
However, watch out:
diff --git a/compiler/coreSyn/CoreUtils.hs b/compiler/coreSyn/CoreUtils.hs
index 5e32dc6..3d5f4bc 100644
--- a/compiler/coreSyn/CoreUtils.hs
+++ b/compiler/coreSyn/CoreUtils.hs
@@ -1083,29 +1083,6 @@ Note that exprIsHNF does not imply exprIsCheap. Eg
This responds True to exprIsHNF (you can discard a seq), but
False to exprIsCheap.
-Note [exprIsExpandable]
-~~~~~~~~~~~~~~~~~~~~~~~
-An expression is "expandable" if we are willing to dupicate it, if doing
-so might make a RULE or case-of-constructor fire. Mainly this means
-data-constructor applications, but it's a bit more generous than exprIsCheap
-because it is true of "CONLIKE" Ids: see Note [CONLIKE pragma] in BasicTypes.
-
-It is used to set the uf_expandable field of an Unfolding, and that
-in turn is used
- * In RULE matching
- * In exprIsConApp_maybe, exprIsLiteral_maybe, exprIsLambda_maybe
-
-But take care: exprIsExpandable should /not/ be true of primops. I
-found this in test T5623a:
- let q = /\a. Ptr a (a +# b)
- in case q @ Float of Ptr v -> ...q...
-
-q's inlining should not be expandable, else exprIsConApp_maybe will
-say that (q @ Float) expands to (Ptr a (a +# b)), and that will
-duplicate the (a +# b) primop, which we should not do lightly.
-(It's quite hard to trigger this bug, but T13155 does so for GHC 8.0.)
-
-
Note [Arguments and let-bindings exprIsCheapX]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
What predicate should we apply to the argument of an application, or the
@@ -1131,16 +1108,12 @@ in this (which it previously was):
-}
--------------------
-exprIsCheap :: CoreExpr -> Bool
-exprIsCheap = exprIsCheapX isCheapApp
-
-exprIsExpandable :: CoreExpr -> Bool -- See Note [exprIsExpandable]
-exprIsExpandable = exprIsCheapX isExpandableApp
-
exprIsWorkFree :: CoreExpr -> Bool -- See Note [exprIsWorkFree]
exprIsWorkFree = exprIsCheapX isWorkFreeApp
---------------------
+exprIsCheap :: CoreExpr -> Bool
+exprIsCheap = exprIsCheapX isCheapApp
+
exprIsCheapX :: CheapAppFun -> CoreExpr -> Bool
exprIsCheapX ok_app e
= ok e
@@ -1168,6 +1141,75 @@ exprIsCheapX ok_app e
-- App, Let: see Note [Arguments and let-bindings exprIsCheapX]
+{- Note [exprIsExpandable]
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+An expression is "expandable" if we are willing to duplicate it, if doing
+so might make a RULE or case-of-constructor fire. Consider
+ let x = (a,b)
+ y = build g
+ in ....(case x of (p,q) -> rhs)....(foldr k z y)....
+
+We don't inline 'x' or 'y' (see Note [Lone variables] in CoreUnfold),
+but we do want
+
+ * the case-expression to simplify
+ (via exprIsConApp_maybe, exprIsLiteral_maybe)
+
+ * the foldr/build RULE to fire
+ (by expanding the unfolding during rule matching)
+
+So we classify the unfolding of a let-binding as "expandable" (via the
+uf_expandable field) if we want to do this kind of on-the-fly
+expansion. Specifically:
+
+* True of constructor applications (K a b)
+
+* True of applications of a "CONLIKE" Id; see Note [CONLIKE pragma] in BasicTypes.
+ (NB: exprIsCheap might not be true of this)
+
+* False of case-expressions. If we have
+ let x = case ... in ...(case x of ...)...
+ we won't simplify. We have to inline x. See Trac #14688.
+
+* False of let-expressions (same reason); and in any case we
+ float lets out of an RHS if doing so will reveal an expandable
+ application (see SimplEnv.doFloatFromRhs).
+
+* Take care: exprIsExpandable should /not/ be true of primops. I
+ found this in test T5623a:
+ let q = /\a. Ptr a (a +# b)
+ in case q @ Float of Ptr v -> ...q...
+
+ q's inlining should not be expandable, else exprIsConApp_maybe will
+ say that (q @ Float) expands to (Ptr a (a +# b)), and that will
+ duplicate the (a +# b) primop, which we should not do lightly.
+ (It's quite hard to trigger this bug, but T13155 does so for GHC 8.0.)
+-}
+
+-------------------------------------
+exprIsExpandable :: CoreExpr -> Bool
+-- See Note [exprIsExpandable]
+exprIsExpandable e
+ = ok e
+ where
+ ok e = go 0 e
+
+ -- n is the number of value arguments
+ go n (Var v) = isExpandableApp v n
+ go _ (Lit {}) = True
+ go _ (Type {}) = True
+ go _ (Coercion {}) = True
+ go n (Cast e _) = go n e
+ go n (Tick t e) | tickishCounts t = False
+ | otherwise = go n e
+ go n (Lam x e) | isRuntimeVar x = n==0 || go (n-1) e
+ | otherwise = go n e
+ go n (App f e) | isRuntimeArg e = go (n+1) f && ok e
+ | otherwise = go n f
+ go _ (Case {}) = False
+ go _ (Let {}) = False
+
+
-------------------------------------
type CheapAppFun = Id -> Arity -> Bool
-- Is an application of this function to n *value* args
More information about the ghc-commits
mailing list