Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-30 Thread Jonathan Wakely

On 30/11/16 09:50 +0100, Christophe Lyon wrote:

I'll try to think about it, but I can live with that single "known" failure.
It's better than the ~3500 failures I used to have, and hopefully
won't report false regressions anymore.


OK, cool. One rather than 3500 is a nice improvement :-)




Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-30 Thread Christophe Lyon
On 29 November 2016 at 21:18, Jonathan Wakely  wrote:
> On 16/11/16 22:18 +0100, Christophe Lyon wrote:
>>
>> On 15 November 2016 at 12:50, Jonathan Wakely  wrote:
>>>
>>> On 14/11/16 14:32 +0100, Christophe Lyon wrote:


 On 20 October 2016 at 19:40, Jonathan Wakely  wrote:
>
>
> On 20/10/16 10:33 -0700, Mike Stump wrote:
>>
>>
>>
>> On Oct 20, 2016, at 9:34 AM, Jonathan Wakely 
>> wrote:
>>>
>>>
>>>
>>>
>>> On 20/10/16 09:26 -0700, Mike Stump wrote:



 On Oct 20, 2016, at 5:20 AM, Jonathan Wakely 
 wrote:
>
>
>
>
> I am considering leaving this in the ARM backend to force people to
> think what they want to do about thread safety with statics and C++
> on bare-metal systems.
>>>
>>>
>>>
>>>
>>> The quoting makes it look like those are my words, but I was quoting
>>> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>>>
 Not quite in the GNU spirit?  The port people should decide the best
 way
 to get as much functionality as possible and everything should just
 work, no
 sharp edges.

 Forcing people to think sounds like a sharp edge?
>>>
>>>
>>>
>>>
>>> I'm inclined to agree, but we are talking about bare metal systems,
>>
>>
>>
>>
>> So?  gcc has been doing bare metal systems for more than 2 years now.
>> It
>> is pretty good at it.  All my primary targets today are themselves
>> bare
>> metal systems (I test with newlib).
>>
>>> where there is no one-size-fits-all solution.
>>
>>
>>
>>
>> Configurations are like ice cream cones.  Everyone gets their flavor
>> no
>> matter how weird or strange.  Putting nails in a cone because you
>> don't
>> know
>> if they like vanilla or chocolate isn't reasonable.  If you want, make
>> two
>> flavors, and vend two, if you want to just do one, pick the flavor and
>> vend
>> it.  Put an enum #define default_flavor vanilla, and you then have
>> support
>> for any flavor you want.  Want to add a configure option for the
>> flavor
>> select, add it.  You want to make a -mflavor=chocolate option, add it.
>> gcc
>> is literally littered with these things.
>
>
>
>
> Like I said, you can either build the library with
> -fno-threadsafe-statics or you can provide a definition of the missing
> symbol.
>
 I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
 It seems to do the trick indeed: almost all tests now pass, the flag is
 added
 to testcase compilation.

 Among the 6 remaining failures, I noticed these two:
 - experimental/type_erased_allocator/2.cc: still complains about the
 missing
 __sync_synchronize. Does it need dg-require-thread-fence?
>>>
>>>
>>>
>>> Yes, I think that test actually uses atomics directly, so does depend
>>> on the fence.
>>>
>> I've attached the patch to achieve this.
>> Is it OK?
>
>
> Yes, OK, thanks.
>
Thanks, committed.

 - abi/header_cxxabi.c complains because the option is not valid for C.
 I can see the test is already skipped for other C++-only options: it is
 OK
 if I submit a patch to skip it if -fno-threadsafe-statics is used?
>>>
>>>
>>>
>>> Yes, it makes sense there too.
>>
>>
>> This one is not as obvious as I hoped. I tried:
>> -// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
>> "-std=gnu++??" } }
>> +// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
>> "-std=gnu++??" "-fno-threadsafe-statics" } }
>>
>> but it does not work.
>>
>> I set CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics
>> before running GCC's configure.
>>
>> This results in -fno-threadsafe-statics being used when compiling the
>> tests,
>> but dg-skip-if does not consider it: it would if I passed it via
>> runtestflags/target-board, but then it would mean passing this flag
>> to all tests, not only the c++ ones, leading to errors everywhere.
>>
>> Am I missing something?
>
>
> I'm not sure how to deal with that.
>
I'll try to think about it, but I can live with that single "known" failure.
It's better than the ~3500 failures I used to have, and hopefully
won't report false regressions anymore.

Christophe


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-29 Thread Jonathan Wakely

On 16/11/16 22:18 +0100, Christophe Lyon wrote:

On 15 November 2016 at 12:50, Jonathan Wakely  wrote:

On 14/11/16 14:32 +0100, Christophe Lyon wrote:


On 20 October 2016 at 19:40, Jonathan Wakely  wrote:


On 20/10/16 10:33 -0700, Mike Stump wrote:



On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:




On 20/10/16 09:26 -0700, Mike Stump wrote:



On Oct 20, 2016, at 5:20 AM, Jonathan Wakely 
wrote:




I am considering leaving this in the ARM backend to force people to
think what they want to do about thread safety with statics and C++
on bare-metal systems.




The quoting makes it look like those are my words, but I was quoting
Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html


Not quite in the GNU spirit?  The port people should decide the best
way
to get as much functionality as possible and everything should just
work, no
sharp edges.

Forcing people to think sounds like a sharp edge?




I'm inclined to agree, but we are talking about bare metal systems,




So?  gcc has been doing bare metal systems for more than 2 years now.
It
is pretty good at it.  All my primary targets today are themselves bare
metal systems (I test with newlib).


where there is no one-size-fits-all solution.




Configurations are like ice cream cones.  Everyone gets their flavor no
matter how weird or strange.  Putting nails in a cone because you don't
know
if they like vanilla or chocolate isn't reasonable.  If you want, make
two
flavors, and vend two, if you want to just do one, pick the flavor and
vend
it.  Put an enum #define default_flavor vanilla, and you then have
support
for any flavor you want.  Want to add a configure option for the flavor
select, add it.  You want to make a -mflavor=chocolate option, add it.
gcc
is literally littered with these things.




Like I said, you can either build the library with
-fno-threadsafe-statics or you can provide a definition of the missing
symbol.


