[Git][ghc/ghc][wip/marge_bot_batch_merge_job] 45 commits: rts: Fix TSAN_ENABLED CPP guard

[Marge Bot (@marge-bot)]

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


Commits:
c8a4c050 by Ben Gamari at 2024-04-02T12:50:35-04:00
rts: Fix TSAN_ENABLED CPP guard

This should be `#if defined(TSAN_ENABLED)`, not `#if TSAN_ENABLED`,
lest we suffer warnings.

- - - - -
e91dad93 by Cheng Shao at 2024-04-02T12:50:35-04:00
rts: fix errors when compiling with TSAN

This commit fixes rts compilation errors when compiling with TSAN:

- xxx_FENCE macros are redefined and trigger CPP warnings.
- Use SIZEOF_W. WORD_SIZE_IN_BITS is provided by MachDeps.h which
  Cmm.h doesn't include by default.

- - - - -
a9ab9455 by Cheng Shao at 2024-04-02T12:50:35-04:00
rts: fix clang-specific errors when compiling with TSAN

This commit fixes clang-specific rts compilation errors when compiling
with TSAN:

- clang doesn't have -Wtsan flag
- Fix prototype of ghc_tsan_* helper functions
- __tsan_atomic_* functions aren't clang built-ins and
  sanitizer/tsan_interface_atomic.h needs to be included
- On macOS, TSAN runtime library is
  libclang_rt.tsan_osx_dynamic.dylib, not libtsan. -fsanitize-thread
  as a link-time flag will take care of linking the TSAN runtime
  library anyway so remove tsan as an rts extra library

- - - - -
865bd717 by Cheng Shao at 2024-04-02T12:50:35-04:00
compiler: fix github link to __tsan_memory_order in a comment

- - - - -
07cb627c by Cheng Shao at 2024-04-02T12:50:35-04:00
ci: improve TSAN CI jobs

- Run TSAN jobs with +thread_sanitizer_cmm which enables Cmm
  instrumentation as well.
- Run TSAN jobs in deb12 which ships gcc-12, a reasonably recent gcc
  that @bgamari confirms he's using in #GHC:matrix.org. Ideally we
  should be using latest clang release for latest improvements in
  sanitizers, though that's left as future work.
- Mark TSAN jobs as manual+allow_failure in validate pipelines. The
  purpose is to demonstrate that we have indeed at least fixed
  building of TSAN mode in CI without blocking the patch to land, and
  once merged other people can begin playing with TSAN using their own
  dev setups and feature branches.

