public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Christophe Lyon <christophe.lyon@linaro.org>
To: Hans-Peter Nilsson <hp@axis.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, libstdc++@gcc.gnu.org
Subject: Re: [PATCH] __atomic_test_and_set: Fall back to library, not non-atomic code
Date: Wed, 4 Oct 2023 10:53:45 +0200	[thread overview]
Message-ID: <CAPS5khavTy-0F6BphpB+-EBf8G6XqW3KEr0aWBCsKzVdrQPpgA@mail.gmail.com> (raw)
In-Reply-To: <20231004004929.9F76B2042E@pchp3.se.axis.com>

[-- Attachment #1: Type: text/plain, Size: 4979 bytes --]

On Wed, 4 Oct 2023 at 02:49, Hans-Peter Nilsson <hp@axis.com> wrote:

> (Just before sending, I noticed you replied off-list; I
> won't add back gcc-patches to cc here myself, but please
> feel free to do it, if you choose to reply.)
>

Sorry, it was a typo of mine, I meant to reply to the list


>
> > From: Christophe Lyon <christophe.lyon@linaro.org>
> > Date: Tue, 3 Oct 2023 18:06:21 +0200
>
> > On Tue, 3 Oct 2023 at 17:16, Hans-Peter Nilsson <hp@axis.com> wrote:
> >
> > > > From: Christophe Lyon <christophe.lyon@linaro.org>
> > > > Date: Tue, 3 Oct 2023 15:20:39 +0200
> > >
> > > > The patch passed almost all our CI configurations, except arm-eabi
> when
> > > > testing with
> > > >  -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto
> > > > where is causes these failures:
> > > > FAIL: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17 (test for excess
> > > > errors)
> > > > UNRESOLVED: 29_atomics/atomic_flag/clear/1.cc -std=gnu++17
> compilation
> > > > failed to produce executable
> [...]
> > > For which set of multilibs in that set, do you get these
> > > errors?  I'm guessing -march=armv6s-m, but I'm checking.
> > >
> >
> > Not sure to understand the question?
>
> By your "testing with
> -mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto"
> I presume you mean "testing with make check
>
> 'RUNTESTFLAGS=--target_board=arm-sim/-mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto'
> (as that's where you usually put that expression)
>
Yes


>
> - but I misremembered what "/" means in RUNTESTFLAGS (it's
> combining options in one single set of options passed to
> gcc, not delimiting separate options used to form
> combinations).  Sorry for the confusion!
>
I see.

Actually I always find "multilib" confusing in the context of running the
tests, where in fact we generally mean we add some flags in
RUNTESTFLAGS and/or --target_board.
To me, multilib refers to the set of lib variants we build, but I noticed
"multilib" is often used in the context of running the tests...


>
> > > > Maybe we need a new variant of dg-require-thread-fence ?
> > >
> > > Perhaps.
>
> Or rather: most certainly.  IIUC, ARMv6 (or whatever you
> prefer to call it) can load and store atomically, but only
> as separate events; it can't atomically exchange a stored
> value and therefore arm-eabi calls out to a library
> function.
>
In this case, it's armv6s-m (which is different from armv6....)
And yes, it seems so, as your patch showed, assuming there's
no bug in the target description ;-)


> I think I'll help and replace the obvious uses of
> dg-require-thread-fence where actually an atomic exchange is
> required; replacing those with a new directive
> dg-require-atomic-exchange.  That will however not be *all*
> places where such a guard should be added.
>
Indeed.


> I also see lots of undefined references to *other* outlined
> atomic builtins, for example __atomic_compare_exchange_4 and
> __atomic_fetch_add_4.  Though, those likely aren't
> regressions.  I understand you focus on regressions here.
>
Yes, my reply to your patch was meant to look at the regressions.

As a separate action, I plan to look at the remaining existing such
failures.


> By the way, how do you test; what simulator, what baseboard
> file?  Please share!  Also, please send *some*
> contrib/test_summary reports for arm-eabi to
> gcc-testresults@ every now and then.  (But also, please
>
We use qemu, with qemu.exp from:
https://git.linaro.org/toolchain/abe.git/tree/config/boards/qemu.exp
nothing fancy ;-)



> don't post multiple results several times a day for similar
> configurations.  Looking at you, powerpc people!)
>
We have plans to restart sending such results, like I was doing several
years ago
(with many results every day, too ;-))