I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
It seems to do the trick indeed: almost all tests now pass, the flag is
added
to testcase compilation.

Among the 6 remaining failures, I noticed these two:
- experimental/type_erased_allocator/2.cc: still complains about the
missing
__sync_synchronize. Does it need dg-require-thread-fence?



Yes, I think that test actually uses atomics directly, so does depend
on the fence.


I've attached the patch to achieve this.
Is it OK?


Yes, OK, thanks.


- abi/header_cxxabi.c complains because the option is not valid for C.
I can see the test is already skipped for other C++-only options: it is OK
if I submit a patch to skip it if -fno-threadsafe-statics is used?



Yes, it makes sense there too.


This one is not as obvious as I hoped. I tried:
-// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
"-std=gnu++??" } }
+// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
"-std=gnu++??" "-fno-threadsafe-statics" } }

but it does not work.

I set CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics
before running GCC's configure.

This results in -fno-threadsafe-statics being used when compiling the tests,
but dg-skip-if does not consider it: it would if I passed it via
runtestflags/target-board, but then it would mean passing this flag
to all tests, not only the c++ ones, leading to errors everywhere.

Am I missing something?


I'm not sure how to deal with that.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-28 Thread Christophe Lyon
Ping?

On 16 November 2016 at 22:18, Christophe Lyon
 wrote:
> On 15 November 2016 at 12:50, Jonathan Wakely  wrote:
>> On 14/11/16 14:32 +0100, Christophe Lyon wrote:
>>>
>>> On 20 October 2016 at 19:40, Jonathan Wakely  wrote:

 On 20/10/16 10:33 -0700, Mike Stump wrote:
>
>
> On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:
>>
>>
>>
>> On 20/10/16 09:26 -0700, Mike Stump wrote:
>>>
>>>
>>> On Oct 20, 2016, at 5:20 AM, Jonathan Wakely 
>>> wrote:



 I am considering leaving this in the ARM backend to force people to
 think what they want to do about thread safety with statics and C++
 on bare-metal systems.
>>
>>
>>
>> The quoting makes it look like those are my words, but I was quoting
>> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>>
>>> Not quite in the GNU spirit?  The port people should decide the best
>>> way
>>> to get as much functionality as possible and everything should just
>>> work, no
>>> sharp edges.
>>>
>>> Forcing people to think sounds like a sharp edge?
>>
>>
>>
>> I'm inclined to agree, but we are talking about bare metal systems,
>
>
>
> So?  gcc has been doing bare metal systems for more than 2 years now.
> It
> is pretty good at it.  All my primary targets today are themselves bare
> metal systems (I test with newlib).
>
>> where there is no one-size-fits-all solution.
>
>
>
> Configurations are like ice cream cones.  Everyone gets their flavor no
> matter how weird or strange.  Putting nails in a cone because you don't
> know
> if they like vanilla or chocolate isn't reasonable.  If you want, make
> two
> flavors, and vend two, if you want to just do one, pick the flavor and
> vend
> it.  Put an enum #define default_flavor vanilla, and you then have
> support
> for any flavor you want.  Want to add a configure option for the flavor
> select, add it.  You want to make a -mflavor=chocolate option, add it.
> gcc
> is literally littered with these things.



 Like I said, you can either build the library with
 -fno-threadsafe-statics or you can provide a definition of the missing
 symbol.

>>> I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
>>> It seems to do the trick indeed: almost all tests now pass, the flag is
>>> added
>>> to testcase compilation.
>>>
>>> Among the 6 remaining failures, I noticed these two:
>>> - experimental/type_erased_allocator/2.cc: still complains about the
>>> missing
>>> __sync_synchronize. Does it need dg-require-thread-fence?
>>
>>
>> Yes, I think that test actually uses atomics directly, so does depend
>> on the fence.
>>
> I've attached the patch to achieve this.
> Is it OK?
>
>>> - abi/header_cxxabi.c complains because the option is not valid for C.
>>> I can see the test is already skipped for other C++-only options: it is OK
>>> if I submit a patch to skip it if -fno-threadsafe-statics is used?
>>
>>
>> Yes, it makes sense there too.
>
> This one is not as obvious as I hoped. I tried:
> -// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
> "-std=gnu++??" } }
> +// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
> "-std=gnu++??" "-fno-threadsafe-statics" } }
>
> but it does not work.
>
> I set CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics
> before running GCC's configure.
>
> This results in -fno-threadsafe-statics being used when compiling the tests,
> but dg-skip-if does not consider it: it would if I passed it via
> runtestflags/target-board, but then it would mean passing this flag
> to all tests, not only the c++ ones, leading to errors everywhere.
>
> Am I missing something?
>
> Thanks,
>
> Christophe
>
>>> I think I'm going to use this flag in validations from now on (target
>>> arm-none-eabi
>>> only, with default mode/cpu/fpu).
>>
>>
>> Thanks for the update on this.
>>


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-16 Thread Christophe Lyon
On 15 November 2016 at 12:50, Jonathan Wakely  wrote:
> On 14/11/16 14:32 +0100, Christophe Lyon wrote:
>>
>> On 20 October 2016 at 19:40, Jonathan Wakely  wrote:
>>>
>>> On 20/10/16 10:33 -0700, Mike Stump wrote:


 On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:
>
>
>
> On 20/10/16 09:26 -0700, Mike Stump wrote:
>>
>>
>> On Oct 20, 2016, at 5:20 AM, Jonathan Wakely 
>> wrote:
>>>
>>>
>>>
>>> I am considering leaving this in the ARM backend to force people to
>>> think what they want to do about thread safety with statics and C++
>>> on bare-metal systems.
>
>
>
> The quoting makes it look like those are my words, but I was quoting
> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>
>> Not quite in the GNU spirit?  The port people should decide the best
>> way
>> to get as much functionality as possible and everything should just
>> work, no
>> sharp edges.
>>
>> Forcing people to think sounds like a sharp edge?
>
>
>
> I'm inclined to agree, but we are talking about bare metal systems,



 So?  gcc has been doing bare metal systems for more than 2 years now.
 It
 is pretty good at it.  All my primary targets today are themselves bare
 metal systems (I test with newlib).

