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
(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).
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>
