diff options
author | Sebastian Graf <sebastian.graf@kit.edu> | 2021-01-07 18:35:29 +0100 |
---|---|---|
committer | Marge Bot <ben+marge-bot@smart-cactus.org> | 2021-01-10 23:34:59 -0500 |
commit | 1a220bcf704f6ce1404516a5b647246a20d34f6e (patch) | |
tree | 77250a8b8386ce3d6eb5e6f86f1cefabdd6269cd | |
parent | 4bb957de6e32722d04f47ddd57b6010dbc7770d2 (diff) | |
download | haskell-1a220bcf704f6ce1404516a5b647246a20d34f6e.tar.gz |
WorkWrap: Use SysLocal Name for Thunk Splitting (#19180)
Since !4493 we annotate top-level bindings with demands, which leads to
novel opportunities for thunk splitting absent top-level thunks.
It turns out that thunk splitting wasn't quite equipped for that,
because it re-used top-level, `External` Names for local helper Ids.
That triggered a CoreLint error (#19180), reproducible with `T19180`.
Fixed by adjusting the thunk splitting code to produce `SysLocal` names
for the local bindings.
Fixes #19180.
Metric Decrease:
T12227
T18282
-rw-r--r-- | compiler/GHC/Core/Opt/WorkWrap.hs | 58 | ||||
-rw-r--r-- | testsuite/tests/stranal/should_compile/T19180.hs | 20 | ||||
-rw-r--r-- | testsuite/tests/stranal/should_compile/all.T | 2 |
3 files changed, 67 insertions, 13 deletions
diff --git a/compiler/GHC/Core/Opt/WorkWrap.hs b/compiler/GHC/Core/Opt/WorkWrap.hs index 68d3e314a3..4ea61f3e85 100644 --- a/compiler/GHC/Core/Opt/WorkWrap.hs +++ b/compiler/GHC/Core/Opt/WorkWrap.hs @@ -781,27 +781,59 @@ then the splitting will go deeper too. NB: For recursive thunks, the Simplifier is unable to float `x-rhs` out of `x*`'s RHS, because `x*` occurs freely in `x-rhs`, and will just change it back to the original definition, so we just split non-recursive thunks. + +Note [Thunk splitting for top-level binders] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Top-level bindings are never strict. Yet they can be absent, as T14270 shows: + + module T14270 (mkTrApp) where + mkTrApp x y + | Just ... <- ... typeRepKind x ... + = undefined + | otherwise + = undefined + typeRepKind = Tick scc undefined + +(T19180 is a profiling-free test case for this) +Note that `typeRepKind` is not exported and its only use site in +`mkTrApp` guards a bottoming expression. Thus, demand analysis +figures out that `typeRepKind` is absent and splits the thunk to + + typeRepKind = + let typeRepKind = Tick scc undefined in + let typeRepKind = absentError in + typeRepKind + +But now we have a local binding with an External Name +(See Note [About the NameSorts]). That will trigger a CoreLint error, which we +get around by localising the Id for the auxiliary bindings in 'splitThunk'. -} --- See Note [Thunk splitting] +-- | See Note [Thunk splitting]. +-- -- splitThunk converts the *non-recursive* binding -- x = e -- into --- x = let x = e --- in case x of --- I# y -> let x = I# y in x } +-- x = let x' = e in +-- case x' of I# y -> let x' = I# y in x' -- See comments above. Is it not beautifully short? -- Moreover, it works just as well when there are -- several binders, and if the binders are lifted -- E.g. x = e --- --> x = let x = e in --- case x of (a,b) -> let x = (a,b) in x - +-- --> x = let x' = e in +-- case x' of (a,b) -> let x' = (a,b) in x' +-- Here, x' is a localised version of x, in case x is a +-- top-level Id with an External Name, because Lint rejects local binders with +-- External Names; see Note [About the NameSorts] in GHC.Types.Name. +-- +-- How can we do thunk-splitting on a top-level binder? See +-- Note [Thunk splitting for top-level binders]. splitThunk :: DynFlags -> FamInstEnvs -> RecFlag -> Var -> Expr Var -> UniqSM [(Var, Expr Var)] -splitThunk dflags fam_envs is_rec fn_id rhs - = ASSERT(not (isJoinId fn_id)) - do { (useful,_, wrap_fn, work_fn) <- mkWWstr dflags fam_envs False [fn_id] - ; let res = [ (fn_id, Let (NonRec fn_id rhs) (wrap_fn (work_fn (Var fn_id)))) ] - ; if useful then ASSERT2( isNonRec is_rec, ppr fn_id ) -- The thunk must be non-recursive +splitThunk dflags fam_envs is_rec x rhs + = ASSERT(not (isJoinId x)) + do { let x' = localiseId x -- See comment above + ; (useful,_, wrap_fn, work_fn) <- mkWWstr dflags fam_envs False [x'] + ; let res = [ (x, Let (NonRec x' rhs) (wrap_fn (work_fn (Var x')))) ] + ; if useful then ASSERT2( isNonRec is_rec, ppr x ) -- The thunk must be non-recursive return res - else return [(fn_id, rhs)] } + else return [(x, rhs)] } diff --git a/testsuite/tests/stranal/should_compile/T19180.hs b/testsuite/tests/stranal/should_compile/T19180.hs new file mode 100644 index 0000000000..d0211ac4e3 --- /dev/null +++ b/testsuite/tests/stranal/should_compile/T19180.hs @@ -0,0 +1,20 @@ +{-# LANGUAGE ScopedTypeVariables #-} +{-# LANGUAGE PolyKinds #-} +module T14270 (mkTrApp) where + +import Data.Kind (Type) + +data TypeRep a = TypeRep + +mkTrApp :: TypeRep a -> TypeRep a +mkTrApp (x :: TypeRep x) + | Just _ <- isTYPE (typeRepKind x) + = undefined +mkTrApp x = TypeRep + +typeRepKind :: TypeRep (a :: k) -> TypeRep k +typeRepKind = if sum [0..100] == 10 then undefined else const TypeRep + +isTYPE :: TypeRep (a :: Type) -> Maybe a +isTYPE _ = if sum [0..100] == 10 then Nothing else undefined +{-# NOINLINE isTYPE #-} diff --git a/testsuite/tests/stranal/should_compile/all.T b/testsuite/tests/stranal/should_compile/all.T index 28c8154a77..3e77a602ae 100644 --- a/testsuite/tests/stranal/should_compile/all.T +++ b/testsuite/tests/stranal/should_compile/all.T @@ -64,3 +64,5 @@ test('T18894', [ grep_errmsg(r'Dmd=\S+C\S+') ], compile, ['-ddump-stranal -dsup test('T18894b', [ grep_errmsg(r'Arity=2') ], compile, ['-ddump-stranal -dsuppress-uniques -fno-call-arity -dppr-cols=200']) # We care about the workers of f,g,h,i: test('T18982', [ grep_errmsg(r'\$w. .*Int#$') ], compile, ['-dppr-cols=1000 -ddump-simpl -dsuppress-idinfo -dsuppress-uniques']) + +test('T19180', normal, compile, ['']) |