> where there is no one-size-fits-all solution.



 Configurations are like ice cream cones.  Everyone gets their flavor no
 matter how weird or strange.  Putting nails in a cone because you don't
 know
 if they like vanilla or chocolate isn't reasonable.  If you want, make
 two
 flavors, and vend two, if you want to just do one, pick the flavor and
 vend
 it.  Put an enum #define default_flavor vanilla, and you then have
 support
 for any flavor you want.  Want to add a configure option for the flavor
 select, add it.  You want to make a -mflavor=chocolate option, add it.
 gcc
 is literally littered with these things.
>>>
>>>
>>>
>>> Like I said, you can either build the library with
>>> -fno-threadsafe-statics or you can provide a definition of the missing
>>> symbol.
>>>
>> I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
>> It seems to do the trick indeed: almost all tests now pass, the flag is
>> added
>> to testcase compilation.
>>
>> Among the 6 remaining failures, I noticed these two:
>> - experimental/type_erased_allocator/2.cc: still complains about the
>> missing
>> __sync_synchronize. Does it need dg-require-thread-fence?
>
>
> Yes, I think that test actually uses atomics directly, so does depend
> on the fence.
>
I've attached the patch to achieve this.
Is it OK?

>> - abi/header_cxxabi.c complains because the option is not valid for C.
>> I can see the test is already skipped for other C++-only options: it is OK
>> if I submit a patch to skip it if -fno-threadsafe-statics is used?
>
>
> Yes, it makes sense there too.

This one is not as obvious as I hoped. I tried:
-// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
"-std=gnu++??" } }
+// { dg-skip-if "invalid options for C" { *-*-* } { "-std=c++??"
"-std=gnu++??" "-fno-threadsafe-statics" } }

but it does not work.

I set CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics
before running GCC's configure.

This results in -fno-threadsafe-statics being used when compiling the tests,
but dg-skip-if does not consider it: it would if I passed it via
runtestflags/target-board, but then it would mean passing this flag
to all tests, not only the c++ ones, leading to errors everywhere.

Am I missing something?

Thanks,

Christophe

>> I think I'm going to use this flag in validations from now on (target
>> arm-none-eabi
>> only, with default mode/cpu/fpu).
>
>
> Thanks for the update on this.
>
libstdc++-v3/ChangeLog:

2016-11-16  Christophe Lyon  

* testsuite/experimental/type_erased_allocator/2.cc: Add
  dg-require-thread-fence.

diff --git a/libstdc++-v3/testsuite/experimental/type_erased_allocator/2.cc 
b/libstdc++-v3/testsuite/experimental/type_erased_allocator/2.cc
index 216a88c..0b73359 100644
--- a/libstdc++-v3/testsuite/experimental/type_erased_allocator/2.cc
+++ b/libstdc++-v3/testsuite/experimental/type_erased_allocator/2.cc
@@ -1,4 +1,5 @@
 // { dg-do run { target c++14 } }
+// { dg-require-thread-fence "" }
 
 // Copyright (C) 2015-2016 Free Software Foundation, Inc.
 //


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-15 Thread Jonathan Wakely

On 14/11/16 14:32 +0100, Christophe Lyon wrote:

On 20 October 2016 at 19:40, Jonathan Wakely  wrote:

On 20/10/16 10:33 -0700, Mike Stump wrote:


On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:



On 20/10/16 09:26 -0700, Mike Stump wrote:


On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:



I am considering leaving this in the ARM backend to force people to
think what they want to do about thread safety with statics and C++
on bare-metal systems.



The quoting makes it look like those are my words, but I was quoting
Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html


Not quite in the GNU spirit?  The port people should decide the best way
to get as much functionality as possible and everything should just work, no
sharp edges.

Forcing people to think sounds like a sharp edge?



I'm inclined to agree, but we are talking about bare metal systems,



So?  gcc has been doing bare metal systems for more than 2 years now.  It
is pretty good at it.  All my primary targets today are themselves bare
metal systems (I test with newlib).


where there is no one-size-fits-all solution.



Configurations are like ice cream cones.  Everyone gets their flavor no
matter how weird or strange.  Putting nails in a cone because you don't know
if they like vanilla or chocolate isn't reasonable.  If you want, make two
flavors, and vend two, if you want to just do one, pick the flavor and vend
it.  Put an enum #define default_flavor vanilla, and you then have support
for any flavor you want.  Want to add a configure option for the flavor
select, add it.  You want to make a -mflavor=chocolate option, add it.  gcc
is literally littered with these things.



Like I said, you can either build the library with
-fno-threadsafe-statics or you can provide a definition of the missing
symbol.


I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
It seems to do the trick indeed: almost all tests now pass, the flag is added
to testcase compilation.

Among the 6 remaining failures, I noticed these two:
- experimental/type_erased_allocator/2.cc: still complains about the missing
__sync_synchronize. Does it need dg-require-thread-fence?


Yes, I think that test actually uses atomics directly, so does depend
on the fence.


- abi/header_cxxabi.c complains because the option is not valid for C.
I can see the test is already skipped for other C++-only options: it is OK
if I submit a patch to skip it if -fno-threadsafe-statics is used?


Yes, it makes sense there too.


I think I'm going to use this flag in validations from now on (target
arm-none-eabi
only, with default mode/cpu/fpu).


Thanks for the update on this.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-15 Thread Christophe Lyon
On 14 November 2016 at 21:31, Ramana Radhakrishnan
 wrote:
>
> On Mon, 14 Nov 2016 at 19:59, Christophe Lyon 
> wrote:
>>
>> On 14 November 2016 at 18:54, Mike Stump  wrote:
>> > On Oct 21, 2016, at 1:00 AM, Christophe Lyon
>> >  wrote:
>> >>
>> >> So if we say that the current behaviour has to keep being the default,
>> >> so that users think about what they are really doing,
>> >
>> > Having a toolchain not work by default to force users to think, isn't a
>> > winning strategy.
>> >
>> > Everything should always, just work.  Those things that don't, we should
>> > fix.
>> >
>> I tend to agree :-)
>>
>> Maybe Ramana changed his mind and would now no longer want to force
>> users to think?
>
>
>
> I haven't been able to deal with this thread having being in and out of the
> office for the past month thanks to various reasons. I am not back at my
> desk until next week for various reasons and ran out of time when I was at
> my desk to get back to this and actually fix the comments in newlib patch
> review.
>
>
> https://sourceware.org/ml/newlib/2015/msg00653.html
>

