On Wed, 4 Oct 2023 at 02:49, Hans-Peter Nilsson <h...@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.l...@linaro.org> > > Date: Tue, 3 Oct 2023 18:06:21 +0200 > > > On Tue, 3 Oct 2023 at 17:16, Hans-Peter Nilsson <h...@axis.com> wrote: > > > > > > From: Christophe Lyon <christophe.l...@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 >