WorkWrap: Use SysLocal Name for Thunk Splitting (#19180)wip/T19180

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

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, [''])