Thanks for the pointer, I missed it.

> This seems to have dropped between the cracks for various reasons but that
> was the approach I was going for. Some of the points made are taken, but
> having users not think about what they want to do about synchronisation and
> just provide empty stub functions which result in random run time crashes
> aren't correct in my book. If anyone is interested in moving forward I would
> suggest they take that approach or refine it further.
>
>
> Thanks,
> Ramana
>


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-14 Thread Mike Stump
On Nov 14, 2016, at 12:31 PM, Ramana Radhakrishnan  
wrote:
> 
> https://sourceware.org/ml/newlib/2015/msg00653.html

I think that patch is wrong.  It is wrong to warn on a system where an empty 
body is correct.  It is wrong to warn when something more than nothing needs to 
be done.  In short, it is never right.

If you implement what is required for any machine (configuration), at least it 
will be right for that configuration.  Others where that isn't correct, can 
then implement what is correct for their machine (configuration), if you cannot.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-14 Thread Christophe Lyon
On 14 November 2016 at 18:54, Mike Stump  wrote:
> On Oct 21, 2016, at 1:00 AM, Christophe Lyon  
> wrote:
>>
>> So if we say that the current behaviour has to keep being the default,
>> so that users think about what they are really doing,
>
> Having a toolchain not work by default to force users to think, isn't a 
> winning strategy.
>
> Everything should always, just work.  Those things that don't, we should fix.
>
I tend to agree :-)

Maybe Ramana changed his mind and would now no longer want to force
users to think?


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-14 Thread Mike Stump
On Oct 21, 2016, at 1:00 AM, Christophe Lyon  wrote:
> 
> So if we say that the current behaviour has to keep being the default,
> so that users think about what they are really doing, 

Having a toolchain not work by default to force users to think, isn't a winning 
strategy.

Everything should always, just work.  Those things that don't, we should fix.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-11-14 Thread Christophe Lyon
On 20 October 2016 at 19:40, Jonathan Wakely  wrote:
> On 20/10/16 10:33 -0700, Mike Stump wrote:
>>
>> On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:
>>>
>>>
>>> On 20/10/16 09:26 -0700, Mike Stump wrote:

 On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:
>
>
> I am considering leaving this in the ARM backend to force people to
> think what they want to do about thread safety with statics and C++
> on bare-metal systems.
>>>
>>>
>>> The quoting makes it look like those are my words, but I was quoting
>>> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>>>
 Not quite in the GNU spirit?  The port people should decide the best way
 to get as much functionality as possible and everything should just work, 
 no
 sharp edges.

 Forcing people to think sounds like a sharp edge?
>>>
>>>
>>> I'm inclined to agree, but we are talking about bare metal systems,
>>
>>
>> So?  gcc has been doing bare metal systems for more than 2 years now.  It
>> is pretty good at it.  All my primary targets today are themselves bare
>> metal systems (I test with newlib).
>>
>>> where there is no one-size-fits-all solution.
>>
>>
>> Configurations are like ice cream cones.  Everyone gets their flavor no
>> matter how weird or strange.  Putting nails in a cone because you don't know
>> if they like vanilla or chocolate isn't reasonable.  If you want, make two
>> flavors, and vend two, if you want to just do one, pick the flavor and vend
>> it.  Put an enum #define default_flavor vanilla, and you then have support
>> for any flavor you want.  Want to add a configure option for the flavor
>> select, add it.  You want to make a -mflavor=chocolate option, add it.  gcc
>> is literally littered with these things.
>
>
> Like I said, you can either build the library with
> -fno-threadsafe-statics or you can provide a definition of the missing
> symbol.
>
I gave this a try (using CXXFLAGS_FOR_TARGET=-fno-threadsafe-statics).
It seems to do the trick indeed: almost all tests now pass, the flag is added
to testcase compilation.

Among the 6 remaining failures, I noticed these two:
- experimental/type_erased_allocator/2.cc: still complains about the missing
__sync_synchronize. Does it need dg-require-thread-fence?

- abi/header_cxxabi.c complains because the option is not valid for C.
I can see the test is already skipped for other C++-only options: it is OK
if I submit a patch to skip it if -fno-threadsafe-statics is used?


I think I'm going to use this flag in validations from now on (target
arm-none-eabi
only, with default mode/cpu/fpu).

Thanks,

Christophe

>> Anything vended should just work.  If it doesn't, that's a bug that needs
>> fixing.  If a port person doesn't understand, we can educate them why _it
>> just works_, is a nice design philosophy; maybe it is new to them.
>
>
> Which is basically what I'm saying. Marking 3000 tests UNSUPPORTED to
> make some test results look clean is not a fix for anything.
>
>
>>> Choosing something that makes most of the library unusable will upset one
>>> group of people, and
>>> choosing something that adds overhead that could be avoided will upset
>>> another group.
>>
>>
>> No, this is a misunderstanding.  Users want things to just work, really.
>> Bosses often like it when things just work as well; so it's not just users.
>> Customers often like it as well.  Anyway, that's my experience.
>
>
>
> OK, I'll put it another way. Under no circumstances am I going to
> accept a patch that requires adding the same redundant directive to
> every single 'do-dg run' test in libstdc++-v3/testsuite/.
>
> Right now I don't care how or if the FAILs get fixed, but it won't be
> by individually marking every file as UNSUPPORTED.
>
>


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-21 Thread Kyrill Tkachov

Hi all,

On 21/10/16 09:00, Christophe Lyon wrote:

[ccying Ramana]


Ramana is currently OoO all of this week.

Kyrill


On 20 October 2016 at 18:34, Jonathan Wakely  wrote:

On 20/10/16 09:26 -0700, Mike Stump wrote:

On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:


I am considering leaving this in the ARM backend to force people to
think what they want to do about thread safety with statics and C++
on bare-metal systems.


The quoting makes it look like those are my words, but I was quoting
Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html


Not quite in the GNU spirit?  The port people should decide the best way
to get as much functionality as possible and everything should just work, no
sharp edges.

Forcing people to think sounds like a sharp edge?


