Commit 06366890 authored by Simon Peyton Jones's avatar Simon Peyton Jones

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%
parent 2a2e6a8f
......@@ -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:
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment