[Git][ghc/ghc][wip/marge_bot_batch_merge_job] 4 commits: Allow waiting for timerfd to be interrupted during rts shutdown

Marge Bot (@marge-bot) gitlab at gitlab.haskell.org
Wed Jan 25 01:34:00 UTC 2023



Marge Bot pushed to branch wip/marge_bot_batch_merge_job at Glasgow Haskell Compiler / GHC


Commits:
e5383a29 by Wander Hillen at 2023-01-24T20:02:26-05:00
Allow waiting for timerfd to be interrupted during rts shutdown

- - - - -
1957eda1 by Ryan Scott at 2023-01-24T20:03:01-05:00
Restore Compose's Read/Show behavior to match Read1/Show1 instances

Fixes #22816.

- - - - -
556e4495 by Matthew Pickering at 2023-01-24T20:33:50-05:00
docs: Update INSTALL.md

Removes references to make.

Fixes #22480

- - - - -
aee92d61 by Cheng Shao at 2023-01-24T20:33:51-05:00
compiler: fix handling of MO_F_Neg in wasm NCG

In the wasm NCG, we used to compile MO_F_Neg to 0.0-x. It was an
oversight, there actually exists f32.neg/f64.neg opcodes in the wasm
spec and those should be used instead! The old behavior almost works,
expect when GHC compiles the -0.0 literal, which will incorrectly
become 0.0.

- - - - -


10 changed files:

- INSTALL.md
- compiler/GHC/CmmToAsm/Wasm/Asm.hs
- compiler/GHC/CmmToAsm/Wasm/FromCmm.hs
- compiler/GHC/CmmToAsm/Wasm/Types.hs
- docs/users_guide/9.8.1-notes.rst
- libraries/base/Data/Functor/Compose.hs
- + libraries/base/tests/T22816.hs
- + libraries/base/tests/T22816.stdout
- libraries/base/tests/all.T
- rts/posix/ticker/Pthread.c


Changes:

=====================================
INSTALL.md
=====================================
@@ -20,15 +20,14 @@ Quick start:  the following gives you a default build:
 
     $ ./boot
     $ ./configure
-    $ make
-    $ make install
+    $ ./hadrian/build
 
   On Windows, you need an extra repository containing some build tools.
   These can be downloaded for you by configure. This only needs to be done once by running:
 
     $ ./configure --enable-tarballs-autodownload
 
-You can use Make's `-jN` option to parallelize the build. It's generally best
+You can use `-jN` option to parallelize the build. It's generally best
 to set `N` somewhere around the core count of the build machine.
 
 The `./boot` step is only necessary if this is a tree checked out from


=====================================
compiler/GHC/CmmToAsm/Wasm/Asm.hs
=====================================
@@ -359,6 +359,7 @@ asmTellWasmInstr ty_word instr = case instr of
   WasmF32DemoteF64 -> asmTellLine "f32.demote_f64"
   WasmF64PromoteF32 -> asmTellLine "f64.promote_f32"
   WasmAbs ty -> asmTellLine $ asmFromWasmType ty <> ".abs"
+  WasmNeg ty -> asmTellLine $ asmFromWasmType ty <> ".neg"
   WasmCond t -> do
     asmTellLine "if"
     asmWithTab $ asmTellWasmInstr ty_word t


=====================================
compiler/GHC/CmmToAsm/Wasm/FromCmm.hs
=====================================
@@ -224,6 +224,28 @@ extendSubword W32 TagI64 (WasmExpr instr) =
   WasmExpr $ instr `WasmConcat` WasmI64Extend32S
 extendSubword _ _ expr = expr
 
+-- | Lower an unary homogeneous operation.
+lower_MO_Un_Homo ::
+  ( forall pre t.
+    WasmTypeTag t ->
+    WasmInstr
+      w
+      (t : pre)
+      (t : pre)
+  ) ->
+  CLabel ->
+  CmmType ->
+  [CmmExpr] ->
+  WasmCodeGenM w (SomeWasmExpr w)
+lower_MO_Un_Homo op lbl t0 [x] = case someWasmTypeFromCmmType t0 of
+  SomeWasmType ty -> do
+    WasmExpr x_instr <- lower_CmmExpr_Typed lbl ty x
+    pure $
+      SomeWasmExpr ty $
+        WasmExpr $
+          x_instr `WasmConcat` op ty
+lower_MO_Un_Homo _ _ _ _ = panic "lower_MO_Un_Homo: unreachable"
+
 -- | Lower a binary homogeneous operation. Homogeneous: result type is
 -- the same with operand types.
 lower_MO_Bin_Homo ::