I'm inclined to agree, but we are talking about bare metal systems,
where there is no one-size-fits-all solution. Choosing something that
makes most of the library unusable will upset one group of people, and
choosing something that adds overhead that could be avoided will upset
another group.

Either way, I don't think disabling 50% of the testsuite is the
answer. If you don't like the failures, configure the library to build
without threadsafe statics, or configure it to depend on libatomic, or
something else. (We might want new --enable-xxx switches to simplify
doing that).


So if we say that the current behaviour has to keep being the default,
so that users think about what they are really doing, I can certainly
patch my validation scripts to add a configure flag for this particular
configuration.

Is arm-none-eabi the only target having this problem?

Thanks,

Christophe





Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-21 Thread Christophe Lyon
[ccying Ramana]

On 20 October 2016 at 18:34, Jonathan Wakely  wrote:
> On 20/10/16 09:26 -0700, Mike Stump wrote:
>>
>> On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:
>>>
>>>
>>> I am considering leaving this in the ARM backend to force people to
>>> think what they want to do about thread safety with statics and C++
>>> on bare-metal systems.
>
>
> The quoting makes it look like those are my words, but I was quoting
> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
>
>> Not quite in the GNU spirit?  The port people should decide the best way
>> to get as much functionality as possible and everything should just work, no
>> sharp edges.
>>
>> Forcing people to think sounds like a sharp edge?
>
>
> I'm inclined to agree, but we are talking about bare metal systems,
> where there is no one-size-fits-all solution. Choosing something that
> makes most of the library unusable will upset one group of people, and
> choosing something that adds overhead that could be avoided will upset
> another group.
>
> Either way, I don't think disabling 50% of the testsuite is the
> answer. If you don't like the failures, configure the library to build
> without threadsafe statics, or configure it to depend on libatomic, or
> something else. (We might want new --enable-xxx switches to simplify
> doing that).
>

So if we say that the current behaviour has to keep being the default,
so that users think about what they are really doing, I can certainly
patch my validation scripts to add a configure flag for this particular
configuration.

Is arm-none-eabi the only target having this problem?

Thanks,

Christophe


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-21 Thread Christophe Lyon
On 20 October 2016 at 18:51, Jonathan Wakely  wrote:
> On 20/10/16 10:40 +0100, Jonathan Wakely wrote:
>>
>> Please CC the libstdc++ list for libstdc++ patches, this is a
>> requirement for patch submission, see https://gcc.gnu.org/lists.html
>>
>>
>> On 20/10/16 09:55 +0200, Christophe Lyon wrote:
>>>
>>> Hi,
>>>
>>> Several times I have noticed/reported test failures because some test
>>> cases wouldn't link on arm-none-eabi using the default 'old' cpu
>>> target: __sync_synchronize cannot be resolved by the linker.
>>
>>
>> That isn't used by libstdc++ anywhere, so the call to it is being
>> emitted by the compiler. It would be good to know where it comes from.
>>
>>
>>> The attached long patch adds
>>> +// { dg-require-thread-fence "" }
>>> to all the tests that require it according to the error messages I get.
>>>
>>> The change is mechanical:
>>> - insert this line below dg-do if present
>>> - insert this line at the top of the file otherwise
>>>
>>> For instance:
>>>
>>> diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>>> b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>>> index 633175b..a048250 100644
>>> --- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>>> +++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>>> @@ -1,3 +1,4 @@
>>> +// { dg-require-thread-fence "" }
>>> // 2007-01-30  Paolo Carlini  
>>>
>>> // Copyright (C) 2007-2016 Free Software Foundation, Inc.
>>> diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>>> b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>>> index e712655..f2a6c5a 100644
>>> --- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>>> +++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>>> @@ -1,4 +1,5 @@
>>> // { dg-do run }
>>> +// { dg-require-thread-fence "" }
>>> // Avoid use of non-overridable new/delete operators in shared
>>> // { dg-options "-static" { target *-*-mingw* } }
>>> // Test __cxa_vec routines
>>>
>>>
>>> If that's OK, I'm not sure how to write the ChangeLog entry: it
>>> modifies 3287 files.
>>>
>>> In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.
>>
>>
>> Wouldn't it be better to remove the dependency on __sync_synchronize
>> rather than declare nearly 50% of the testsuite UNSUPPORTED?
>>
>> If 3287 tests can't use it is the resulting libstdc++.so actually
>> useful to anyone?
>
>
> Are those *all* the tests that link to libstdc++.so / libstdc++.a and
> aren't disabled for arm-none-eabi by some other directive? It would be
> about the right number.
>
> If Every. Single. Test. that uses the libstdc++ library has this
> failure, and the library can't be made to be usable, the answer is
> surely to change the meaning of "dg-do run" to not link+run tests, not
> add a new directive to Every. Single. Test.  (and every single test we
> add in future too!)
>
I'm not sure what you really mean here.
I can see 147 execution tests pass, 1 fail.
and 3287 fail to link.

So that's most of the executable tests, yes.

And I agree that my patch is not a viable solution.


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

On 20/10/16 10:33 -0700, Mike Stump wrote:

On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:


On 20/10/16 09:26 -0700, Mike Stump wrote:

On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:


I am considering leaving this in the ARM backend to force people to
think what they want to do about thread safety with statics and C++
on bare-metal systems.


The quoting makes it look like those are my words, but I was quoting
Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html


Not quite in the GNU spirit?  The port people should decide the best way to get 
as much functionality as possible and everything should just work, no sharp 
edges.

Forcing people to think sounds like a sharp edge?


I'm inclined to agree, but we are talking about bare metal systems,


So?  gcc has been doing bare metal systems for more than 2 years now.  It is 
pretty good at it.  All my primary targets today are themselves bare metal 
systems (I test with newlib).


where there is no one-size-fits-all solution.


Configurations are like ice cream cones.  Everyone gets their flavor no matter 
how weird or strange.  Putting nails in a cone because you don't know if they 
like vanilla or chocolate isn't reasonable.  If you want, make two flavors, and 
vend two, if you want to just do one, pick the flavor and vend it.  Put an enum 
#define default_flavor vanilla, and you then have support for any flavor you 
want.  Want to add a configure option for the flavor select, add it.  You want 
to make a -mflavor=chocolate option, add it.  gcc is literally littered with 
these things.