> I can't test *anything* newer than default arm-eabi (armv4t)
> on arm-sim (the one next to gdb), or else execution tests
> get lost and time out while also complaining about "Unknown
> machine type".  I noticed there's a qemu.exp in ToT dejagnu,
> but it doesn't work for arm-eabi for at least two reasons.
> (I might get to that yak later, I just take it as a sign
> that qemu-arm isn't what I look for.)

We do use qemu-arm, depending on how you want to test,
maybe you need to add a -cpu flag such that it supports
the required instructions.


> >  Unless of course, there's a multilib combination
>
> > for which you *can* emit the proper atomic spell; missing it
> > > because the need for it, has been hidden!
> > >
> > > (At first I thought it was related to caching the
> > > thread-fence property across multilib testing, but I don't
> > > think that was correct.)
> > >
> > Not sure what you mean? We run the tests for a single multilib here
> > (mthumb/-march=armv6s-m/-mtune=cortex-m0/-mfloat-abi=soft/-mfpu=auto)
> > so the cached value should always be correct?
>
> Yeah, part of my RUNTESTFLAGS confusion per above, please
> ignore that.
>
> brgds, H-P
>

      parent reply	other threads:[~2023-10-04  8:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230926143439.B589920431@pchp3.se.axis.com>
     [not found] ` <CAPS5khYXtGYLr-pqAQRy_UAsOJATted1Gs0xY4ytTWppFPVJaQ@mail.gmail.com>
2023-10-04  2:55   ` [PATCH 1/2] testsuite: Add dg-require-atomic-exchange " Hans-Peter Nilsson
2023-10-04  8:13     ` Jonathan Wakely
2023-10-04  3:11   ` [PATCH 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-exchange Hans-Peter Nilsson
2023-10-04  8:29     ` Jonathan Wakely
2023-10-04 15:15       ` Hans-Peter Nilsson
2023-10-04 16:01         ` Jonathan Wakely
2023-10-04 17:04         ` [PATCH v2 1/2] testsuite: Add dg-require-atomic-cmpxchg-word Hans-Peter Nilsson
2023-10-12  2:21           ` Ping: " Hans-Peter Nilsson
2023-10-12 14:38             ` Christophe Lyon
2023-10-12 16:10               ` Jeff Law
2023-10-12 22:23                 ` Jonathan Wakely
2024-02-07 16:31                   ` Torbjorn SVENSSON
2024-02-07 16:33                     ` Jonathan Wakely
2024-02-07 17:37                       ` Torbjorn SVENSSON
2023-10-04 17:08         ` [PATCH v2 2/2] testsuite: Replace many dg-require-thread-fence with dg-require-atomic-cmpxchg-word Hans-Peter Nilsson
2023-10-12  2:22           ` Ping: " Hans-Peter Nilsson
2023-10-12 14:40             ` Christophe Lyon
     [not found]   ` <20231003151633.CADF520410@pchp3.se.axis.com>
     [not found]     ` <CAPS5khY5fNB+AuOJOJPT7U6SgyfnvBKc+PNE4jY9oVG7UNOTCg@mail.gmail.com>
     [not found]       ` <20231004004929.9F76B2042E@pchp3.se.axis.com>
2023-10-04  8:53         ` Christophe Lyon [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPS5khavTy-0F6BphpB+-EBf8G6XqW3KEr0aWBCsKzVdrQPpgA@mail.gmail.com \
    --to=christophe.lyon@linaro.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hp@axis.com \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).