@@ -699,11 +721,12 @@ lower_CmmMachOp lbl (MO_F_Sub w0) xs =
     lbl
     (cmmFloat w0)
     xs
-lower_CmmMachOp lbl (MO_F_Neg w0) [x] =
-  lower_CmmMachOp
+lower_CmmMachOp lbl (MO_F_Neg w0) xs =
+  lower_MO_Un_Homo
+    WasmNeg
     lbl
-    (MO_F_Sub w0)
-    [CmmLit $ CmmFloat 0 w0, x]
+    (cmmFloat w0)
+    xs
 lower_CmmMachOp lbl (MO_F_Mul w0) xs =
   lower_MO_Bin_Homo
     WasmMul


=====================================
compiler/GHC/CmmToAsm/Wasm/Types.hs
=====================================
@@ -305,6 +305,7 @@ data WasmInstr :: WasmType -> [WasmType] -> [WasmType] -> Type where
   WasmF32DemoteF64 :: WasmInstr w ('F64 : pre) ('F32 : pre)
   WasmF64PromoteF32 :: WasmInstr w ('F32 : pre) ('F64 : pre)
   WasmAbs :: WasmTypeTag t -> WasmInstr w (t : pre) (t : pre)
+  WasmNeg :: WasmTypeTag t -> WasmInstr w (t : pre) (t : pre)
   WasmCond :: WasmInstr w pre pre -> WasmInstr w (w : pre) pre
 
 newtype WasmExpr w t = WasmExpr (forall pre. WasmInstr w pre (t : pre))


=====================================
docs/users_guide/9.8.1-notes.rst
=====================================
@@ -10,4 +10,74 @@ Compiler
 ~~~~~~~~
 
 - Added a new warning :ghc-flag:`-Wterm-variable-capture` that helps to make code compatible with
-  the future extension ``RequiredTypeArguments``.
\ No newline at end of file
+  the future extension ``RequiredTypeArguments``.
+=======
+
+GHCi
+~~~~
+
+
+Runtime system
+~~~~~~~~~~~~~~
+
+- On POSIX systems that support timerfd, RTS shutdown no longer has to wait for
+  the next RTS 'tick' to occur before continuing the shutdown process. See #22692.
+
+``base`` library
+~~~~~~~~~~~~~~~~
+
+
+``ghc-prim`` library
+~~~~~~~~~~~~~~~~~~~~
+
+``ghc`` library
+~~~~~~~~~~~~~~~
+
+``ghc-heap`` library
+~~~~~~~~~~~~~~~~~~~~
+
+
+Included libraries
+------------------
+
+The package database provided with this distribution also contains a number of
+packages other than GHC itself. See the changelogs provided with these packages
+for further change information.
+
+.. ghc-package-list::
+
+    libraries/array/array.cabal:             Dependency of ``ghc`` library
+    libraries/base/base.cabal:               Core library
+    libraries/binary/binary.cabal:           Dependency of ``ghc`` library
+    libraries/bytestring/bytestring.cabal:   Dependency of ``ghc`` library
+    libraries/Cabal/Cabal/Cabal.cabal:       Dependency of ``ghc-pkg`` utility
+    libraries/Cabal/Cabal-syntax/Cabal-syntax.cabal:  Dependency of ``ghc-pkg`` utility
+    libraries/containers/containers/containers.cabal: Dependency of ``ghc`` library
+    libraries/deepseq/deepseq.cabal:         Dependency of ``ghc`` library
+    libraries/directory/directory.cabal:     Dependency of ``ghc`` library
+    libraries/exceptions/exceptions.cabal:   Dependency of ``ghc`` and ``haskeline`` library
+    libraries/filepath/filepath.cabal:       Dependency of ``ghc`` library
+    compiler/ghc.cabal:                      The compiler itself
+    libraries/ghci/ghci.cabal:               The REPL interface
+    libraries/ghc-boot/ghc-boot.cabal:       Internal compiler library
+    libraries/ghc-boot-th/ghc-boot-th.cabal: Internal compiler library
+    libraries/ghc-compact/ghc-compact.cabal: Core library
+    libraries/ghc-heap/ghc-heap.cabal:       GHC heap-walking library
+    libraries/ghc-prim/ghc-prim.cabal:       Core library
+    libraries/haskeline/haskeline.cabal:     Dependency of ``ghci`` executable
+    libraries/hpc/hpc.cabal:                 Dependency of ``hpc`` executable
+    libraries/integer-gmp/integer-gmp.cabal: Core library
+    libraries/libiserv/libiserv.cabal:       Internal compiler library
+    libraries/mtl/mtl.cabal:                 Dependency of ``Cabal`` library
+    libraries/parsec/parsec.cabal:           Dependency of ``Cabal`` library
+    libraries/pretty/pretty.cabal:           Dependency of ``ghc`` library
+    libraries/process/process.cabal:         Dependency of ``ghc`` library
+    libraries/stm/stm.cabal:                 Dependency of ``haskeline`` library
+    libraries/template-haskell/template-haskell.cabal: Core library
+    libraries/terminfo/terminfo.cabal:       Dependency of ``haskeline`` library
+    libraries/text/text.cabal:               Dependency of ``Cabal`` library
+    libraries/time/time.cabal:               Dependency of ``ghc`` library
+    libraries/transformers/transformers.cabal: Dependency of ``ghc`` library
+    libraries/unix/unix.cabal:               Dependency of ``ghc`` library
+    libraries/Win32/Win32.cabal:             Dependency of ``ghc`` library
+    libraries/xhtml/xhtml.cabal:             Dependency of ``haddock`` executable


=====================================
libraries/base/Data/Functor/Compose.hs
=====================================
@@ -33,7 +33,7 @@ import Data.Coerce (coerce)
 import Data.Data (Data)
 import Data.Type.Equality (TestEquality(..), (:~:)(..))
 import GHC.Generics (Generic, Generic1)
-import Text.Read ()
+import Text.Read (Read(..), ReadPrec, readListDefault, readListPrecDefault)
 
 infixr 9 `Compose`
 
@@ -55,9 +55,14 @@ deriving instance Eq (f (g a)) => Eq (Compose f g a)
 -- | @since 4.18.0.0
 deriving instance Ord (f (g a)) => Ord (Compose f g a)
 -- | @since 4.18.0.0
-deriving instance Read (f (g a)) => Read (Compose f g a)
+instance Read (f (g a)) => Read (Compose f g a) where
+    readPrec = liftReadPrecCompose readPrec
+
+    readListPrec = readListPrecDefault
+    readList     = readListDefault
 -- | @since 4.18.0.0
-deriving instance Show (f (g a)) => Show (Compose f g a)
+instance Show (f (g a)) => Show (Compose f g a) where
+    showsPrec = liftShowsPrecCompose showsPrec
 
 -- Instances of lifted Prelude classes
 
@@ -72,8 +77,8 @@ instance (Ord1 f, Ord1 g) => Ord1 (Compose f g) where
 
 -- | @since 4.9.0.0
 instance (Read1 f, Read1 g) => Read1 (Compose f g) where
-    liftReadPrec rp rl = readData $
-        readUnaryWith (liftReadPrec rp' rl') "Compose" Compose
+    liftReadPrec rp rl =
+        liftReadPrecCompose (liftReadPrec rp' rl')
       where
         rp' = liftReadPrec     rp rl
         rl' = liftReadListPrec rp rl
@@ -83,12 +88,20 @@ instance (Read1 f, Read1 g) => Read1 (Compose f g) where
 
 -- | @since 4.9.0.0
 instance (Show1 f, Show1 g) => Show1 (Compose f g) where
-    liftShowsPrec sp sl d (Compose x) =
-        showsUnaryWith (liftShowsPrec sp' sl') "Compose" d x
+    liftShowsPrec sp sl =
+        liftShowsPrecCompose (liftShowsPrec sp' sl')
       where
         sp' = liftShowsPrec sp sl
         sl' = liftShowList sp sl
 