Like I said, you can either build the library with
-fno-threadsafe-statics or you can provide a definition of the missing
symbol.


Anything vended should just work.  If it doesn't, that's a bug that needs 
fixing.  If a port person doesn't understand, we can educate them why _it just 
works_, is a nice design philosophy; maybe it is new to them.


Which is basically what I'm saying. Marking 3000 tests UNSUPPORTED to
make some test results look clean is not a fix for anything.



Choosing something that makes most of the library unusable will upset one group 
of people, and
choosing something that adds overhead that could be avoided will upset
another group.


No, this is a misunderstanding.  Users want things to just work, really.  
Bosses often like it when things just work as well; so it's not just users.  
Customers often like it as well.  Anyway, that's my experience.



OK, I'll put it another way. Under no circumstances am I going to
accept a patch that requires adding the same redundant directive to
every single 'do-dg run' test in libstdc++-v3/testsuite/.

Right now I don't care how or if the FAILs get fixed, but it won't be
by individually marking every file as UNSUPPORTED.




Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Mike Stump
On Oct 20, 2016, at 9:34 AM, Jonathan Wakely  wrote:
> 
> On 20/10/16 09:26 -0700, Mike Stump wrote:
>> On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:
>>> 
>>> I am considering leaving this in the ARM backend to force people to
>>> think what they want to do about thread safety with statics and C++
>>> on bare-metal systems.
> 
> The quoting makes it look like those are my words, but I was quoting
> Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
> 
>> Not quite in the GNU spirit?  The port people should decide the best way to 
>> get as much functionality as possible and everything should just work, no 
>> sharp edges.
>> 
>> Forcing people to think sounds like a sharp edge?
> 
> I'm inclined to agree, but we are talking about bare metal systems,

So?  gcc has been doing bare metal systems for more than 2 years now.  It is 
pretty good at it.  All my primary targets today are themselves bare metal 
systems (I test with newlib).

> where there is no one-size-fits-all solution.

Configurations are like ice cream cones.  Everyone gets their flavor no matter 
how weird or strange.  Putting nails in a cone because you don't know if they 
like vanilla or chocolate isn't reasonable.  If you want, make two flavors, and 
vend two, if you want to just do one, pick the flavor and vend it.  Put an enum 
#define default_flavor vanilla, and you then have support for any flavor you 
want.  Want to add a configure option for the flavor select, add it.  You want 
to make a -mflavor=chocolate option, add it.  gcc is literally littered with 
these things.

Anything vended should just work.  If it doesn't, that's a bug that needs 
fixing.  If a port person doesn't understand, we can educate them why _it just 
works_, is a nice design philosophy; maybe it is new to them.

> Choosing something that makes most of the library unusable will upset one 
> group of people, and
> choosing something that adds overhead that could be avoided will upset
> another group.

No, this is a misunderstanding.  Users want things to just work, really.  
Bosses often like it when things just work as well; so it's not just users.  
Customers often like it as well.  Anyway, that's my experience.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Ville Voutilainen
On 20 October 2016 at 20:08, Mike Stump  wrote:
> So, from a test suite perspective, the correct fix it to make the port just 
> work.  I have a bare metal port, I test libstdc++.
> I'd be curious to hear from the arm folks about it.


I daresay that would be the correct fix from many other perspectives
besides just from a test suite one. :)


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Mike Stump
On Oct 20, 2016, at 9:51 AM, Jonathan Wakely  wrote:
> If Every. Single. Test. that uses the libstdc++ library has this
> failure, and the library can't be made to be usable, the answer is
> surely to change the meaning of "dg-do run" to not link+run tests, not
> add a new directive to Every. Single. Test.  (and every single test we
> add in future too!)

So, from a test suite perspective, the correct fix it to make the port just 
work.  I have a bare metal port, I test libstdc++.
I'd be curious to hear from the arm folks about it.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Ville Voutilainen
On 20 October 2016 at 19:51, Jonathan Wakely  wrote:
> add a new directive to Every. Single. Test.  (and every single test we
> add in future too!)


Uh, that would be a rather unfortunate burden for every library patch
submitter, and to the maintainer
as well, because we _will_ forget it and then it will break bare-metal
arm on every patch.
Let's please figure out a better way to solve this problem.


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Ville Voutilainen
On 20 October 2016 at 19:34, Jonathan Wakely  wrote:
> Either way, I don't think disabling 50% of the testsuite is the
> answer. If you don't like the failures, configure the library to build
> without threadsafe statics, or configure it to depend on libatomic, or
> something else. (We might want new --enable-xxx switches to simplify
> doing that).
>

I think having to add a dg-require to *every* run-time test we have,
even and especially to ones that have *nothing* to do with
any threading is an indication that this approach might not be a good
way to solve the problem.


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

On 20/10/16 10:40 +0100, Jonathan Wakely wrote:

Please CC the libstdc++ list for libstdc++ patches, this is a
requirement for patch submission, see https://gcc.gnu.org/lists.html


On 20/10/16 09:55 +0200, Christophe Lyon wrote:

Hi,

Several times I have noticed/reported test failures because some test
cases wouldn't link on arm-none-eabi using the default 'old' cpu
target: __sync_synchronize cannot be resolved by the linker.


That isn't used by libstdc++ anywhere, so the call to it is being
emitted by the compiler. It would be good to know where it comes from.



The attached long patch adds
+// { dg-require-thread-fence "" }
to all the tests that require it according to the error messages I get.

The change is mechanical:
- insert this line below dg-do if present
- insert this line at the top of the file otherwise

For instance:

diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
index 633175b..a048250 100644
--- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
+++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
@@ -1,3 +1,4 @@
+// { dg-require-thread-fence "" }
// 2007-01-30  Paolo Carlini  

// Copyright (C) 2007-2016 Free Software Foundation, Inc.
diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
index e712655..f2a6c5a 100644
--- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
+++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
@@ -1,4 +1,5 @@
// { dg-do run }
+// { dg-require-thread-fence "" }
// Avoid use of non-overridable new/delete operators in shared
// { dg-options "-static" { target *-*-mingw* } }
// Test __cxa_vec routines


If that's OK, I'm not sure how to write the ChangeLog entry: it
modifies 3287 files.

In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.


