On 03-09-2014 11:01, Maciej W. Rozycki wrote:
> On Tue, 2 Sep 2014, Adhemerval Zanella wrote:
>
>> Ping.
>>
>> On 19-08-2014 13:54, Adhemerval Zanella wrote:
>>> Ping.
>>>
>>> On 06-08-2014 17:21, Adhemerval Zanella wrote:
>>>> On 01-08-2014 12:31, Joseph S. Myers wrote:
>>>>> On Thu, 31 Jul 2014, David Edelsohn wrote:
>>>>>
>>>>>> Thanks for implementing the FENV support. The patch generally looks
>>>>>> good to me.
>>>>>>
>>>>>> My one concern is a detail in the implementation of "update". I do not
>>>>>> have enough experience with GENERIC to verify the details and it seems
>>>>>> like it is missing building an outer COMPOUND_EXPR containing
>>>>>> update_mffs and the CALL_EXPR for update mtfsf.
>>>>> I suppose what's actually odd there is that you have
>>>>>
>>>>> + tree update_mffs = build2 (MODIFY_EXPR, void_type_node, old_fenv,
>>>>> call_mffs);
>>>>> +
>>>>> + tree old_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node,
>>>>> update_mffs);
>>>>>
>>>>> so you build a MODIFY_EXPR in void_type_node but then convert it with a
>>>>> VIEW_CONVERT_EXPR. If you'd built the MODIFY_EXPR in double_type_node
>>>>> then the VIEW_CONVERT_EXPR would be meaningful (the value of an
>>>>> assignment
>>>>> a = b being the new value of a), but reinterpreting a void value doesn't
>>>>> make sense. Or you could probably just use call_mffs directly in the
>>>>> VIEW_CONVERT_EXPR without explicitly creating the old_fenv variable.
>>>>>
>>>> Thanks for the review Josephm. I have changed to avoid the void
>>>> reinterpretation
>>>> and use call_mffs directly. I have also removed the the mask generation
>>>> in 'clear'
>>>> from your previous message, it is now reusing the mas used in
>>>> feholdexcept. The
>>>> testcase patch is the same as before.
>>>>
>>>> Checked on both linux-powerpc64/powerpc64le and no regressions found.
>>>>
>>>> --
>>>>
>>>> 2014-08-06 Adhemerval Zanella <azane...@linux.vnet.ibm.com>
>>>>
>>>> gcc:
>>>> * config/rs6000/rs6000.c (rs6000_atomic_assign_expand_fenv): New
>>>> function.
>>>>
>>>> gcc/testsuite:
>>>> * gcc.dg/atomic/c11-atomic-exec-5.c
>>>> (test_main_long_double_add_overflow): Define and run only for
>>>> LDBL_MANT_DIG != 106.
>>>> (test_main_complex_long_double_add_overflow): Likewise.
>>>> (test_main_long_double_sub_overflow): Likewise.
>>>> (test_main_complex_long_double_sub_overflow): Likewise.
> FWIW I pushed it through regression testing across my usual set of
> powerpc-linux-gnu multilibs with the results (for c11-atomic-exec-5.c) as
> follows:
>
> -mcpu=603e PASS
> -mcpu=603e -msoft-float UNSUPPORTED
> -mcpu=8540 -mfloat-gprs=single -mspe=yes -mabi=spe UNSUPPORTED
> -mcpu=8548 -mfloat-gprs=double -mspe=yes -mabi=spe UNSUPPORTED
> -mcpu=7400 -maltivec -mabi=altivec PASS
> -mcpu=e6500 -maltivec -mabi=altivec PASS
> -mcpu=e5500 -m64 PASS
> -mcpu=e6500 -m64 -maltivec -mabi=altivec PASS
Thanks for testing it, I'll to add these permutations on my own testbench.
>
> (floating-point environment is of course unsupported for soft-float
> targets and for the SPE FPU another change is required to implement
> floating-point environment handling to complement one proposed here).
> No regressions otherwise.
>
> While at it, may I propose another change on top of this?
>
> I've noticed the test case is rather slow, it certainly takes much more
> time than the average one, I've seen elapsed times of well over a minute
> on reasonably fast hardware and occasionally a timeout midway through even
> though the test case was otherwise progressing just fine. I think lock
> contention or unrelated system activity such as hardware interrupts (think
> a busy network!) may contribute to it for systems using LL/SC loops for
> atomicity.
>
> So I think the default timeout that's used for really quick tests should
> be extended a bit. I propose a factor of 2, just not to make it too
> excessive, at least for the beginning (maybe it'll have to be higher
> eventually).
Do you mind if I incorporate this change on my patchset?
>
> OK?
>
> 2014-09-03 Maciej W. Rozycki <ma...@codesourcery.com>
>
> gcc/testsuite/
> * gcc.dg/atomic/c11-atomic-exec-5.c (dg-timeout-factor): New
> setting.
>
> Maciej
>
> gcc-test-c11-atomic-exec-5-timeout-factor.diff
> Index: gcc-fsf-trunk-quilt/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> ===================================================================
> --- gcc-fsf-trunk-quilt.orig/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> 2014-09-02 17:34:06.718927043 +0100
> +++ gcc-fsf-trunk-quilt/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> 2014-09-03 14:51:12.958927233 +0100
> @@ -9,6 +9,7 @@
> /* { dg-additional-options "-D_XOPEN_SOURCE=600" { target
> *-*-solaris2.1[0-9]* } } */
> /* { dg-require-effective-target fenv_exceptions } */
> /* { dg-require-effective-target pthread } */
> +/* { dg-timeout-factor 2 } */
>
> #include <fenv.h>
> #include <float.h>
>