- - - - -
a1c18c7b by Andrei Borzenkov at 2024-04-02T12:51:11-04:00
Merge tc_infer_hs_type and tc_hs_type into one function using ExpType philosophy (#24299, #23639)

This patch implements refactoring which is a prerequisite to
updating kind checking of type patterns. This is a huge simplification
of the main worker that checks kind of HsType.

It also fixes the issues caused by previous code duplication, e.g.
that we didn't add module finalizers from splices in inference mode.

- - - - -
dda18f97 by Duncan Coutts at 2024-04-02T13:22:18-04:00
Initial ./configure support for selecting I/O managers

In this patch we just define new CPP vars, but don't yet use them
or replace the existing approach. That will follow.

The intention here is that every I/O manager can be enabled/disabled at
GHC build time (subject to some constraints). More than one I/O manager
can be enabled to be built. At least one I/O manager supporting the
non-threaded RTS must be enabled as well as at least one supporting the
non-threaded RTS. The I/O managers enabled here will become the choices
available at runtime at RTS startup (in later patches). The choice can
be made with RTS flags. There are separate sets of choices for the
threaded and non-threaded RTS ways, because most I/O managers are
specific to these ways. Furthermore we must establish a default I/O
manager for the threaded and non-threaded RTS.

Most I/O managers are platform-specific so there are checks to ensure
each one can be enabled on the platform. Such checks are also where (in
future) any system dependencies (e.g. libraries) can be checked.

The output is a set of CPP flags (in the mk/config.h file), with one
flag per named I/O manager:
* IOMGR_BUILD_<name>                : which ones should be built (some)
* IOMGR_DEFAULT_NON_THREADED_<name> : which one is default (exactly one)
* IOMGR_DEFAULT_THREADED_<name>     : which one is default (exactly one)

and a set of derived flags in IOManager.h

* IOMGR_ENABLED_<name>              : enabled for the current RTS way

Note that IOMGR_BUILD_<name> just says that an I/O manager will be
built for _some_ RTS way (i.e. threaded or non-threaded). The derived
flags IOMGR_ENABLED_<name> in IOManager.h say if each I/O manager is
enabled in the "current" RTS way. These are the ones that can be used
for conditional compilation of the I/O manager code.

Co-authored-by: Pi Delport <pi at well-typed.com>

- - - - -
f6405b9f by Duncan Coutts at 2024-04-02T13:22:18-04:00
Change the handling of the RTS flag --io-manager=

Now instead of it being just used on Windows to select between the WinIO
vs the MIO or Win32-legacy I/O managers, it is now used on all platforms
for selecting the I/O manager to use.

Right now it remains the case that there is only an actual choice on
Windows, but that will change later.

This change is also reflected in the RTS flags types in the base
library.

The way the 'IoSubSystem' is detected also changes. Instead of looking
at the RTS flag, there is now a C bool global var in the RTS which gets
set on startup when the I/O manager is selected. This bool var says
whether the selected I/O manager classifies as "native" on Windows,
which in practice means the WinIO I/O manager has been selected.

Similarly, the is_io_mng_native_p RTS helper function is re-implemented
in terms of the selected I/O manager, rather than based on the RTS
flags.

- - - - -
ec863cb3 by Duncan Coutts at 2024-04-02T13:22:18-04:00
Convert {init,stop,exit}IOManager to switch style

Rather than ad-hoc cpp conitionals on THREADED_RTS and mingw32_HOST_OS,
we use a style where we switch on the I/O manager impl, with cases for
each I/O manager impl.

- - - - -
75ba245f by Duncan Coutts at 2024-04-02T13:22:18-04:00
Split up the CapIOManager content by I/O manager

Using the new IOMGR_ENABLED_<name> CPP defines.

- - - - -
b0d3b19f by Duncan Coutts at 2024-04-02T13:22:18-04:00
Convert initIOManagerAfterFork and wakeupIOManager to switch style

- - - - -
b7649169 by Duncan Coutts at 2024-04-02T13:22:18-04:00
Move most of waitRead#/Write# from cmm to C

Moves it into the IOManager.c where we can follow the new pattern of
switching on the selected I/O manager.

- - - - -
a18cb695 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Move most of the delay# impl from cmm to C

Moves it into the IOManager.c where we can follow the new pattern of
switching on the selected I/O manager.

Uses a new IOManager API: syncDelay, following the naming convention of
sync* for thread-synchronous I/O & timer/delay operations.

- - - - -
155602b5 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Use atomic operations to update tso->why_blocked in the C code

In the primops ported from Cmm to C.

Since patch 515eb33d4fc the why_blocked gets accessed using load
acquire and store release atomics.

There was one exception to this new rule in 515eb33d4fc: for the delay#
primop on posix (not win32), the why_blocked was being updated using a
store relaxed, not a store release. I've no idea why. I'm playing it
safe here and using store release consistently.

- - - - -
88a3b93c by Duncan Coutts at 2024-04-02T13:22:19-04:00
insertIntoSleepingQueue is no longer public

No longer defined in IOManager.h, just a private function in
IOManager.c. Since it is no longer called from cmm code, just from
syncDelay. It ought to get moved further into the select() I/O manager
impl, rather than living in IOManager.c.

On the other hand appendToIOBlockedQueue is still called from cmm code
in the win32-legacy I/O manager primops async{Read,Write}#, and it is
also used by the select() I/O manager. Update the CPP and comments to
reflect this.

- - - - -
136edfdb by Duncan Coutts at 2024-04-02T13:22:19-04:00
Move anyPendingTimeoutsOrIO impl from .h to .c

The implementation is eventually going to need to use more private
things, which will drag in unwanted includes into IOManager.h, so it's
better to move the impl out of the header file and into the .c file, at
the slight cost of it no longer being inline.

At the same time, change to the "switch (iomgr_type)" style.

- - - - -
2433bc3f by Duncan Coutts at 2024-04-02T13:22:19-04:00
Take a simpler approach to gcc warnings in IOManager.c

We have lots of functions with conditional implementations for
different I/O managers. Some functions, for some I/O managers,
naturally have implementations that do nothing or barf. When only one
such I/O manager is enabled then the whole function implementation will
have an implementation that does nothing or barfs. This then results in
warnings from gcc that parameters are unused, or that the function
should be marked with attribute noreturn (since barf does not return).
The USED_IF_THREADS trick for fine-grained warning supression is fine
for just two cases, but an equivalent here would need
USED_IF_THE_ONLY_ENABLED_IOMGR_IS_X_OR_Y which would have combinitorial
blowup. So we take a coarse grained approach and simply disable these
two warnings for the whole file.

So we use a GCC pragma, with its handy push/pop support:

 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wsuggest-attribute=noreturn"
 #pragma GCC diagnostic ignored "-Wunused-parameter"

...

 #pragma GCC diagnostic pop

- - - - -
f194aaec by Duncan Coutts at 2024-04-02T13:22:19-04:00
Add a new trace class for the iomanager

It makes sense now for it to be separate from the scheduler class of
tracers.

Enabled with +RTS -Do.

- - - - -
f0545200 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Have the throwTo impl go via (new) IOManager APIs

rather than directly operating on the IO manager's data structures.

Specifically, when thowing an async exception to a thread that is
blocked waiting for I/O or waiting for a timer, then we want to cancel
that I/O waiting or cancel the timer. Currently this is done directly in
removeFromQueues() in RaiseAsync.c. We want it to go via proper APIs
both for modularity but also to let us support multiple I/O managers.

So add sync{IO,Delay}Cancel, which is the cancellation for the
corresponding sync{IO,Delay}. The implementations of these use the usual
"switch (iomgr_type)" style.

- - - - -
df660784 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Move awaitEvent into a proper IOManager API

and have the scheduler use it.

Previously the scheduler calls awaitEvent directly, and awaitEvent is
implemented directly in the RTS I/O managers (select, win32). This
relies on the old scheme where there's a single active I/O manager for
each platform and RTS way.

We want to move that to go via an API in IOManager.{h,c} which can then
call out to the active I/O manager.

Also take the opportunity to split awaitEvent into two. The existing
awaitEvent has a bool wait parameter, to say if the call should be
blocking or non-blocking. We split this into two separate functions:
pollCompletedTimeoutsOrIO and awaitCompletedTimeoutsOrIO. We split them
for a few reasons: they have different post-conditions (specifically the
await version is supposed to guarantee that there are threads runnable
when it completes). Secondly, it is also anticipated that in future I/O
managers the implementations of the two cases will be simpler if they
are separated.

- - - - -
b402e158 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Rename awaitEvent in select and win32 I/O managers

These are now just called from IOManager.c and are the per-I/O manager
backend impls (whereas previously awaitEvent was the entry point).

Follow the new naming convention in the IOManager.{h,c} of
awaitCompletedTimeoutsOrIO, with the I/O manager's name as a suffix:
so awaitCompletedTimeoutsOrIO{Select,Win32}.

- - - - -
d694b0fa by Duncan Coutts at 2024-04-02T13:22:19-04:00
Tidy up a couple things in Select.{h,c}

Use the standard #include {Begin,End}Private.h style rather than
RTS_PRIVATE on individual decls.

And conditionally build the code for the select I/O manager based on
the new CPP IOMGR_ENABLED_SELECT rather than on THREADED_RTS.

- - - - -
ef60a964 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Add an IOManager API for scavenging TSO blocked_info

When the GC scavenges a TSO it needs to scavenge the tso->blocked_info
but the blocked_info is a big union and what lives there depends on the
two->why_blocked, which for I/O-related reasons is something that in
principle is the responsibility of the I/O manager and not the GC. So
the right thing to do is for the GC to ask the I/O manager to sscavenge
the blocked_info if it encounters any I/O-related why_blocked reasons.

So we add scavengeTSOIOManager in IOManager.{h,c} with the usual style.

Now as it happens, right now, there is no special scavenging to do, so
the implementation of scavengeTSOIOManager is a fancy no-op. That's
because the select I/O manager uses only the fd and target members,
which are not GC pointers, and the win32-legacy I/O manager _ought_ to
be using GC-managed heap objects for the StgAsyncIOResult but it is
actually usingthe C heap, so again no GC pointers. If the win32-legacy
were doing this more sensibly, then scavengeTSOIOManager would be the
right place to do the GC magic.

Future I/O managers will need GC heap objects in the tso->blocked_info
and will make use of this functionality.

- - - - -
14f7ed5c by Duncan Coutts at 2024-04-02T13:22:19-04:00
Add I/O manager API notifyIOManagerCapabilitiesChanged

Used in setNumCapabilities.

It only does anything for MIO on Posix.

Previously it always invoked Haskell code, but that code only did
anything on non-Windows (and non-JS), and only threaded. That currently
effectively means the MIO I/O manager on Posix.

So now it only invokes it for the MIO Posix case.

- - - - -
c9861100 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Select an I/O manager early in RTS startup

We need to select the I/O manager to use during startup before the
per-cap I/O manager initialisation.

- - - - -
57abc5d3 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Make struct CapIOManager be fully opaque

Provide an opaque (forward) definition in Capability.h (since the cap
contains a *CapIOManager) and then only provide a full definition in
a new file IOManagerInternals.h. This new file is only supposed to be
included by the IOManager implementation, not by its users. So that
means IOManager.c and individual I/O manager implementations.

The posix/Signals.c still needs direct access, but that should be
eliminated. Anything that needs direct access either needs to be clearly
part of an I/O manager (e.g. the sleect() one) or go via a proper API.

- - - - -
e891bb2e by Duncan Coutts at 2024-04-02T13:22:19-04:00
The select() I/O manager does have some global initialisation

It's just to make sure an exception CAF is a GC root.

- - - - -
a1905d4d by Duncan Coutts at 2024-04-02T13:22:19-04:00
Add tracing for the main I/O manager actions

Using the new tracer class.

Note: The unconditional definition of showIOManager should be
compatible with the debugTrace change in 7c7d1f6.

Co-authored-by: Pi Delport <pi at well-typed.com>

- - - - -
13734227 by Duncan Coutts at 2024-04-02T13:22:19-04:00
waitRead# / waitWrite# do not work for win32-legacy I/O manager

Previously it was unclear that they did not work because the code path
was shared with other I/O managers (in particular select()).

Following the code carefully shows that what actually happens is that
the calling thread would block forever: the thread will be put into the
blocked queue, but no other action is scheduled that will ever result in
it getting unblocked.

It's better to just fail loudly in case anyone accidentally calls it,
also it's less confusing code.

- - - - -
1fc6ee4c by Duncan Coutts at 2024-04-02T13:22:19-04:00
Conditionally ignore some GCC warnings

Some GCC versions don't know about some warnings, and they complain
that we're ignoring unknown warnings. So we try to ignore the warning
based on the GCC version.

- - - - -
2ac87360 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Accept changes to base-exports

All the changes are in fact not changes at all.

Previously, the IoSubSystem data type was defined in GHC.RTS.Flags and
exported from both GHC.RTS.Flags and GHC.IO.SubSystem. Now, the data
type is defined in GHC.IO.SubSystem and still exported from both
modules.

Therefore, the same exports and same instances are still available from
both modules. But the base-exports records only the defining module, and
so it looks like a change when it is fully compatible.

Related: we do add a deprecation to the export of the type via
GHC.RTS.Flags, telling people to use the export from GHC.IO.SubSystem.

Also the sort order for some unrelated Show instances changed. No idea
why.

The same changes apply in the other versions, with a few more changes
due to sort order weirdness.

- - - - -
b14bb8a2 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Accept metric decrease in T12227

I can't think of any good reason that anything in this MR should have
changed the number of allocations, up or down.

(Yes this is an empty commit.)

Metric Decrease:
    T12227

- - - - -
4922cb30 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Review feedback: simplify ./configure support for I/O managers

Have all decisions be automatic without any user influence, rather than
automatic with user-specified flags to override.

This entails removing the existing --enable-native-io-manager flag.

Also simplify the CPP flags defined by ./configure and have more stuff
done by IOManager.h instead. This also gives us a place to put sanity
checks for there being a default I/O manager.

- - - - -
33350cbe by Duncan Coutts at 2024-04-02T13:22:19-04:00
Fixup: make showIOManager public (non-static)

declare it in IOManager.h

- - - - -
75507feb by Duncan Coutts at 2024-04-02T13:22:19-04:00
Include the default I/O manager in the +RTS --info output

- - - - -
b5fcc525 by Duncan Coutts at 2024-04-02T13:22:19-04:00
fixup: fix spelling in src/GHC/Internal/IO/SubSystem.hs

- - - - -
f936ecba by Duncan Coutts at 2024-04-02T13:22:19-04:00
fixup: fix spelling in rts/IOManager.h

- - - - -
5b29b688 by Duncan Coutts at 2024-04-02T13:22:19-04:00
review feedback: document the post-condition of awaitCompletedTimeoutsOrIO

- - - - -
1f7a18e3 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Document --io-manager flag in the user guide

And the -Do debug flag.

- - - - -
c3cdce8a by Duncan Coutts at 2024-04-02T13:22:19-04:00
Use "unrecognised" rather than "unknown" for io-manager flag parsing

Somewhat clearer meaning in the code and error messages.

- - - - -
79bfa225 by Duncan Coutts at 2024-04-02T13:22:19-04:00
Re-export IoSubSystem from GHC.RTS.Flags with deprecation warning

It was exported here previously. Reinstate the export for ease of
migration, now with a deprecation warning for where to get it from
instead.

See https://github.com/haskell/core-libraries-committee/issues/263

- - - - -
8547e0a0 by Duncan Coutts at 2024-04-02T13:22:19-04:00
FIXUP: base-exports

- - - - -
27d1a25b by Rodrigo Mesquita at 2024-04-02T13:22:20-04:00
th: Hide the Language.Haskell.TH.Lib.Internal module from haddock

Fixes #24562

- - - - -
1f26c63b by Sylvain Henry at 2024-04-02T13:22:32-04:00
JS: reenable h$appendToHsString optimization (#24495)

The optimization introducing h$appendToHsString wasn't kicking in
anymore (while it did in 9.8.1) because of the changes introduced in #23270 (7e0c8b3bab30).
This patch reenables the optimization by matching on case-expression, as
done in Cmm for unpackCString# standard thunks.

The test is also T24495 added in the next commits (two commits for ease
of backporting to 9.8).

- - - - -
2e84acd9 by Sylvain Henry at 2024-04-02T13:22:32-04:00
JS: fix h$appendToHsString implementation (#24495)

h$appendToHsString needs to wrap its argument in an updatable thunk
to behave like unpackAppendCString#. Otherwise if a SingleEntry thunk is
passed, it is stored as-is in a CONS cell, making the resulting list
impossible to deepseq (forcing the thunk doesn't update the contents of
the CONS cell)!

The added test checks that the optimization kicks in and that
h$appendToHsString works as intended.

Fix #24495

- - - - -


30 changed files:

- .gitlab/generate-ci/gen_ci.hs
- .gitlab/jobs.yaml
- compiler/GHC/Cmm/ThreadSanitizer.hs
- compiler/GHC/Rename/Splice.hs
- compiler/GHC/StgToJS/Apply.hs
- compiler/GHC/StgToJS/Expr.hs
- compiler/GHC/StgToJS/Linker/Utils.hs
- compiler/GHC/StgToJS/Rts/Rts.hs
- compiler/GHC/Tc/Gen/Bind.hs
- compiler/GHC/Tc/Gen/HsType.hs
- compiler/GHC/Tc/TyCl.hs
- compiler/GHC/Tc/Utils/TcType.hs
- compiler/GHC/Tc/Utils/Unify.hs
- compiler/Language/Haskell/Syntax/Type.hs
- docs/users_guide/runtime_control.rst
- libraries/base/src/GHC/IO/SubSystem.hs
- libraries/base/src/GHC/RTS/Flags.hs
- libraries/ghc-internal/src/GHC/Internal/IO/SubSystem.hs
- libraries/ghc-internal/src/GHC/Internal/RTS/Flags.hsc
- libraries/template-haskell/Language/Haskell/TH/Lib/Internal.hs
- + m4/ghc_iomanagers.m4
- rts/Capability.c
- rts/Capability.h
- rts/IOManager.c
- rts/IOManager.h
- + rts/IOManagerInternals.h
- rts/PrimOps.cmm
- rts/RaiseAsync.c
- rts/RtsFlags.c
- rts/RtsFlags.h


The diff was not included because it is too large.


View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c1c37c47130f1f78137f49a1bfb5e0e3dd322b37...2e84acd912d4db77a25cb68f434de8f41d30b455

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c1c37c47130f1f78137f49a1bfb5e0e3dd322b37...2e84acd912d4db77a25cb68f434de8f41d30b455
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/20240402/d4bbaee0/attachment-0001.html>