Wouldn't it be better to remove the dependency on __sync_synchronize
rather than declare nearly 50% of the testsuite UNSUPPORTED?

If 3287 tests can't use it is the resulting libstdc++.so actually
useful to anyone?


Are those *all* the tests that link to libstdc++.so / libstdc++.a and
aren't disabled for arm-none-eabi by some other directive? It would be
about the right number.

If Every. Single. Test. that uses the libstdc++ library has this
failure, and the library can't be made to be usable, the answer is
surely to change the meaning of "dg-do run" to not link+run tests, not
add a new directive to Every. Single. Test.  (and every single test we
add in future too!)




Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

On 20/10/16 09:26 -0700, Mike Stump wrote:

On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:


I am considering leaving this in the ARM backend to force people to
think what they want to do about thread safety with statics and C++
on bare-metal systems.


The quoting makes it look like those are my words, but I was quoting
Ramana from https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html


Not quite in the GNU spirit?  The port people should decide the best way to get 
as much functionality as possible and everything should just work, no sharp 
edges.

Forcing people to think sounds like a sharp edge?


I'm inclined to agree, but we are talking about bare metal systems,
where there is no one-size-fits-all solution. Choosing something that
makes most of the library unusable will upset one group of people, and
choosing something that adds overhead that could be avoided will upset
another group.

Either way, I don't think disabling 50% of the testsuite is the
answer. If you don't like the failures, configure the library to build
without threadsafe statics, or configure it to depend on libatomic, or
something else. (We might want new --enable-xxx switches to simplify
doing that).



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Mike Stump
On Oct 20, 2016, at 5:20 AM, Jonathan Wakely  wrote:
> 
> I am considering leaving this in the ARM backend to force people to
> think what they want to do about thread safety with statics and C++
> on bare-metal systems.

Not quite in the GNU spirit?  The port people should decide the best way to get 
as much functionality as possible and everything should just work, no sharp 
edges.

Forcing people to think sounds like a sharp edge?



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

On 20/10/16 14:01 +0200, Christophe Lyon wrote:

On 20 October 2016 at 11:40, Jonathan Wakely  wrote:

Please CC the libstdc++ list for libstdc++ patches, this is a
requirement for patch submission, see https://gcc.gnu.org/lists.html


OK, I thought I wasn't really modifying the lib itself :-)



On 20/10/16 09:55 +0200, Christophe Lyon wrote:


Hi,

Several times I have noticed/reported test failures because some test
cases wouldn't link on arm-none-eabi using the default 'old' cpu
target: __sync_synchronize cannot be resolved by the linker.



That isn't used by libstdc++ anywhere, so the call to it is being
emitted by the compiler. It would be good to know where it comes from.




The attached long patch adds
+// { dg-require-thread-fence "" }
to all the tests that require it according to the error messages I get.

The change is mechanical:
- insert this line below dg-do if present
- insert this line at the top of the file otherwise

For instance:

diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
index 633175b..a048250 100644
--- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
+++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
@@ -1,3 +1,4 @@
+// { dg-require-thread-fence "" }
// 2007-01-30  Paolo Carlini  

// Copyright (C) 2007-2016 Free Software Foundation, Inc.
diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
index e712655..f2a6c5a 100644
--- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
+++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
@@ -1,4 +1,5 @@
// { dg-do run }
+// { dg-require-thread-fence "" }
// Avoid use of non-overridable new/delete operators in shared
// { dg-options "-static" { target *-*-mingw* } }
// Test __cxa_vec routines


If that's OK, I'm not sure how to write the ChangeLog entry: it
modifies 3287 files.

In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.



Wouldn't it be better to remove the dependency on __sync_synchronize
rather than declare nearly 50% of the testsuite UNSUPPORTED?

If 3287 tests can't use it is the resulting libstdc++.so actually
useful to anyone?



I thought I would follow the discussion in
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01196.html
when this directive was introduced.


It makes sense to disable tests for atomics if the target doesn't
support atomics, which was the original purpose of that directive.

But I'm concerned about disabling tests for thousands of tests that
have nothing to do with atomics, like
18_support/numeric_limits/char16_32_t.cc 




Furthermore, I saw Ramana's comment in
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
and I assumed the use of _sync_synchronize() was on
purpose.


Ah, so this explains where it's coming from. Any local static variable
that needs a guard will emit a reference to __sync_synchronize. As
Ramana said:

 I am considering leaving this in the ARM backend to force people to
 think what they want to do about thread safety with statics and C++
 on bare-metal systems. If they really do not want thread safety they
 can well add -fno-threadsafe-statics or provide an appropriate
 implementation for __sync_synchronize on their platforms. 


So if libstdc++ is built without -fno-threadsafe-statics for
bare-metal then it needs to link to libatomic or find some other
definition of __sync_synchronize. Alternatively, we could build it
with -fno-threadsafe-statics. That would allow 99% of the library (and
the testsuite) to work correctly.

We might want to review the library for any cases where we are relying
on -fthreadsafe-statics and conditionally perform initialization some
other way, e.g. pthread_once.



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Christophe Lyon
On 20 October 2016 at 11:40, Jonathan Wakely  wrote:
> Please CC the libstdc++ list for libstdc++ patches, this is a
> requirement for patch submission, see https://gcc.gnu.org/lists.html
>
OK, I thought I wasn't really modifying the lib itself :-)

>
> On 20/10/16 09:55 +0200, Christophe Lyon wrote:
>>
>> Hi,
>>
>> Several times I have noticed/reported test failures because some test
>> cases wouldn't link on arm-none-eabi using the default 'old' cpu
>> target: __sync_synchronize cannot be resolved by the linker.
>
>
> That isn't used by libstdc++ anywhere, so the call to it is being
> emitted by the compiler. It would be good to know where it comes from.
>
>
>
>> The attached long patch adds
>> +// { dg-require-thread-fence "" }
>> to all the tests that require it according to the error messages I get.
>>
>> The change is mechanical:
>> - insert this line below dg-do if present
>> - insert this line at the top of the file otherwise
>>
>> For instance:
>>
>> diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> index 633175b..a048250 100644
>> --- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> +++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> @@ -1,3 +1,4 @@
>> +// { dg-require-thread-fence "" }
>> // 2007-01-30  Paolo Carlini  
>>
>> // Copyright (C) 2007-2016 Free Software Foundation, Inc.
>> diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> index e712655..f2a6c5a 100644
>> --- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> +++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> @@ -1,4 +1,5 @@
>> // { dg-do run }
>> +// { dg-require-thread-fence "" }
>> // Avoid use of non-overridable new/delete operators in shared
>> // { dg-options "-static" { target *-*-mingw* } }
>> // Test __cxa_vec routines
>>
>>
>> If that's OK, I'm not sure how to write the ChangeLog entry: it
>> modifies 3287 files.
>>
>> In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.
>
>
> Wouldn't it be better to remove the dependency on __sync_synchronize
> rather than declare nearly 50% of the testsuite UNSUPPORTED?
>
> If 3287 tests can't use it is the resulting libstdc++.so actually
> useful to anyone?
>

