[Git][ghc/ghc][ghc-9.0] 6 commits: Allow unsaturated runRW# applications
Ben Gamari
gitlab at gitlab.haskell.org
Tue Aug 18 04:58:45 UTC 2020
Ben Gamari pushed to branch ghc-9.0 at Glasgow Haskell Compiler / GHC
Commits:
57fd3ff0 by Ben Gamari at 2020-08-16T12:18:11-04:00
Allow unsaturated runRW# applications
Previously we had a very aggressive Core Lint check which caught
unsaturated applications of runRW#. However, there is nothing
wrong with such applications and they may naturally arise in desugared
Core. For instance, the desugared Core of Data.Primitive.Array.runArray#
from the `primitive` package contains:
case ($) (runRW# @_ @_) (\s -> ...) of ...
In this case it's almost certain that ($) will be inlined, turning the
application into a saturated application. However, even if this weren't
the case there isn't a problem: CorePrep (after deleting an unnecessary
case) can simply generate code in its usual way, resulting in a call to
the Haskell definition of runRW#.
Fixes #18291.
(cherry picked from commit 2f0bae734e2dc8737fbbb8465de7ded89c1121b6)
- - - - -
0af2db18 by Ben Gamari at 2020-08-16T12:18:19-04:00
testsuite: Add test for #18291
(cherry picked from commit 6c7785f8e17a43a2578366134f74fd1989077b73)
- - - - -
614ac76d by Ben Gamari at 2020-08-17T15:14:35-04:00
Clean up TBDs in changelog
(cherry picked from commit 4f334120c8e9cc4aefcbf11d99f169f648af9fde)
- - - - -
1a54d708 by Ben Gamari at 2020-08-17T15:14:35-04:00
Bump bytestring submodule
- - - - -
20e19811 by Ben Gamari at 2020-08-17T15:14:35-04:00
Bump binary submodule
- - - - -
8c7e8e1c by Ben Gamari at 2020-08-17T20:09:30+00:00
Bump Cabal submodule
- - - - -
15 changed files:
- compiler/GHC/Core/Lint.hs
- compiler/GHC/Core/Opt/Simplify.hs
- compiler/GHC/CoreToStg/Prep.hs
- libraries/Cabal
- libraries/base/changelog.md
- libraries/binary
- libraries/bytestring
- libraries/ghc-prim/GHC/Magic.hs
- libraries/template-haskell/changelog.md
- + testsuite/tests/codeGen/should_compile/T18291.hs
- testsuite/tests/codeGen/should_compile/all.T
- utils/check-api-annotations/check-api-annotations.cabal
- utils/check-ppr/check-ppr.cabal
- utils/ghc-cabal/ghc-cabal.cabal
- utils/ghc-cabal/ghc.mk
Changes:
=====================================
compiler/GHC/Core/Lint.hs
=====================================
@@ -729,8 +729,6 @@ lintJoinLams join_arity enforce rhs
where
go 0 expr = lintCoreExpr expr
go n (Lam var body) = lintLambda var $ go (n-1) body
- -- N.B. join points can be cast. e.g. we consider ((\x -> ...) `cast` ...)
- -- to be a join point at join arity 1.
go n expr | Just bndr <- enforce -- Join point with too few RHS lambdas
= failWithL $ mkBadJoinArityMsg bndr join_arity n rhs
| otherwise -- Future join point, not yet eta-expanded
@@ -779,36 +777,26 @@ hurts us here.
Note [Linting of runRW#]
~~~~~~~~~~~~~~~~~~~~~~~~
-runRW# has some very peculiar behavior (see Note [runRW magic] in
-GHC.CoreToStg.Prep) which CoreLint must accommodate.
+runRW# has some very special behavior (see Note [runRW magic] in
+GHC.CoreToStg.Prep) which CoreLint must accommodate, by allowing
+join points in its argument. For example, this is fine:
-As described in Note [Casts and lambdas] in
-GHC.Core.Opt.Simplify.Utils, the simplifier pushes casts out of
-lambdas. Concretely, the simplifier will transform
+ join j x = ...
+ in runRW# (\s. case v of
+ A -> j 3
+ B -> j 4)
- runRW# @r @ty (\s -> expr `cast` co)
+Usually those calls to the join point 'j' would not be valid tail calls,
+because they occur in a function argument. But in the case of runRW#
+they are fine, because runRW# (\s.e) behaves operationally just like e.
+(runRW# is ultimately inlined in GHC.CoreToStg.Prep.)
-into
-
- runRW# @r @ty ((\s -> expr) `cast` co)
-
-Consequently we need to handle the case that the continuation is a
-cast of a lambda. See Note [Casts and lambdas] in
-GHC.Core.Opt.Simplify.Utils.
-
-In the event that the continuation is headed by a lambda (which
-will bind the State# token) we can safely allow calls to join
-points since CorePrep is going to apply the continuation to
-RealWorld.
-
-In the case that the continuation is not a lambda we lint the
-continuation disallowing join points, to rule out things like,
+In the case that the continuation is /not/ a lambda we simply disable this
+special behaviour. For example, this is /not/ fine:
join j = ...
- in runRW# @r @ty (
- let x = jump j
- in x
- )
+ in runRW# @r @ty (jump j)
+
************************************************************************
@@ -929,10 +917,6 @@ lintCoreExpr e@(App _ _)
; (fun_ty2, ue2) <- lintCoreArg fun_pair1 arg_ty2
-- See Note [Linting of runRW#]
; let lintRunRWCont :: CoreArg -> LintM (LintedType, UsageEnv)
- lintRunRWCont (Cast expr co) = do
- (ty, ue) <- lintRunRWCont expr
- new_ty <- lintCastExpr expr ty co
- return (new_ty, ue)
lintRunRWCont expr@(Lam _ _) = do
lintJoinLams 1 (Just fun) expr
lintRunRWCont other = markAllJoinsBad $ lintCoreExpr other
@@ -941,10 +925,6 @@ lintCoreExpr e@(App _ _)
; app_ty <- lintValApp arg3 fun_ty2 arg3_ty ue2 ue3
; lintCoreArgs app_ty rest }
- | Var fun <- fun
- , fun `hasKey` runRWKey
- = failWithL (text "Invalid runRW# application")
-
| otherwise
= do { pair <- lintCoreFun fun (length args)
; lintCoreArgs pair args }
=====================================
compiler/GHC/Core/Opt/Simplify.hs
=====================================
@@ -1970,8 +1970,10 @@ rebuildCall env info (ApplyToTy { sc_arg_ty = arg_ty, sc_hole_ty = hole_ty, sc_c
= rebuildCall env (addTyArgTo info arg_ty hole_ty) cont
---------- The runRW# rule. Do this after absorbing all arguments ------
+-- See Note [Simplification of runRW#] in GHC.CoreToSTG.Prep.
+--
-- runRW# :: forall (r :: RuntimeRep) (o :: TYPE r). (State# RealWorld -> o) -> o
--- K[ runRW# rr ty body ] --> runRW rr' ty' (\s. K[ body s ])
+-- K[ runRW# rr ty body ] --> runRW rr' ty' (\s. K[ body s ])
rebuildCall env (ArgInfo { ai_fun = fun_id, ai_args = rev_args })
(ApplyToVal { sc_arg = arg, sc_env = arg_se
, sc_cont = cont, sc_hole_ty = fun_ty })
=====================================
compiler/GHC/CoreToStg/Prep.hs
=====================================
@@ -721,18 +721,6 @@ instance Outputable ArgInfo where
ppr (CpeCast co) = text "cast" <+> ppr co
ppr (CpeTick tick) = text "tick" <+> ppr tick
-{-
- Note [runRW arg]
-~~~~~~~~~~~~~~~~~~~
-If we got, say
- runRW# (case bot of {})
-which happened in #11291, we do /not/ want to turn it into
- (case bot of {}) realWorldPrimId#
-because that gives a panic in CoreToStg.myCollectArgs, which expects
-only variables in function position. But if we are sure to make
-runRW# strict (which we do in GHC.Types.Id.Make), this can't happen
--}
-
cpeApp :: CorePrepEnv -> CoreExpr -> UniqSM (Floats, CpeRhs)
-- May return a CpeRhs because of saturating primops
cpeApp top_env expr
@@ -798,10 +786,6 @@ cpeApp top_env expr
_ -> cpe_app env arg (CpeApp (Var realWorldPrimId) : rest) (n-1)
-- TODO: What about casts?
- cpe_app _env (Var f) args n
- | f `hasKey` runRWKey
- = pprPanic "cpe_app(runRW#)" (ppr args $$ ppr n)
-
cpe_app env (Var v) args depth
= do { v1 <- fiddleCCall v
; let e2 = lookupCorePrepEnv env v1
@@ -923,34 +907,96 @@ optimization (right before lowering to STG, in CorePrep), we can ensure that
no further floating will occur. This allows us to safely inline things like
@runST@, which are otherwise needlessly expensive (see #10678 and #5916).
-'runRW' is defined (for historical reasons) in GHC.Magic, with a NOINLINE
-pragma. It is levity-polymorphic.
+'runRW' has a variety of quirks:
+
+ * 'runRW' is known-key with a NOINLINE definition in
+ GHC.Magic. This definition is used in cases where runRW is curried.
+
+ * In addition to its normal Haskell definition in GHC.Magic, we give it
+ a special late inlining here in CorePrep and GHC.CoreToByteCode, avoiding
+ the incorrect sharing due to float-out noted above.
+
+ * It is levity-polymorphic:
runRW# :: forall (r1 :: RuntimeRep). (o :: TYPE r)
=> (State# RealWorld -> (# State# RealWorld, o #))
- -> (# State# RealWorld, o #)
+ -> (# State# RealWorld, o #)
+
+ * It has some special simplification logic to allow unboxing of results when
+ runRW# appears in a strict context. See Note [Simplification of runRW#]
+ below.
+
+ * Since its body is inlined, we allow runRW#'s argument to contain jumps to
+ join points. That is, the following is allowed:
+
+ join j x = ...
+ in runRW# @_ @_ (\s -> ... jump j 42 ...)
+
+ The Core Linter knows about this. See Note [Linting of runRW#] in
+ GHC.Core.Lint for details.
+
+ The occurrence analyser and SetLevels also know about this, as described in
+ Note [Simplification of runRW#].
+
+Other relevant Notes:
-It's correctness needs no special treatment in GHC except this special inlining
-here in CorePrep (and in GHC.CoreToByteCode).
+ * Note [Simplification of runRW#] below, describing a transformation of runRW
+ applications in strict contexts performed by the simplifier.
+ * Note [Linting of runRW#] in GHC.Core.Lint
+ * Note [runRW arg] below, describing a non-obvious case where the
+ late-inlining could go wrong.
-However, there are a variety of optimisation opportunities that the simplifier
-takes advantage of. See Note [Simplification of runRW#].
+
+ Note [runRW arg]
+~~~~~~~~~~~~~~~~~~~
+Consider the Core program (from #11291),
+
+ runRW# (case bot of {})
+
+The late inlining logic in cpe_app would transform this into:
+
+ (case bot of {}) realWorldPrimId#
+
+Which would rise to a panic in CoreToStg.myCollectArgs, which expects only
+variables in function position.
+
+However, as runRW#'s strictness signature captures the fact that it will call
+its argument this can't happen: the simplifier will transform the bottoming
+application into simply (case bot of {}).
+
+Note that this reasoning does *not* apply to non-bottoming continuations like:
+
+ hello :: Bool -> Int
+ hello n =
+ runRW# (
+ case n of
+ True -> \s -> 23
+ _ -> \s -> 10)
+
+Why? The difference is that (case bot of {}) is considered by okCpeArg to be
+trivial, consequently cpeArg (which the catch-all case of cpe_app calls on both
+the function and the arguments) will forgo binding it to a variable. By
+contrast, in the non-bottoming case of `hello` above the function will be
+deemed non-trivial and consequently will be case-bound.
Note [Simplification of runRW#]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Consider the program,
- case runRW# (\s -> let n = I# 42# in n) of
+ case runRW# (\s -> I# 42#) of
I# n# -> f n#
There is no reason why we should allocate an I# constructor given that we
-immediately destructure it. To avoid this the simplifier will push strict
-contexts into runRW's continuation. That is, it transforms
+immediately destructure it.
+
+To avoid this the simplifier has a special transformation rule, specific to
+runRW#, that pushes a strict context into runRW#'s continuation. See the
+`runRW#` guard in `GHC.Core.Opt.Simplify.rebuildCall`. That is, it transforms
K[ runRW# @r @ty cont ]
~>
- runRW# @r @ty K[cont]
+ runRW# @r @ty (\s -> K[cont s])
This has a few interesting implications. Consider, for instance, this program:
@@ -969,15 +1015,29 @@ Performing the transform described above would result in:
If runRW# were a "normal" function this call to join point j would not be
allowed in its continuation argument. However, since runRW# is inlined (as
described in Note [runRW magic] above), such join point occurences are
-completely fine. Both occurrence analysis and Core Lint have special treatment
-for runRW# applications. See Note [Linting of runRW#] for details on the latter.
+completely fine. Both occurrence analysis (see the runRW guard in occAnalApp)
+and Core Lint (see the App case of lintCoreExpr) have special treatment for
+runRW# applications. See Note [Linting of runRW#] for details on the latter.
Moreover, it's helpful to ensure that runRW's continuation isn't floated out
-(since doing so would then require a call, whereas we would otherwise end up
-with straight-line). Consequently, GHC.Core.Opt.SetLevels.lvlApp has special
-treatment for runRW# applications, ensure the arguments are not floated if
+For instance, if we have
+
+ runRW# (\s -> do_something)
+
+where do_something contains only top-level free variables, we may be tempted to
+float the argument to the top-level. However, we must resist this urge as since
+doing so would then require that runRW# produce an allocation and call, e.g.:
+
+ let lvl = \s -> do_somethign
+ in
+ ....(runRW# lvl)....
+
+whereas without floating the inlining of the definition of runRW would result
+in straight-line code. Consequently, GHC.Core.Opt.SetLevels.lvlApp has special
+treatment for runRW# applications, ensure the arguments are not floated as
MFEs.
+
Other considered designs
------------------------
=====================================
libraries/Cabal
=====================================
@@ -1 +1 @@
-Subproject commit 32dad5c1cf70d65ecb93b0ec214445cf9c9f6615
+Subproject commit 1d886476c443b227bf93eba62781a6cad5012d9e
=====================================
libraries/base/changelog.md
=====================================
@@ -30,7 +30,7 @@
* Add `Ix` instances for tuples of size 6 through 15
-## 4.14.0.0 *TBA*
+## 4.14.0.0 *Jan 2020
* Bundled with GHC 8.10.1
* Add a `TestEquality` instance for the `Compose` newtype.
@@ -322,7 +322,7 @@
in constant space when applied to lists. (#10830)
* `mkFunTy`, `mkAppTy`, and `mkTyConApp` from `Data.Typeable` no longer exist.
- This functionality is superseded by the interfaces provided by
+ This functionality is superceded by the interfaces provided by
`Type.Reflection`.
* `mkTyCon3` is no longer exported by `Data.Typeable`. This function is
=====================================
libraries/binary
=====================================
@@ -1 +1 @@
-Subproject commit dfaf780596328c9184758452b78288e8f405fcc1
+Subproject commit f9b1c92a2ff34cc3457fa27faf3e16b8203b4b9f
=====================================
libraries/bytestring
=====================================
@@ -1 +1 @@
-Subproject commit e6cb01e2ec0bfdd19298418c85f220925a9fa307
+Subproject commit e043aacfc4202a59ccae8b8c8cf0e1ad83a3f209
=====================================
libraries/ghc-prim/GHC/Magic.hs
=====================================
@@ -122,7 +122,7 @@ oneShot f = f
runRW# :: forall (r :: RuntimeRep) (o :: TYPE r).
(State# RealWorld -> o) -> o
--- See Note [runRW magic] in CorePrep
+-- See Note [runRW magic] in GHC.CoreToStg.Prep.
{-# NOINLINE runRW# #-} -- runRW# is inlined manually in CorePrep
#if !defined(__HADDOCK_VERSION__)
runRW# m = m realWorld#
=====================================
libraries/template-haskell/changelog.md
=====================================
@@ -34,7 +34,9 @@
* The argument to `TExpQ` can now be levity polymorphic.
-## 2.16.0.0 *TBA*
+## 2.16.0.0 *Jan 2020*
+
+ * Bundled with GHC 8.10.1
* Add support for tuple sections. (#15843) The type signatures of `TupE` and
`UnboxedTupE` have changed from `[Exp] -> Exp` to `[Maybe Exp] -> Exp`.
@@ -58,6 +60,8 @@
## 2.15.0.0 *May 2019*
+ * Bundled with GHC 8.8.1
+
* In `Language.Haskell.TH.Syntax`, `DataInstD`, `NewTypeInstD`, `TySynEqn`,
and `RuleP` now all have a `Maybe [TyVarBndr]` argument, which contains a
list of quantified type variables if an explicit `forall` is present, and
@@ -80,6 +84,8 @@
## 2.14.0.0 *September 2018*
+ * Bundled with GHC 8.6.1
+
* Introduce an `addForeignFilePath` function, as well as a corresponding
`qAddForeignFile` class method to `Quasi`. Unlike `addForeignFile`, which
takes the contents of the file as an argument, `addForeignFilePath` takes
=====================================
testsuite/tests/codeGen/should_compile/T18291.hs
=====================================
@@ -0,0 +1,7 @@
+{-# LANGUAGE MagicHash #-}
+module T18291 where
+
+import GHC.Magic
+
+hi :: Int
+hi = runRW# $ \_ -> 42
=====================================
testsuite/tests/codeGen/should_compile/all.T
=====================================
@@ -91,6 +91,7 @@ test('T17648', normal, makefile_test, [])
test('T17904', normal, compile, ['-O'])
test('T18227A', normal, compile, [''])
test('T18227B', normal, compile, [''])
+test('T18291', normal, compile, ['-O0'])
test('T15570',
when(unregisterised(), skip),
compile, ['-Wno-overflowed-literals'])
=====================================
utils/check-api-annotations/check-api-annotations.cabal
=====================================
@@ -24,6 +24,6 @@ Executable check-api-annotations
Build-Depends: base >= 4 && < 5,
containers,
- Cabal >= 3.0 && < 3.4,
+ Cabal >= 3.0 && < 3.6,
directory,
ghc
=====================================
utils/check-ppr/check-ppr.cabal
=====================================
@@ -25,7 +25,7 @@ Executable check-ppr
Build-Depends: base >= 4 && < 5,
bytestring,
containers,
- Cabal >= 3.0 && < 3.4,
+ Cabal >= 3.0 && < 3.6,
directory,
filepath,
ghc
=====================================
utils/ghc-cabal/ghc-cabal.cabal
=====================================
@@ -21,6 +21,6 @@ Executable ghc-cabal
Build-Depends: base >= 3 && < 5,
bytestring >= 0.10 && < 0.11,
- Cabal >= 3.0 && < 3.4,
+ Cabal >= 3.2 && < 3.6,
directory >= 1.1 && < 1.4,
filepath >= 1.2 && < 1.5
=====================================
utils/ghc-cabal/ghc.mk
=====================================
@@ -37,25 +37,13 @@ ifneq "$(BINDIST)" "YES"
$(ghc-cabal_INPLACE) : $(ghc-cabal_DIST_BINARY) | $$(dir $$@)/.
"$(CP)" $< $@
-# Minor hack, since we can't reuse the `hs-suffix-rules-srcdir` macro
-ifneq ($(wildcard libraries/Cabal/Cabal/Distribution/Fields/Lexer.x),)
-# Lexer.x exists so we have to call Alex ourselves
-CABAL_LEXER_DEP := bootstrapping/Cabal/Distribution/Fields/Lexer.hs
-
-bootstrapping/Cabal/Distribution/Fields/Lexer.hs: libraries/Cabal/Cabal/Distribution/Fields/Lexer.x
- mkdir -p bootstrapping/Cabal/Distribution/Fields
- $(call cmd,ALEX) $< -o $@
-else
-CABAL_LEXER_DEP := libraries/Cabal/Cabal/Distribution/Fields/Lexer.hs
-endif
-
-$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/Distribution/*/*/*.hs)
-$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/Distribution/*/*.hs)
-$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/Distribution/*.hs)
+$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/src/Distribution/*/*/*.hs)
+$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/src/Distribution/*/*.hs)
+$(ghc-cabal_DIST_BINARY): $(wildcard libraries/Cabal/Cabal/src/Distribution/*.hs)
# N.B. Compile with -O0 since this is not a performance-critical executable
# and the Cabal takes nearly twice as long to build with -O1. See #16817.
-$(ghc-cabal_DIST_BINARY): $(CABAL_LEXER_DEP) utils/ghc-cabal/Main.hs $(TOUCH_DEP) | $$(dir $$@)/. bootstrapping/.
+$(ghc-cabal_DIST_BINARY): utils/ghc-cabal/Main.hs $(TOUCH_DEP) | $$(dir $$@)/. bootstrapping/.
"$(GHC)" $(SRC_HC_OPTS) \
$(addprefix -optc, $(SRC_CC_OPTS) $(CONF_CC_OPTS_STAGE0)) \
$(addprefix -optl, $(SRC_LD_OPTS) $(CONF_GCC_LINKER_OPTS_STAGE0)) \
@@ -69,8 +57,7 @@ $(ghc-cabal_DIST_BINARY): $(CABAL_LEXER_DEP) utils/ghc-cabal/Main.hs $(TOUCH_DEP
-DBOOTSTRAPPING \
-odir bootstrapping \
-hidir bootstrapping \
- $(CABAL_LEXER_DEP) \
- -ilibraries/Cabal/Cabal \
+ -ilibraries/Cabal/Cabal/src \
-ilibraries/binary/src \
-ilibraries/filepath \
-ilibraries/hpc \
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/ce32390cd4221e2721c18f89bd6d08e3b8b884dc...8c7e8e1c802791dda0f1d30c7658c995a8349c51
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/ce32390cd4221e2721c18f89bd6d08e3b8b884dc...8c7e8e1c802791dda0f1d30c7658c995a8349c51
You're receiving this email because of your account on gitlab.haskell.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20200818/3080ce7f/attachment-0001.html>
More information about the ghc-commits
mailing list