+-- The workhorse for Compose's Read and Read1 instances.
+liftReadPrecCompose :: ReadPrec (f (g a)) -> ReadPrec (Compose f g a)
+liftReadPrecCompose rp = readData $ readUnaryWith rp "Compose" Compose
+
+-- The workhorse for Compose's Show and Show1 instances.
+liftShowsPrecCompose :: (Int -> f (g a) -> ShowS) -> Int -> Compose f g a -> ShowS
+liftShowsPrecCompose sp d (Compose x) = showsUnaryWith sp "Compose" d x
+
 -- Functor instances
 
 -- | @since 4.9.0.0


=====================================
libraries/base/tests/T22816.hs
=====================================
@@ -0,0 +1,31 @@
+module Main (main) where
+
+import Data.Functor.Classes
+import Data.Functor.Compose
+import Text.ParserCombinators.ReadP as P
+import Text.ParserCombinators.ReadPrec (ReadPrec, lift, minPrec, readPrec_to_S)
+
+readEither' :: ReadPrec a -> String -> Either String a
+readEither' rp s =
+  case [ x | (x,"") <- readPrec_to_S read' minPrec s ] of
+    [x] -> Right x
+    []  -> Left "read1: no parse"
+    _   -> Left "read1: ambiguous parse"
+ where
+  read' =
+    do x <- rp
+       lift P.skipSpaces
+       return x
+
+-- | Like 'read', but tailored to 'Read1'.
+read1 :: (Read1 f, Read a) => String -> f a
+read1 s = either errorWithoutStackTrace id (readEither' readPrec1 s)
+
+exRead, exRead1 :: Compose Maybe Maybe Int
+exRead  = read  "Compose Nothing"
+exRead1 = read1 "Compose Nothing"
+
+main :: IO ()
+main = do
+  putStrLn $ showsPrec  0 exRead  ""
+  putStrLn $ showsPrec1 0 exRead1 ""


=====================================
libraries/base/tests/T22816.stdout
=====================================
@@ -0,0 +1,2 @@
+Compose Nothing
+Compose Nothing


=====================================
libraries/base/tests/all.T
=====================================
@@ -286,6 +286,7 @@ test('T18642',
 test('T19288', exit_code(1), compile_and_run, [''])
 test('T19719', normal, compile_and_run, [''])
 test('T20107', extra_run_opts('+RTS -M50M'), compile_and_run, ['-package bytestring'])
+test('T22816', normal, compile_and_run, [''])
 test('trace', normal, compile_and_run, [''])
 test('listThreads', js_broken(22261), compile_and_run, [''])
 test('inits1tails1', normal, compile_and_run, [''])


=====================================
rts/posix/ticker/Pthread.c
=====================================
@@ -43,6 +43,7 @@
 #include "Proftimer.h"
 #include "Schedule.h"
 #include "posix/Clock.h"
+#include <sys/poll.h>
 
 #include <time.h>
 #if HAVE_SYS_TIME_H
@@ -95,28 +96,53 @@ static OSThreadId thread;
 // file descriptor for the timer (Linux only)
 static int timerfd = -1;
 
+// pipe for signaling exit
+static int pipefds[2];
+
 static void *itimer_thread_func(void *_handle_tick)
 {
     TickProc handle_tick = _handle_tick;
     uint64_t nticks;
+    ssize_t r = 0;
+    struct pollfd pollfds[2];
+
+#if USE_TIMERFD_FOR_ITIMER
+    pollfds[0].fd = pipefds[0];
+    pollfds[0].events = POLLIN;
+    pollfds[1].fd = timerfd;
+    pollfds[1].events = POLLIN;
+#endif
 
     // Relaxed is sufficient: If we don't see that exited was set in one iteration we will
     // see it next time.
     TSAN_ANNOTATE_BENIGN_RACE(&exited, "itimer_thread_func");
     while (!RELAXED_LOAD(&exited)) {
         if (USE_TIMERFD_FOR_ITIMER) {
-            ssize_t r = read(timerfd, &nticks, sizeof(nticks));
-            if ((r == 0) && (errno == 0)) {
-               /* r == 0 is expected only for non-blocking fd (in which case
-                * errno should be EAGAIN) but we use a blocking fd.
-                *
-                * Due to a kernel bug (cf https://lkml.org/lkml/2019/8/16/335)
-                * on some platforms we could see r == 0 and errno == 0.
-                */
-               IF_DEBUG(scheduler, debugBelch("read(timerfd) returned 0 with errno=0. This is a known kernel bug. We just ignore it."));
+            if (poll(pollfds, 2, -1) == -1) {
+                sysErrorBelch("Ticker: poll failed: %s", strerror(errno));
             }
-            else if (r != sizeof(nticks) && errno != EINTR) {
-               barf("Ticker: read(timerfd) failed with %s and returned %zd", strerror(errno), r);
+
+            // We check the pipe first, even though the timerfd may also have triggered.
+            if (pollfds[0].revents & POLLIN) {
+                // the pipe is ready for reading, the only possible reason is that we're exiting
+                exited = true; // set this again to make sure even RELAXED_LOAD will read the proper value
+                // no further action needed, skip ahead to handling the final tick and then stopping
+            }
+            else if (pollfds[1].revents & POLLIN) { // the timerfd is ready for reading
+                r = read(timerfd, &nticks, sizeof(nticks)); // this should never block now
+
+                if ((r == 0) && (errno == 0)) {
+                   /* r == 0 is expected only for non-blocking fd (in which case
+                    * errno should be EAGAIN) but we use a blocking fd.
+                    *
+                    * Due to a kernel bug (cf https://lkml.org/lkml/2019/8/16/335)
+                    * on some platforms we could see r == 0 and errno == 0.
+                    */
+                   IF_DEBUG(scheduler, debugBelch("read(timerfd) returned 0 with errno=0. This is a known kernel bug. We just ignore it."));
+                }
+                else if (r != sizeof(nticks) && errno != EINTR) {
+                   barf("Ticker: read(timerfd) failed with %s and returned %zd", strerror(errno), r);
+                }
             }
         } else {
             if (rtsSleep(itimer_interval) != 0) {
@@ -138,8 +164,10 @@ static void *itimer_thread_func(void *_handle_tick)
         }
     }
 
-    if (USE_TIMERFD_FOR_ITIMER)
+    if (USE_TIMERFD_FOR_ITIMER) {
         close(timerfd);
+    }
+
     return NULL;
 }
 
@@ -185,6 +213,10 @@ initTicker (Time interval, TickProc handle_tick)
     if (timerfd_settime(timerfd, 0, &it, NULL)) {
         barf("timerfd_settime: %s", strerror(errno));
     }
+
+    if (pipe(pipefds) < 0) {
+        barf("pipe: %s", strerror(errno));
+    }
 #endif
 
     /*
@@ -237,9 +269,21 @@ exitTicker (bool wait)
 
     // wait for ticker to terminate if necessary
     if (wait) {
+#if USE_TIMERFD_FOR_ITIMER
+        // write anything to the pipe to trigger poll() in the ticker thread
+        if (write(pipefds[1], "stop", 5) < 0) {
+            sysErrorBelch("Ticker: Failed to write to pipe: %s", strerror(errno));
+        }
+#endif
         if (pthread_join(thread, NULL)) {
             sysErrorBelch("Ticker: Failed to join: %s", strerror(errno));
         }
+#if USE_TIMERFD_FOR_ITIMER
+        // These need to happen AFTER the ticker thread has finished to prevent a race condition
+        // where the ticker thread closes the read end of the pipe before we're done writing to it.
+        close(pipefds[0]);
+        close(pipefds[1]);
+#endif
         closeMutex(&mutex);
         closeCondition(&start_cond);
     } else {



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/0690c24f51a5bf0d8dff585bf8871ea90caa4528...aee92d61be975088a68983ac8c8a354c1fd87298

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/0690c24f51a5bf0d8dff585bf8871ea90caa4528...aee92d61be975088a68983ac8c8a354c1fd87298
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/20230124/a88302d2/attachment-0001.html>


More information about the ghc-commits mailing list