I thought I would follow the discussion in
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01196.html
when this directive was introduced.

Furthermore, I saw Ramana's comment in
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
and I assumed the use of _sync_synchronize() was on
purpose.

I'm happy to prepare a patch with a better fix, as I'd like
to get rid of these errors.

Thanks,

Christophe


Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

On 20/10/16 10:40 +0100, Jonathan Wakely wrote:

Please CC the libstdc++ list for libstdc++ patches, this is a
requirement for patch submission, see https://gcc.gnu.org/lists.html


CCing ...


On 20/10/16 09:55 +0200, Christophe Lyon wrote:

Hi,

Several times I have noticed/reported test failures because some test
cases wouldn't link on arm-none-eabi using the default 'old' cpu
target: __sync_synchronize cannot be resolved by the linker.


That isn't used by libstdc++ anywhere, so the call to it is being
emitted by the compiler. It would be good to know where it comes from.



The attached long patch adds
+// { dg-require-thread-fence "" }
to all the tests that require it according to the error messages I get.

The change is mechanical:
- insert this line below dg-do if present
- insert this line at the top of the file otherwise

For instance:

diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
index 633175b..a048250 100644
--- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
+++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
@@ -1,3 +1,4 @@
+// { dg-require-thread-fence "" }
// 2007-01-30  Paolo Carlini  

// Copyright (C) 2007-2016 Free Software Foundation, Inc.
diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
index e712655..f2a6c5a 100644
--- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
+++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
@@ -1,4 +1,5 @@
// { dg-do run }
+// { dg-require-thread-fence "" }
// Avoid use of non-overridable new/delete operators in shared
// { dg-options "-static" { target *-*-mingw* } }
// Test __cxa_vec routines


If that's OK, I'm not sure how to write the ChangeLog entry: it
modifies 3287 files.

In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.


Wouldn't it be better to remove the dependency on __sync_synchronize
rather than declare nearly 50% of the testsuite UNSUPPORTED?

If 3287 tests can't use it is the resulting libstdc++.so actually
useful to anyone?



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Jonathan Wakely

Please CC the libstdc++ list for libstdc++ patches, this is a
requirement for patch submission, see https://gcc.gnu.org/lists.html


On 20/10/16 09:55 +0200, Christophe Lyon wrote:

Hi,

Several times I have noticed/reported test failures because some test
cases wouldn't link on arm-none-eabi using the default 'old' cpu
target: __sync_synchronize cannot be resolved by the linker.


That isn't used by libstdc++ anywhere, so the call to it is being
emitted by the compiler. It would be good to know where it comes from.



The attached long patch adds
+// { dg-require-thread-fence "" }
to all the tests that require it according to the error messages I get.

The change is mechanical:
- insert this line below dg-do if present
- insert this line at the top of the file otherwise

For instance:

diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
index 633175b..a048250 100644
--- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
+++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
@@ -1,3 +1,4 @@
+// { dg-require-thread-fence "" }
// 2007-01-30  Paolo Carlini  

// Copyright (C) 2007-2016 Free Software Foundation, Inc.
diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
index e712655..f2a6c5a 100644
--- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
+++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
@@ -1,4 +1,5 @@
// { dg-do run }
+// { dg-require-thread-fence "" }
// Avoid use of non-overridable new/delete operators in shared
// { dg-options "-static" { target *-*-mingw* } }
// Test __cxa_vec routines


If that's OK, I'm not sure how to write the ChangeLog entry: it
modifies 3287 files.

In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.


Wouldn't it be better to remove the dependency on __sync_synchronize
rather than declare nearly 50% of the testsuite UNSUPPORTED?

If 3287 tests can't use it is the resulting libstdc++.so actually
useful to anyone?



Re: [libstdc++, testsuite] Add dg-require-thread-fence

2016-10-20 Thread Christophe Lyon
On 20 October 2016 at 09:55, Christophe Lyon  wrote:
> Hi,
>
> Several times I have noticed/reported test failures because some test
> cases wouldn't link on arm-none-eabi using the default 'old' cpu
> target: __sync_synchronize cannot be resolved by the linker.
>
> The attached long patch adds
> +// { dg-require-thread-fence "" }
> to all the tests that require it according to the error messages I get.
>
> The change is mechanical:
> - insert this line below dg-do if present
> - insert this line at the top of the file otherwise
>
> For instance:
>
> diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
> b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
> index 633175b..a048250 100644
> --- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
> +++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
> @@ -1,3 +1,4 @@
> +// { dg-require-thread-fence "" }
>  // 2007-01-30  Paolo Carlini  
>
>  // Copyright (C) 2007-2016 Free Software Foundation, Inc.
> diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
> b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
> index e712655..f2a6c5a 100644
> --- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
> +++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
> @@ -1,4 +1,5 @@
>  // { dg-do run }
> +// { dg-require-thread-fence "" }
>  // Avoid use of non-overridable new/delete operators in shared
>  // { dg-options "-static" { target *-*-mingw* } }
>  // Test __cxa_vec routines
>
>
> If that's OK, I'm not sure how to write the ChangeLog entry: it
> modifies 3287 files.
>
> In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.
>
>
> OK?
>

Jonathan,

The new test you introduced yesterday would need a similar fix:
experimental/memory/shared_ptr/cons/enable_shared_from_this.cc

Christophe

> Other question: I've noticed similar errors in the g++ validation, but
> I'm not sure what is the corresponding dg-require directive?
>
> Thanks,
>
> Christophe