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

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On 29 November 2016 at 21:18, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 16/11/16 22:18 +0100, Christophe Lyon wrote:
>>
>> On 15 November 2016 at 12:50, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>> On 14/11/16 14:32 +0100, Christophe Lyon wrote:
>>>>
>>>>
>>>> On 20 October 2016 at 19:40, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>>
>>>>>
>>>>> On 20/10/16 10:33 -0700, Mike Stump wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Oct 20, 2016, at 9:34 AM, Jonathan Wakely <jwakely@redhat.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 20/10/16 09:26 -0700, Mike Stump wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Oct 20, 2016, at 5:20 AM, Jonathan Wakely <jwakely@redhat.com>
>>>>>>>> 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