On 11/09/2023 16:22, Jonathan Wakely via Gcc-patches wrote:
> On Mon, 11 Sept 2023 at 14:57, Christophe Lyon
> <christophe.l...@linaro.org> wrote:
>>
>>
>>
>> On Mon, 11 Sept 2023 at 15:12, Jonathan Wakely <jwak...@redhat.com> wrote:
>>>
>>> On Mon, 11 Sept 2023 at 13:36, Christophe Lyon
>>> <christophe.l...@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On Mon, 11 Sept 2023 at 12:59, Jonathan Wakely <jwak...@redhat.com> wrote:
>>>>>
>>>>> On Sun, 10 Sept 2023 at 20:31, Christophe Lyon
>>>>> <christophe.l...@linaro.org> wrote:
>>>>>>
>>>>>> Some targets like arm-eabi with newlib and default settings rely on
>>>>>> __sync_synchronize() to ensure synchronization.  Newlib does not
>>>>>> implement it by default, to make users aware they have to take special
>>>>>> care.
>>>>>>
>>>>>> This makes a few tests fail to link.
>>>>>
>>>>> Does this mean those features are unusable on the target, or just that
>>>>> users need to provide their own __sync_synchronize to use them?
>>>>
>>>>
>>>> IIUC the user is expected to provide them.
>>>> Looks like we discussed this in the past :-)
>>>> In  https://gcc.gnu.org/legacy-ml/gcc-patches/2016-10/msg01632.html,
>>>> see the pointer to Ramana's comment: 
>>>> https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>>>
>>> Oh yes, thanks for the reminder!
>>>
>>>>
>>>> The default arch for arm-eabi is armv4t which is very old.
>>>> When running the testsuite with something more recent (either as default 
>>>> by configuring GCC --with-arch=XXX or by forcing -march/-mcpu via 
>>>> dejagnu's target-board), the compiler generates barrier instructions and 
>>>> there are no such errors.
>>>
>>> Ah yes, that's fine then.
>>>
>>>> For instance, here is a log with the defaults:
>>>> https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-arm_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-arm_eabi
>>>> and a log when we target cortex-m0 which is still a very small cpu but has 
>>>> barriers:
>>>> https://git.linaro.org/toolchain/ci/base-artifacts/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi.git/tree/00-sumfiles?h=linaro-local/ci/tcwg_gnu_embed_check_gcc/master-thumb_m0_eabi
>>>>
>>>> I somehow wanted to get rid of such errors with the default 
>>>> configuration....
>>>
>>> Yep, that makes sense, and we'll still be testing them for newer
>>> arches on the target, so it's not completely disabling those parts of
>>> the testsuite.
>>>
>>> But I'm still curious why some of those tests need this change. I
>>> think the ones I noted below are probably failing for some other
>>> reasons.
>>>
>> Just looked at  23_containers/span/back_assert_neg.cc, the linker says it 
>> needs
>> arm-eabi/libstdc++-v3/src/.libs/libstdc++.a(debug.o) to resolve
>> ./back_assert_neg-back_assert_neg.o (std::__glibcxx_assert_fail(char const*, 
>> int, char const*, char const*))
>> and indeed debug.o has a reference to __sync_synchronize
> 
> Aha, that's just because I put __glibcxx_assert_fail in debug.o, but
> there are no dependencies on anything else in that file, including the
> _M_detach member function that uses atomics.
> 
> This would also be solved by -Wl,--gc-sections :-)
> 
> I think it would be better to move __glibcxx_assert_fail to a new
> file, so that it doesn't make every assertion unnecessarily depend on
> __sync_synchronize. I'll do that now.
> 
> We could also make the atomics in debug.o conditional, so that debug
> mode doesn't depend on __sync_synchronize for single-threaded targets.
> Does the arm4t arch have pthreads support in newlib?  I didn't bother
> making the use of atomics conditional, because performance is not
> really a priority for debug mode bookkeeping. But the problem here
> isn't just a slight performance overhead of atomics, it's that they
> aren't even supported for arm4t.

I might be wrong, but I don't think newlib has any support for pthreads.

R.
> 

Reply via email to