Commit 1675d40a authored by Sebastian Graf's avatar Sebastian Graf Committed by Marge Bot

Always do the worker/wrapper split for NOINLINEs

Trac #10069 revealed that small NOINLINE functions didn't get split
into worker and wrapper. This was due to `certainlyWillInline`
saying that any unfoldings with a guidance of `UnfWhen` inline
unconditionally. That isn't the case for NOINLINE functions, so we
catch this case earlier now.

Nofib results:

--------------------------------------------------------------------------------
        Program         Allocs    Instrs
--------------------------------------------------------------------------------
 fannkuch-redux          -0.3%      0.0%
             gg          +0.0%     +0.1%
       maillist          -0.2%     -0.2%
        minimax           0.0%     -0.8%
--------------------------------------------------------------------------------
            Min          -0.3%     -0.8%
            Max          +0.0%     +0.1%
 Geometric Mean          -0.0%     -0.0%

Fixes #10069.

-------------------------
Metric Increase:
    T9233
-------------------------
parent 068b7e98
Pipeline #3229 passed with stages
in 375 minutes and 14 seconds
......@@ -1118,13 +1118,14 @@ smallEnoughToInline _ _
----------------
certainlyWillInline :: DynFlags -> IdInfo -> Maybe Unfolding
-- Sees if the unfolding is pretty certain to inline
-- If so, return a *stable* unfolding for it, that will always inline
-- ^ Sees if the unfolding is pretty certain to inline.
-- If so, return a *stable* unfolding for it, that will always inline.
certainlyWillInline dflags fn_info
= case unfoldingInfo fn_info of
CoreUnfolding { uf_tmpl = e, uf_guidance = g }
| loop_breaker -> Nothing -- Won't inline, so try w/w
| otherwise -> do_cunf e g -- Depends on size, so look at that
| loop_breaker -> Nothing -- Won't inline, so try w/w
| noinline -> Nothing -- See Note [Worker-wrapper for NOINLINE functions]
| otherwise -> do_cunf e g -- Depends on size, so look at that
DFunUnfolding {} -> Just fn_unf -- Don't w/w DFuns; it never makes sense
-- to do so, and even if it is currently a
......@@ -1134,6 +1135,7 @@ certainlyWillInline dflags fn_info
where
loop_breaker = isStrongLoopBreaker (occInfo fn_info)
noinline = inlinePragmaSpec (inlinePragInfo fn_info) == NoInline
fn_unf = unfoldingInfo fn_info
do_cunf :: CoreExpr -> UnfoldingGuidance -> Maybe Unfolding
......@@ -1148,9 +1150,6 @@ certainlyWillInline dflags fn_info
-- See Note [certainlyWillInline: INLINABLE]
do_cunf expr (UnfIfGoodArgs { ug_size = size, ug_args = args })
| not (null args) -- See Note [certainlyWillInline: be careful of thunks]
, case inlinePragmaSpec (inlinePragInfo fn_info) of
NoInline -> False -- NOINLINE; do not say certainlyWillInline!
_ -> True -- INLINE, INLINABLE, or nothing
, not (isBottomingSig (strictnessInfo fn_info))
-- Do not unconditionally inline a bottoming functions even if
-- it seems smallish. We've carefully lifted it out to top level,
......
......@@ -242,6 +242,37 @@ NOINLINE pragma to the worker.
(See Trac #13143 for a real-world example.)
It is crucial that we do this for *all* NOINLINE functions. Trac #10069
demonstrates what happens when we promise to w/w a (NOINLINE) leaf function, but
fail to deliver:
data C = C Int# Int#
{-# NOINLINE c1 #-}
c1 :: C -> Int#
c1 (C _ n) = n
{-# NOINLINE fc #-}
fc :: C -> Int#
fc c = 2 *# c1 c
Failing to w/w `c1`, but still w/wing `fc` leads to the following code:
c1 :: C -> Int#
c1 (C _ n) = n
$wfc :: Int# -> Int#
$wfc n = let c = C 0# n in 2 #* c1 c
fc :: C -> Int#
fc (C _ n) = $wfc n
Yikes! The reboxed `C` in `$wfc` can't cancel out, so we are in a bad place.
This generalises to any function that derives its strictness signature from
its callees, so we have to make sure that when a function announces particular
strictness properties, we have to w/w them accordingly, even if it means
splitting a NOINLINE function.
Note [Worker activation]
~~~~~~~~~~~~~~~~~~~~~~~~
Follows on from Note [Worker-wrapper for INLINABLE functions]
......
......@@ -58,7 +58,7 @@ test('T5654b-O0', [only_ways(['prof'])], compile_and_run, [''])
test('T5654b-O1', [only_ways(['profasm'])], compile_and_run, [''])
test('scc005', [], compile_and_run, [''])
test('scc005', [], compile_and_run, ['-fno-worker-wrapper'])
test('T5314', [extra_ways(extra_prof_ways)], compile_and_run, [''])
......
......@@ -8,7 +8,7 @@ g (p,q) = p+q
f :: Int -> Int -> Int -> Int
f x p q
= g (let j y = (p,q)
= g (let j y = (y+p,q)
{-# NOINLINE j #-}
in
case x of
......
{- Arity: 1, HasNoCafRefs, Strictness: <S,1*U()>m,
{- Arity: 1, HasNoCafRefs, Strictness: <S,1*H>,
Unfolding: InlineRule (0, True, True)
bof `cast` (Sym (N:Foo[0]) ->_R <T>_R) -}
......@@ -20,15 +20,7 @@ T7360.$WFoo3
-- RHS size: {terms: 5, types: 2, coercions: 0, joins: 0/0}
fun1 [InlPrag=NOINLINE] :: Foo -> ()
[GblId,
Arity=1,
Caf=NoCafRefs,
Str=<S,1*U>,
Unf=Unf{Src=InlineStable, TopLvl=True, Value=True, ConLike=True,
WorkFree=True, Expandable=True,
Guidance=ALWAYS_IF(arity=1,unsat_ok=True,boring_ok=True)
Tmpl= \ (x [Occ=Once] :: Foo) ->
case x of { __DEFAULT -> GHC.Tuple.() }}]
[GblId, Arity=1, Caf=NoCafRefs, Str=<S,1*U>, Unf=OtherCon []]
fun1 = \ (x :: Foo) -> case x of { __DEFAULT -> GHC.Tuple.() }
-- RHS size: {terms: 2, types: 0, coercions: 0, joins: 0/0}
......
......@@ -21,7 +21,7 @@ test('simplrun009', normal, compile_and_run, [''])
test('simplrun010', [extra_run_opts('24 16 8 +RTS -M10m -RTS'),
exit_code(251)]
, compile_and_run, [''])
test('simplrun011', normal, compile_and_run, [''])
test('simplrun011', normal, compile_and_run, ['-fno-worker-wrapper'])
# Really we'd like to run T2486 too, to check that its
# runtime has not gone up, but here I just compile it so that
......
......@@ -9,12 +9,4 @@ def f( name, opts ):
setTestOpts(f)
def checkStgString(needle):
def norm(str):
if needle in str:
return "%s contained in -ddump-simpl\n" % needle
else:
return "%s not contained in -ddump-simpl\n" % needle
return normalise_errmsg_fun(norm)
test('T13588', [ checkStgString('case') ] , compile, ['-dverbose-stg2stg'])
test('T13588', [ grep_errmsg('case') ] , compile, ['-dverbose-stg2stg -fno-worker-wrapper'])
module T10069 where
data C = C !Int !Int
{-# NOINLINE c1 #-}
c1 :: C -> Int
c1 (C _ c) = c
{-# NOINLINE fc #-}
fc :: C -> Int
fc c = c1 c + c1 c
T10069.$wc1 [InlPrag=NOINLINE] :: GHC.Prim.Int# -> GHC.Prim.Int#
......@@ -48,3 +48,4 @@ test('T13077a', normal, compile, [''])
test('T15627', [ grep_errmsg(r'(wmutVar|warray).*Int#') ], compile, ['-dppr-cols=200 -ddump-simpl'])
test('T16029', normal, makefile_test, [])
test('T10069', [ grep_errmsg(r'(wc1).*Int#$') ], compile, ['-dppr-cols=200 -ddump-simpl'])
......@@ -611,7 +611,13 @@ gen_wrappers (Info _ entries)
= "{-# LANGUAGE MagicHash, NoImplicitPrelude, UnboxedTuples #-}\n"
-- Dependencies on Prelude must be explicit in libraries/base, but we
-- don't need the Prelude here so we add NoImplicitPrelude.
++ "{-# OPTIONS_GHC -Wno-deprecations #-}\n"
++ "{-# OPTIONS_GHC -Wno-deprecations -O0 #-}\n"
-- No point in optimising this at all.
-- Performing WW on this module is harmful even, two reasons:
-- 1. Inferred strictness signatures are all bottom, which is a lie
-- 2. Doing the worker/wrapper split based on that information will
-- introduce references to Control.Exception.Base.absentError,
-- which isn't available at this point.
++ "module GHC.PrimopWrappers where\n"
++ "import qualified GHC.Prim\n"
++ "import GHC.Tuple ()\n"
......
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