[Git][ghc/ghc][wip/backports-8.8] 5 commits: testsuite: Add test for #18151
Ben Gamari
gitlab at gitlab.haskell.org
Wed Jul 8 19:02:55 UTC 2020
Ben Gamari pushed to branch wip/backports-8.8 at Glasgow Haskell Compiler / GHC
Commits:
eda32b7a by Ben Gamari at 2020-07-08T15:02:32-04:00
testsuite: Add test for #18151
- - - - -
2e17d60c by Ben Gamari at 2020-07-08T15:02:46-04:00
HsToCore: Eta expand left sections
Strangely, the comment next to this code already alluded to the fact
that even simply eta-expanding will sacrifice laziness. It's quite
unclear how we regressed so far.
See #18151.
(cherry picked from commit 2b89ca5b850b4097447cc4908cbb0631011ce979)
- - - - -
433341fb by Travis Whitaker at 2020-07-08T15:02:48-04:00
Build a threaded stage 1 if the bootstrapping GHC supports it.
(cherry picked from commit 67738db10010fd28a8e997b5c8f83ea591b88a0e)
- - - - -
98a053a9 by Ben Gamari at 2020-07-08T15:02:48-04:00
More release notes
- - - - -
1fdd6165 by GHC GitLab CI at 2020-07-08T15:02:48-04:00
Bump to 8.8.4, RELEASE=YES
- - - - -
13 changed files:
- compiler/deSugar/DsExpr.hs
- compiler/ghc.mk
- configure.ac
- docs/users_guide/8.8.4-notes.rst
- ghc/ghc.mk
- hadrian/cfg/system.config.in
- hadrian/src/Expression.hs
- hadrian/src/Oracles/Flag.hs
- hadrian/src/Settings/Packages.hs
- mk/config.mk.in
- + testsuite/tests/deSugar/should_run/T18151.hs
- + testsuite/tests/deSugar/should_run/T18151.stdout
- testsuite/tests/deSugar/should_run/all.T
Changes:
=====================================
compiler/deSugar/DsExpr.hs
=====================================
@@ -331,26 +331,47 @@ Then we get
That 'g' in the 'in' part is an evidence variable, and when
converting to core it must become a CO.
-Operator sections. At first it looks as if we can convert
-\begin{verbatim}
- (expr op)
-\end{verbatim}
-to
-\begin{verbatim}
- \x -> op expr x
-\end{verbatim}
+
+Note [Desugaring operator sections]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+At first it looks as if we can convert
+
+ (expr `op`)
+
+naively to
+
+ \x -> op expr x
But no! expr might be a redex, and we can lose laziness badly this
way. Consider
-\begin{verbatim}
- map (expr op) xs
-\end{verbatim}
-for example. So we convert instead to
-\begin{verbatim}
- let y = expr in \x -> op y x
-\end{verbatim}
-If \tr{expr} is actually just a variable, say, then the simplifier
-will sort it out.
+
+ map (expr `op`) xs
+
+for example. If expr were a redex then eta-expanding naively would
+result in multiple evaluations where the user might only have expected one.
+
+So we convert instead to
+
+ let y = expr in \x -> op y x
+
+Also, note that we must do this for both right and (perhaps surprisingly) left
+sections. Why are left sections necessary? Consider the program (found in #18151),
+
+ seq (True `undefined`) ()
+
+according to the Haskell Report this should reduce to () (as it specifies
+desugaring via eta expansion). However, if we fail to eta expand we will rather
+bottom. Consequently, we must eta expand even in the case of a left section.
+
+If `expr` is actually just a variable, say, then the simplifier
+will inline `y`, eliminating the redundant `let`.
+
+Note that this works even in the case that `expr` is unlifted. In this case
+bindNonRec will automatically do the right thing, giving us:
+
+ case expr of y -> (\x -> op y x)
+
+See #18151.
-}
ds_expr _ e@(OpApp _ e1 op e2)
@@ -359,17 +380,35 @@ ds_expr _ e@(OpApp _ e1 op e2)
; dsWhenNoErrs (mapM dsLExprNoLP [e1, e2])
(\exprs' -> mkCoreAppsDs (text "opapp" <+> ppr e) op' exprs') }
-ds_expr _ (SectionL _ expr op) -- Desugar (e !) to ((!) e)
- = do { op' <- dsLExpr op
- ; dsWhenNoErrs (dsLExprNoLP expr)
- (\expr' -> mkCoreAppDs (text "sectionl" <+> ppr expr) op' expr') }
-
--- dsLExpr (SectionR op expr) -- \ x -> op x expr
+-- dsExpr (SectionL op expr) === (expr `op`) ~> \y -> op expr y
+--
+-- See Note [Desugaring operator sections].
+-- N.B. this also must handle postfix operator sections due to -XPostfixOperators.
+ds_expr _ e@(SectionL _ expr op) = do
+ core_op <- dsLExpr op
+ x_core <- dsLExpr expr
+ case splitFunTys (exprType core_op) of
+ -- Binary operator section
+ (x_ty:y_ty:_, _) -> do
+ dsWhenNoErrs
+ (mapM newSysLocalDsNoLP [x_ty, y_ty])
+ (\[x_id, y_id] ->
+ bindNonRec x_id x_core
+ $ Lam y_id (mkCoreAppsDs (text "sectionl" <+> ppr e)
+ core_op [Var x_id, Var y_id]))
+
+ -- Postfix operator section
+ (_:_, _) -> do
+ return $ mkCoreAppDs (text "sectionl" <+> ppr e) core_op x_core
+
+ _ -> pprPanic "dsExpr(SectionL)" (ppr e)
+
+-- dsExpr (SectionR op expr) === (`op` expr) ~> \x -> op x expr
+--
+-- See Note [Desugaring operator sections].
ds_expr _ e@(SectionR _ op expr) = do
core_op <- dsLExpr op
- -- for the type of x, we need the type of op's 2nd argument
let (x_ty:y_ty:_, _) = splitFunTys (exprType core_op)
- -- See comment with SectionL
y_core <- dsLExpr expr
dsWhenNoErrs (mapM newSysLocalDsNoLP [x_ty, y_ty])
(\[x_id, y_id] -> bindNonRec y_id y_core $
=====================================
compiler/ghc.mk
=====================================
@@ -326,6 +326,12 @@ ifeq "$(GhcThreaded)" "YES"
compiler_stage2_CONFIGURE_OPTS += --ghc-option=-optc-DTHREADED_RTS
endif
+# If the bootstrapping GHC supplies the threaded RTS, then we can have a
+# threaded stage 1 too.
+ifeq "$(GhcThreadedRts)" "YES"
+compiler_stage1_CONFIGURE_OPTS += --ghc-option=-optc-DTHREADED_RTS
+endif
+
ifeq "$(GhcWithNativeCodeGen)" "YES"
compiler_stage1_CONFIGURE_OPTS += --flags=ncg
compiler_stage2_CONFIGURE_OPTS += --flags=ncg
=====================================
configure.ac
=====================================
@@ -13,10 +13,10 @@ dnl
# see what flags are available. (Better yet, read the documentation!)
#
-AC_INIT([The Glorious Glasgow Haskell Compilation System], [8.8.3], [glasgow-haskell-bugs at haskell.org], [ghc-AC_PACKAGE_VERSION])
+AC_INIT([The Glorious Glasgow Haskell Compilation System], [8.8.4], [glasgow-haskell-bugs at haskell.org], [ghc-AC_PACKAGE_VERSION])
# Set this to YES for a released version, otherwise NO
-: ${RELEASE=NO}
+: ${RELEASE=YES}
# The primary version (e.g. 7.5, 7.4.1) is set in the AC_INIT line
# above. If this is not a released version, then we will append the
@@ -127,6 +127,9 @@ dnl CC_STAGE0 is like the "previous" variable CC (inherited by CC_STAGE[123])
dnl but instead used by stage0 for bootstrapping stage1
AC_ARG_VAR(CC_STAGE0, [C compiler command (bootstrap)])
+dnl RTS ways supplied by the bootstrapping compiler.
+AC_ARG_VAR(RTS_WAYS_STAGE0, [RTS ways])
+
if test "$WithGhc" != ""; then
FPTOOLS_GHC_VERSION([GhcVersion], [GhcMajVersion], [GhcMinVersion], [GhcPatchLevel])dnl
@@ -150,6 +153,17 @@ if test "$WithGhc" != ""; then
BOOTSTRAPPING_GHC_INFO_FIELD([AR_STAGE0],[ar command])
BOOTSTRAPPING_GHC_INFO_FIELD([AR_OPTS_STAGE0],[ar flags])
BOOTSTRAPPING_GHC_INFO_FIELD([ArSupportsAtFile_STAGE0],[ar supports at file])
+ BOOTSTRAPPING_GHC_INFO_FIELD([RTS_WAYS_STAGE0],[RTS ways])
+
+ dnl Check whether or not the bootstrapping GHC has a threaded RTS. This
+ dnl determines whether or not we can have a threaded stage 1.
+ dnl See Note [Linking ghc-bin against threaded stage0 RTS] in
+ dnl hadrian/src/Settings/Packages.hs for details.
+ if echo ${RTS_WAYS_STAGE0} | grep '.*thr.*' 2>&1 >/dev/null; then
+ AC_SUBST(GhcThreadedRts, YES)
+ else
+ AC_SUBST(GhcThreadedRts, NO)
+ fi
fi
dnl ** Must have GHC to build GHC
@@ -1372,6 +1386,7 @@ Configure completed successfully.
echo "\
Bootstrapping using : $WithGhc
which is version : $GhcVersion
+ with threaded RTS? : $GhcThreadedRts
"
if test "x$CC_LLVM_BACKEND" = "x1"; then
=====================================
docs/users_guide/8.8.4-notes.rst
=====================================
@@ -6,13 +6,27 @@ Release notes for version 8.8.4
GHC 8.8.4 is a minor release intended to fix regressions and minor bugs in the
8.8.1, 8.8.2 and 8.8.3 releases.
+Like previous releases in the 8.8 series, the :ghc-flag:`LLVM backend <-fllvm>`
+of this release is to be used with LLVM 7.
+
Highlights
----------
-- Fix a bug in process creation on Windows (:ghc-ticket:`17926`).
+- Fixes a bug in process creation on Windows (:ghc-ticket:`17926`).
+
+- Works around a Linux kernel bug in the implementation of ``timerfd``\s (:ghc-ticket:`18033`).
+
+- Fixes a few linking issues affecting ARM
+
+- Fixes "missing interface file" error triggered by some uses of ``Ordering`` (:ghc-ticket:`18185`)
+
+- Fixes an integer overflow in the compact-normal-form import implementation (:ghc-ticket:`16992`)
+
+- ``configure`` now accepts ``--enable-numa`` flag to enable/disable ``numactl`` support on Linux.
-- Workaround a Linux kernel bug in the implementation of ``timerfd``\s (:ghc-ticket:`18033`).
+- Fixes potentially lost sharing due to the desugaring of left operator sections (:ghc-ticket:`18151`).
+- Fixes a build-system bug resulting in potential miscompilation by unregisteised compilers (:ghc-ticket:`18024`)
Known issues
------------
=====================================
ghc/ghc.mk
=====================================
@@ -63,6 +63,15 @@ ghc_stage2_MORE_HC_OPTS += -threaded
ghc_stage3_MORE_HC_OPTS += -threaded
endif
+# If stage 0 supplies a threaded RTS, we can use it for stage 1.
+# See Note [Linking ghc-bin against threaded stage0 RTS] in
+# hadrian/src/Settings/Packages.hs for details.
+ifeq "$(GhcThreadedRts)" "YES"
+ghc_stage1_MORE_HC_OPTS += -threaded
+else
+ghc_stage1_CONFIGURE_OPTS += -f-threaded
+endif
+
ifeq "$(GhcProfiled)" "YES"
ghc_stage2_PROGRAM_WAY = p
endif
=====================================
hadrian/cfg/system.config.in
=====================================
@@ -77,6 +77,8 @@ ghc-major-version = @GhcMajVersion@
ghc-minor-version = @GhcMinVersion@
ghc-patch-level = @GhcPatchLevel@
+bootstrap-threaded-rts = @GhcThreadedRts@
+
supports-this-unit-id = @SUPPORTS_THIS_UNIT_ID@
project-name = @ProjectName@
=====================================
hadrian/src/Expression.hs
=====================================
@@ -6,8 +6,8 @@ module Expression (
expr, exprIO, arg, remove,
-- ** Predicates
- (?), stage, stage0, stage1, stage2, notStage0, package, notPackage,
- libraryPackage, builder, way, input, inputs, output, outputs,
+ (?), stage, stage0, stage1, stage2, notStage0, threadedBootstrapper,
+ package, notPackage, libraryPackage, builder, way, input, inputs, output, outputs,
-- ** Evaluation
interpret, interpretInContext,
@@ -26,6 +26,7 @@ import Base
import Builder
import Context hiding (stage, package, way)
import Expression.Type
+import Oracles.Flag
import Hadrian.Expression hiding (Expr, Predicate, Args)
import Hadrian.Haskell.Cabal.Type
import Hadrian.Oracles.Cabal
@@ -83,6 +84,19 @@ instance BuilderPredicate a => BuilderPredicate (FilePath -> a) where
way :: Way -> Predicate
way w = (w ==) <$> getWay
+{-
+Note [Stage Names]
+~~~~~~~~~~~~~~~~~~
+
+Code referring to specific stages can be a bit tricky. In Hadrian, the stages
+have the same names they carried in the autoconf build system, but they are
+often referred to by the stage used to construct them. For example, the stage 1
+artifacts will be placed in _build/stage0, because they are constructed by the
+stage 0 compiler. The stage predicates in this module behave the same way,
+'stage0' will return 'True' while stage 0 is being used to build the stage 1
+compiler.
+-}
+
-- | Is the build currently in stage 0?
stage0 :: Predicate
stage0 = stage Stage0
@@ -99,6 +113,13 @@ stage2 = stage Stage2
notStage0 :: Predicate
notStage0 = notM stage0
+-- | Whether or not the bootstrapping compiler provides a threaded RTS. We need
+-- to know this when building stage 1, since stage 1 links against the
+-- compiler's RTS ways. See Note [Linking ghc-bin against threaded stage0 RTS]
+-- in Settings.Packages for details.
+threadedBootstrapper :: Predicate
+threadedBootstrapper = expr (flag BootstrapThreadedRts)
+
-- | Is a certain package /not/ built right now?
notPackage :: Package -> Predicate
notPackage = notM . package
=====================================
hadrian/src/Oracles/Flag.hs
=====================================
@@ -21,6 +21,7 @@ data Flag = ArSupportsAtFile
| WithLibdw
| HaveLibMingwEx
| UseSystemFfi
+ | BootstrapThreadedRts
-- Note, if a flag is set to empty string we treat it as set to NO. This seems
-- fragile, but some flags do behave like this, e.g. GccIsClang.
@@ -39,6 +40,7 @@ flag f = do
WithLibdw -> "with-libdw"
HaveLibMingwEx -> "have-lib-mingw-ex"
UseSystemFfi -> "use-system-ffi"
+ BootstrapThreadedRts -> "bootstrap-threaded-rts"
value <- lookupValueOrError configFile key
when (value `notElem` ["YES", "NO", ""]) . error $ "Configuration flag "
++ quote (key ++ " = " ++ value) ++ " cannot be parsed."
=====================================
hadrian/src/Settings/Packages.hs
=====================================
@@ -85,10 +85,24 @@ packageArgs = do
, builder (Cabal Flags) ? mconcat
[ ghcWithInterpreter ? notStage0 ? arg "ghci"
, flag CrossCompiling ? arg "-terminfo"
- -- the 'threaded' flag is True by default, but
- -- let's record explicitly that we link all ghc
- -- executables with the threaded runtime.
- , arg "threaded" ] ]
+ -- Note [Linking ghc-bin against threaded stage0 RTS]
+ -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ -- We must maintain the invariant that GHCs linked with '-threaded'
+ -- are built with '-optc=-DTHREADED_RTS', otherwise we'll end up
+ -- with a GHC that can use the threaded runtime, but contains some
+ -- non-thread-safe functions. See
+ -- https://gitlab.haskell.org/ghc/ghc/issues/18024 for an example of
+ -- the sort of issues this can cause.
+ , ifM stage0
+ -- We build a threaded stage 1 if the bootstrapping compiler
+ -- supports it.
+ (ifM threadedBootstrapper
+ (arg "threaded")
+ (arg "-threaded"))
+ -- We build a threaded stage N.
+ (arg "threaded")
+ ]
+ ]
-------------------------------- ghcPkg --------------------------------
, package ghcPkg ?
=====================================
mk/config.mk.in
=====================================
@@ -199,6 +199,9 @@ endif
# `GhcUnregisterised` mode doesn't allow that.
GhcWithSMP := $(strip $(if $(filter YESNO, $(ArchSupportsSMP)$(GhcUnregisterised)),YES,NO))
+# Whether or not the bootstrapping GHC supplies a threaded RTS.
+GhcThreadedRts = @GhcThreadedRts@
+
# Whether to include GHCi in the compiler. Depends on whether the RTS linker
# has support for this OS/ARCH combination.
=====================================
testsuite/tests/deSugar/should_run/T18151.hs
=====================================
@@ -0,0 +1,10 @@
+-- According to the Report this should reduce to (). However, in #18151 it was
+-- reported that GHC bottoms.
+x :: ()
+x = seq (True `undefined`) ()
+{-# NOINLINE x #-}
+
+main :: IO ()
+main = do
+ print x
+
=====================================
testsuite/tests/deSugar/should_run/T18151.stdout
=====================================
@@ -0,0 +1 @@
+()
\ No newline at end of file
=====================================
testsuite/tests/deSugar/should_run/all.T
=====================================
@@ -63,3 +63,4 @@ test('T11601', exit_code(1), compile_and_run, [''])
test('T11747', normal, compile_and_run, ['-dcore-lint'])
test('T12595', normal, compile_and_run, [''])
test('T13285', normal, compile_and_run, [''])
+test('T18151', normal, compile_and_run, [''])
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/927e8c91d53d9d0c36debfd57fd1f30fa04e6b9a...1fdd616527070b0b09661f78363c176f35049d9c
--
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/927e8c91d53d9d0c36debfd57fd1f30fa04e6b9a...1fdd616527070b0b09661f78363c176f35049d9c
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/20200708/e9aab671/attachment-0001.html>
More information about the ghc-commits
mailing list