* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
@ 2014-07-16 20:33 David Edelsohn
2014-08-01 3:28 ` David Edelsohn
0 siblings, 1 reply; 18+ messages in thread
From: David Edelsohn @ 2014-07-16 20:33 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GCC Patches
Adhemerval,
This looks very good. Thanks for helping with the FENV implementation.
I will discuss this with some GIMPLE experts during Cauldron. I want
to check that the GIMPLE is correct before approving this.
Thanks, David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-07-16 20:33 [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV David Edelsohn
@ 2014-08-01 3:28 ` David Edelsohn
2014-08-01 15:31 ` Joseph S. Myers
0 siblings, 1 reply; 18+ messages in thread
From: David Edelsohn @ 2014-08-01 3:28 UTC (permalink / raw)
To: Adhemerval Zanella, Richard Henderson, Myers, Joseph; +Cc: GCC Patches
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 would like someone to double check.
Thanks, David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-08-01 3:28 ` David Edelsohn
@ 2014-08-01 15:31 ` Joseph S. Myers
2014-08-06 20:21 ` Adhemerval Zanella
0 siblings, 1 reply; 18+ messages in thread
From: Joseph S. Myers @ 2014-08-01 15:31 UTC (permalink / raw)
To: David Edelsohn; +Cc: Adhemerval Zanella, Richard Henderson, GCC Patches
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.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-08-01 15:31 ` Joseph S. Myers
@ 2014-08-06 20:21 ` Adhemerval Zanella
2014-08-19 16:54 ` Adhemerval Zanella
0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2014-08-06 20:21 UTC (permalink / raw)
To: gcc-patches
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 <azanella@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.
---
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d088ff6..7d66eb1 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1631,6 +1631,9 @@ static const struct attribute_spec rs6000_attribute_table[] =
#undef TARGET_CAN_USE_DOLOOP_P
#define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
+
+#undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV
+#define TARGET_ATOMIC_ASSIGN_EXPAND_FENV rs6000_atomic_assign_expand_fenv
\f
/* Processor table. */
@@ -33337,6 +33340,80 @@ emit_fusion_gpr_load (rtx *operands)
return "";
}
+/* Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook. */
+
+static void
+rs6000_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
+{
+ if (!TARGET_HARD_FLOAT || !TARGET_FPRS)
+ return;
+
+ tree mffs = rs6000_builtin_decls[RS6000_BUILTIN_MFFS];
+ tree mtfsf = rs6000_builtin_decls[RS6000_BUILTIN_MTFSF];
+ tree call_mffs = build_call_expr (mffs, 0);
+
+ /* Generates the equivalent of feholdexcept (&fenv_var)
+
+ *fenv_var = __builtin_mffs ();
+ double fenv_hold;
+ *(uint64_t*)&fenv_hold = *(uint64_t*)fenv_var & 0xffffffff00000007LL;
+ __builtin_mtfsf (0xff, fenv_hold); */
+
+ /* Mask to clear everything except for the rounding modes and non-IEEE
+ arithmetic flag. */
+ const unsigned HOST_WIDE_INT hold_exception_mask =
+ HOST_WIDE_INT_UC (0xffffffff00000007);
+
+ tree fenv_var = create_tmp_var (double_type_node, NULL);
+
+ tree hold_mffs = build2 (MODIFY_EXPR, void_type_node, fenv_var, call_mffs);
+
+ tree fenv_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, fenv_var);
+ tree fenv_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
+ build_int_cst (uint64_type_node, hold_exception_mask));
+
+ tree fenv_mtfsf = build1 (VIEW_CONVERT_EXPR, double_type_node, fenv_llu_and);
+
+ tree hold_mtfsf = build_call_expr (mtfsf, 2,
+ build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
+
+ *hold = build2 (COMPOUND_EXPR, void_type_node, hold_mffs, hold_mtfsf);
+
+ /* Reload the value of fenv_hold to clear the exceptions. */
+
+ *clear = build_call_expr (mtfsf, 2,
+ build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
+
+ /* Generates the equivalent of feupdateenv (&fenv_var)
+
+ double old_fenv = __builtin_mffs ();
+ double fenv_update;
+ *(uint64_t*)&fenv_update = (*(uint64_t*)&old & 0xffffffff1fffff00LL) |
+ (*(uint64_t*)fenv_var 0x1ff80fff);
+ __builtin_mtfsf (0xff, fenv_update); */
+
+ const unsigned HOST_WIDE_INT update_exception_mask =
+ HOST_WIDE_INT_UC (0xffffffff1fffff00);
+ const unsigned HOST_WIDE_INT new_exception_mask =
+ HOST_WIDE_INT_UC (0x1ff80fff);
+
+ tree old_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, call_mffs);
+ tree old_llu_and = build2 (BIT_AND_EXPR, uint64_type_node,
+ old_llu, build_int_cst (uint64_type_node, update_exception_mask));
+
+ tree new_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
+ build_int_cst (uint64_type_node, new_exception_mask));
+
+ tree new_llu_mask = build2 (BIT_IOR_EXPR, uint64_type_node,
+ old_llu_and, new_llu_and);
+
+ tree fenv_mtfsf_update = build1 (VIEW_CONVERT_EXPR, double_type_node,
+ new_llu_mask);
+
+ *update = build_call_expr (mtfsf, 2,
+ build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf_update);
+}
+
\f
struct gcc_target targetm = TARGET_INITIALIZER;
diff --git a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
index b47a228..6bcb302 100644
--- a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
+++ b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
@@ -331,11 +331,11 @@ TEST_FUNCS (complex_double_div_overflow, _Complex double, , /= DBL_MIN, 0,
TEST_FUNCS (long_double_add_invalid, long double, , += __builtin_infl (), 0,
0, __builtin_isinf, 0,
-__builtin_infl (), FE_INVALID)
+#if LDBL_MANT_DIG != 106
TEST_FUNCS (long_double_add_overflow, long double, , += LDBL_MAX, 0,
LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
0, 0)
#define NOT_LDBL_EPSILON_2(X) ((X) != LDBL_EPSILON / 2)
-#if LDBL_MANT_DIG != 106
TEST_FUNCS (long_double_add_inexact, long double, , += LDBL_EPSILON / 2, 0,
1.0L, NOT_LDBL_EPSILON_2, FE_INEXACT,
0, 0)
@@ -348,18 +348,18 @@ TEST_FUNCS (long_double_preinc_inexact, long double, ++, , 0,
TEST_FUNCS (long_double_postinc_inexact, long double, , ++, 0,
LDBL_EPSILON / 2, NOT_MINUS_1, FE_INEXACT,
-1, 0)
-#endif
TEST_FUNCS (complex_long_double_add_overflow, _Complex long double, , += LDBL_MAX, 0,
LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
0, 0)
+#endif
TEST_FUNCS (long_double_sub_invalid, long double, , -= __builtin_infl (), 0,
0, __builtin_isinf, 0,
__builtin_infl (), FE_INVALID)
+#if LDBL_MANT_DIG != 106
TEST_FUNCS (long_double_sub_overflow, long double, , -= LDBL_MAX, 0,
-LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
0, 0)
#define NOT_MINUS_LDBL_EPSILON_2(X) ((X) != -LDBL_EPSILON / 2)
-#if LDBL_MANT_DIG != 106
TEST_FUNCS (long_double_sub_inexact, long double, , -= LDBL_EPSILON / 2, 0,
-1.0L, NOT_MINUS_LDBL_EPSILON_2, FE_INEXACT,
0, 0)
@@ -372,10 +372,10 @@ TEST_FUNCS (long_double_predec_inexact, long double, --, , 0,
TEST_FUNCS (long_double_postdec_inexact, long double, , --, 0,
-LDBL_EPSILON / 2, NOT_1, FE_INEXACT,
1, 0)
-#endif
TEST_FUNCS (complex_long_double_sub_overflow, _Complex long double, , -= LDBL_MAX, 0,
-LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
0, 0)
+#endif
TEST_FUNCS (long_double_mul_invalid, long double, , *= __builtin_infl (), 0,
__builtin_infl (), __builtin_isinf, 0,
0, FE_INVALID)
@@ -507,23 +507,23 @@ main (void)
ret |= test_main_int_div_double_inexact ();
ret |= test_main_complex_double_div_overflow ();
ret |= test_main_long_double_add_invalid ();
- ret |= test_main_long_double_add_overflow ();
#if LDBL_MANT_DIG != 106
+ ret |= test_main_long_double_add_overflow ();
ret |= test_main_long_double_add_inexact ();
ret |= test_main_long_double_add_inexact_int ();
ret |= test_main_long_double_preinc_inexact ();
ret |= test_main_long_double_postinc_inexact ();
-#endif
ret |= test_main_complex_long_double_add_overflow ();
+#endif
ret |= test_main_long_double_sub_invalid ();
- ret |= test_main_long_double_sub_overflow ();
#if LDBL_MANT_DIG != 106
+ ret |= test_main_long_double_sub_overflow ();
ret |= test_main_long_double_sub_inexact ();
ret |= test_main_long_double_sub_inexact_int ();
ret |= test_main_long_double_predec_inexact ();
ret |= test_main_long_double_postdec_inexact ();
-#endif
ret |= test_main_complex_long_double_sub_overflow ();
+#endif
ret |= test_main_long_double_mul_invalid ();
ret |= test_main_long_double_mul_overflow ();
ret |= test_main_long_double_mul_overflow_float ();
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-08-06 20:21 ` Adhemerval Zanella
@ 2014-08-19 16:54 ` Adhemerval Zanella
2014-09-02 22:23 ` Adhemerval Zanella
0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2014-08-19 16:54 UTC (permalink / raw)
To: gcc-patches
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 <azanella@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.
>
> ---
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index d088ff6..7d66eb1 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1631,6 +1631,9 @@ static const struct attribute_spec rs6000_attribute_table[] =
>
> #undef TARGET_CAN_USE_DOLOOP_P
> #define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
> +
> +#undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV
> +#define TARGET_ATOMIC_ASSIGN_EXPAND_FENV rs6000_atomic_assign_expand_fenv
> \f
>
> /* Processor table. */
> @@ -33337,6 +33340,80 @@ emit_fusion_gpr_load (rtx *operands)
> return "";
> }
>
> +/* Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook. */
> +
> +static void
> +rs6000_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
> +{
> + if (!TARGET_HARD_FLOAT || !TARGET_FPRS)
> + return;
> +
> + tree mffs = rs6000_builtin_decls[RS6000_BUILTIN_MFFS];
> + tree mtfsf = rs6000_builtin_decls[RS6000_BUILTIN_MTFSF];
> + tree call_mffs = build_call_expr (mffs, 0);
> +
> + /* Generates the equivalent of feholdexcept (&fenv_var)
> +
> + *fenv_var = __builtin_mffs ();
> + double fenv_hold;
> + *(uint64_t*)&fenv_hold = *(uint64_t*)fenv_var & 0xffffffff00000007LL;
> + __builtin_mtfsf (0xff, fenv_hold); */
> +
> + /* Mask to clear everything except for the rounding modes and non-IEEE
> + arithmetic flag. */
> + const unsigned HOST_WIDE_INT hold_exception_mask =
> + HOST_WIDE_INT_UC (0xffffffff00000007);
> +
> + tree fenv_var = create_tmp_var (double_type_node, NULL);
> +
> + tree hold_mffs = build2 (MODIFY_EXPR, void_type_node, fenv_var, call_mffs);
> +
> + tree fenv_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, fenv_var);
> + tree fenv_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
> + build_int_cst (uint64_type_node, hold_exception_mask));
> +
> + tree fenv_mtfsf = build1 (VIEW_CONVERT_EXPR, double_type_node, fenv_llu_and);
> +
> + tree hold_mtfsf = build_call_expr (mtfsf, 2,
> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
> +
> + *hold = build2 (COMPOUND_EXPR, void_type_node, hold_mffs, hold_mtfsf);
> +
> + /* Reload the value of fenv_hold to clear the exceptions. */
> +
> + *clear = build_call_expr (mtfsf, 2,
> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
> +
> + /* Generates the equivalent of feupdateenv (&fenv_var)
> +
> + double old_fenv = __builtin_mffs ();
> + double fenv_update;
> + *(uint64_t*)&fenv_update = (*(uint64_t*)&old & 0xffffffff1fffff00LL) |
> + (*(uint64_t*)fenv_var 0x1ff80fff);
> + __builtin_mtfsf (0xff, fenv_update); */
> +
> + const unsigned HOST_WIDE_INT update_exception_mask =
> + HOST_WIDE_INT_UC (0xffffffff1fffff00);
> + const unsigned HOST_WIDE_INT new_exception_mask =
> + HOST_WIDE_INT_UC (0x1ff80fff);
> +
> + tree old_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, call_mffs);
> + tree old_llu_and = build2 (BIT_AND_EXPR, uint64_type_node,
> + old_llu, build_int_cst (uint64_type_node, update_exception_mask));
> +
> + tree new_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
> + build_int_cst (uint64_type_node, new_exception_mask));
> +
> + tree new_llu_mask = build2 (BIT_IOR_EXPR, uint64_type_node,
> + old_llu_and, new_llu_and);
> +
> + tree fenv_mtfsf_update = build1 (VIEW_CONVERT_EXPR, double_type_node,
> + new_llu_mask);
> +
> + *update = build_call_expr (mtfsf, 2,
> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf_update);
> +}
> +
> \f
> struct gcc_target targetm = TARGET_INITIALIZER;
>
> diff --git a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> index b47a228..6bcb302 100644
> --- a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> +++ b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
> @@ -331,11 +331,11 @@ TEST_FUNCS (complex_double_div_overflow, _Complex double, , /= DBL_MIN, 0,
> TEST_FUNCS (long_double_add_invalid, long double, , += __builtin_infl (), 0,
> 0, __builtin_isinf, 0,
> -__builtin_infl (), FE_INVALID)
> +#if LDBL_MANT_DIG != 106
> TEST_FUNCS (long_double_add_overflow, long double, , += LDBL_MAX, 0,
> LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
> 0, 0)
> #define NOT_LDBL_EPSILON_2(X) ((X) != LDBL_EPSILON / 2)
> -#if LDBL_MANT_DIG != 106
> TEST_FUNCS (long_double_add_inexact, long double, , += LDBL_EPSILON / 2, 0,
> 1.0L, NOT_LDBL_EPSILON_2, FE_INEXACT,
> 0, 0)
> @@ -348,18 +348,18 @@ TEST_FUNCS (long_double_preinc_inexact, long double, ++, , 0,
> TEST_FUNCS (long_double_postinc_inexact, long double, , ++, 0,
> LDBL_EPSILON / 2, NOT_MINUS_1, FE_INEXACT,
> -1, 0)
> -#endif
> TEST_FUNCS (complex_long_double_add_overflow, _Complex long double, , += LDBL_MAX, 0,
> LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
> 0, 0)
> +#endif
> TEST_FUNCS (long_double_sub_invalid, long double, , -= __builtin_infl (), 0,
> 0, __builtin_isinf, 0,
> __builtin_infl (), FE_INVALID)
> +#if LDBL_MANT_DIG != 106
> TEST_FUNCS (long_double_sub_overflow, long double, , -= LDBL_MAX, 0,
> -LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
> 0, 0)
> #define NOT_MINUS_LDBL_EPSILON_2(X) ((X) != -LDBL_EPSILON / 2)
> -#if LDBL_MANT_DIG != 106
> TEST_FUNCS (long_double_sub_inexact, long double, , -= LDBL_EPSILON / 2, 0,
> -1.0L, NOT_MINUS_LDBL_EPSILON_2, FE_INEXACT,
> 0, 0)
> @@ -372,10 +372,10 @@ TEST_FUNCS (long_double_predec_inexact, long double, --, , 0,
> TEST_FUNCS (long_double_postdec_inexact, long double, , --, 0,
> -LDBL_EPSILON / 2, NOT_1, FE_INEXACT,
> 1, 0)
> -#endif
> TEST_FUNCS (complex_long_double_sub_overflow, _Complex long double, , -= LDBL_MAX, 0,
> -LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
> 0, 0)
> +#endif
> TEST_FUNCS (long_double_mul_invalid, long double, , *= __builtin_infl (), 0,
> __builtin_infl (), __builtin_isinf, 0,
> 0, FE_INVALID)
> @@ -507,23 +507,23 @@ main (void)
> ret |= test_main_int_div_double_inexact ();
> ret |= test_main_complex_double_div_overflow ();
> ret |= test_main_long_double_add_invalid ();
> - ret |= test_main_long_double_add_overflow ();
> #if LDBL_MANT_DIG != 106
> + ret |= test_main_long_double_add_overflow ();
> ret |= test_main_long_double_add_inexact ();
> ret |= test_main_long_double_add_inexact_int ();
> ret |= test_main_long_double_preinc_inexact ();
> ret |= test_main_long_double_postinc_inexact ();
> -#endif
> ret |= test_main_complex_long_double_add_overflow ();
> +#endif
> ret |= test_main_long_double_sub_invalid ();
> - ret |= test_main_long_double_sub_overflow ();
> #if LDBL_MANT_DIG != 106
> + ret |= test_main_long_double_sub_overflow ();
> ret |= test_main_long_double_sub_inexact ();
> ret |= test_main_long_double_sub_inexact_int ();
> ret |= test_main_long_double_predec_inexact ();
> ret |= test_main_long_double_postdec_inexact ();
> -#endif
> ret |= test_main_complex_long_double_sub_overflow ();
> +#endif
> ret |= test_main_long_double_mul_invalid ();
> ret |= test_main_long_double_mul_overflow ();
> ret |= test_main_long_double_mul_overflow_float ();
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-08-19 16:54 ` Adhemerval Zanella
@ 2014-09-02 22:23 ` Adhemerval Zanella
2014-09-03 14:01 ` Maciej W. Rozycki
0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2014-09-02 22:23 UTC (permalink / raw)
To: gcc-patches
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 <azanella@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.
>>
>> ---
>>
>> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> index d088ff6..7d66eb1 100644
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -1631,6 +1631,9 @@ static const struct attribute_spec rs6000_attribute_table[] =
>>
>> #undef TARGET_CAN_USE_DOLOOP_P
>> #define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost
>> +
>> +#undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV
>> +#define TARGET_ATOMIC_ASSIGN_EXPAND_FENV rs6000_atomic_assign_expand_fenv
>> \f
>>
>> /* Processor table. */
>> @@ -33337,6 +33340,80 @@ emit_fusion_gpr_load (rtx *operands)
>> return "";
>> }
>>
>> +/* Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV hook. */
>> +
>> +static void
>> +rs6000_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
>> +{
>> + if (!TARGET_HARD_FLOAT || !TARGET_FPRS)
>> + return;
>> +
>> + tree mffs = rs6000_builtin_decls[RS6000_BUILTIN_MFFS];
>> + tree mtfsf = rs6000_builtin_decls[RS6000_BUILTIN_MTFSF];
>> + tree call_mffs = build_call_expr (mffs, 0);
>> +
>> + /* Generates the equivalent of feholdexcept (&fenv_var)
>> +
>> + *fenv_var = __builtin_mffs ();
>> + double fenv_hold;
>> + *(uint64_t*)&fenv_hold = *(uint64_t*)fenv_var & 0xffffffff00000007LL;
>> + __builtin_mtfsf (0xff, fenv_hold); */
>> +
>> + /* Mask to clear everything except for the rounding modes and non-IEEE
>> + arithmetic flag. */
>> + const unsigned HOST_WIDE_INT hold_exception_mask =
>> + HOST_WIDE_INT_UC (0xffffffff00000007);
>> +
>> + tree fenv_var = create_tmp_var (double_type_node, NULL);
>> +
>> + tree hold_mffs = build2 (MODIFY_EXPR, void_type_node, fenv_var, call_mffs);
>> +
>> + tree fenv_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, fenv_var);
>> + tree fenv_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
>> + build_int_cst (uint64_type_node, hold_exception_mask));
>> +
>> + tree fenv_mtfsf = build1 (VIEW_CONVERT_EXPR, double_type_node, fenv_llu_and);
>> +
>> + tree hold_mtfsf = build_call_expr (mtfsf, 2,
>> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
>> +
>> + *hold = build2 (COMPOUND_EXPR, void_type_node, hold_mffs, hold_mtfsf);
>> +
>> + /* Reload the value of fenv_hold to clear the exceptions. */
>> +
>> + *clear = build_call_expr (mtfsf, 2,
>> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf);
>> +
>> + /* Generates the equivalent of feupdateenv (&fenv_var)
>> +
>> + double old_fenv = __builtin_mffs ();
>> + double fenv_update;
>> + *(uint64_t*)&fenv_update = (*(uint64_t*)&old & 0xffffffff1fffff00LL) |
>> + (*(uint64_t*)fenv_var 0x1ff80fff);
>> + __builtin_mtfsf (0xff, fenv_update); */
>> +
>> + const unsigned HOST_WIDE_INT update_exception_mask =
>> + HOST_WIDE_INT_UC (0xffffffff1fffff00);
>> + const unsigned HOST_WIDE_INT new_exception_mask =
>> + HOST_WIDE_INT_UC (0x1ff80fff);
>> +
>> + tree old_llu = build1 (VIEW_CONVERT_EXPR, uint64_type_node, call_mffs);
>> + tree old_llu_and = build2 (BIT_AND_EXPR, uint64_type_node,
>> + old_llu, build_int_cst (uint64_type_node, update_exception_mask));
>> +
>> + tree new_llu_and = build2 (BIT_AND_EXPR, uint64_type_node, fenv_llu,
>> + build_int_cst (uint64_type_node, new_exception_mask));
>> +
>> + tree new_llu_mask = build2 (BIT_IOR_EXPR, uint64_type_node,
>> + old_llu_and, new_llu_and);
>> +
>> + tree fenv_mtfsf_update = build1 (VIEW_CONVERT_EXPR, double_type_node,
>> + new_llu_mask);
>> +
>> + *update = build_call_expr (mtfsf, 2,
>> + build_int_cst (unsigned_type_node, 0xff), fenv_mtfsf_update);
>> +}
>> +
>> \f
>> struct gcc_target targetm = TARGET_INITIALIZER;
>>
>> diff --git a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
>> index b47a228..6bcb302 100644
>> --- a/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
>> +++ b/gcc/testsuite/gcc.dg/atomic/c11-atomic-exec-5.c
>> @@ -331,11 +331,11 @@ TEST_FUNCS (complex_double_div_overflow, _Complex double, , /= DBL_MIN, 0,
>> TEST_FUNCS (long_double_add_invalid, long double, , += __builtin_infl (), 0,
>> 0, __builtin_isinf, 0,
>> -__builtin_infl (), FE_INVALID)
>> +#if LDBL_MANT_DIG != 106
>> TEST_FUNCS (long_double_add_overflow, long double, , += LDBL_MAX, 0,
>> LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
>> 0, 0)
>> #define NOT_LDBL_EPSILON_2(X) ((X) != LDBL_EPSILON / 2)
>> -#if LDBL_MANT_DIG != 106
>> TEST_FUNCS (long_double_add_inexact, long double, , += LDBL_EPSILON / 2, 0,
>> 1.0L, NOT_LDBL_EPSILON_2, FE_INEXACT,
>> 0, 0)
>> @@ -348,18 +348,18 @@ TEST_FUNCS (long_double_preinc_inexact, long double, ++, , 0,
>> TEST_FUNCS (long_double_postinc_inexact, long double, , ++, 0,
>> LDBL_EPSILON / 2, NOT_MINUS_1, FE_INEXACT,
>> -1, 0)
>> -#endif
>> TEST_FUNCS (complex_long_double_add_overflow, _Complex long double, , += LDBL_MAX, 0,
>> LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
>> 0, 0)
>> +#endif
>> TEST_FUNCS (long_double_sub_invalid, long double, , -= __builtin_infl (), 0,
>> 0, __builtin_isinf, 0,
>> __builtin_infl (), FE_INVALID)
>> +#if LDBL_MANT_DIG != 106
>> TEST_FUNCS (long_double_sub_overflow, long double, , -= LDBL_MAX, 0,
>> -LDBL_MAX, __builtin_isinf, FE_OVERFLOW | FE_INEXACT,
>> 0, 0)
>> #define NOT_MINUS_LDBL_EPSILON_2(X) ((X) != -LDBL_EPSILON / 2)
>> -#if LDBL_MANT_DIG != 106
>> TEST_FUNCS (long_double_sub_inexact, long double, , -= LDBL_EPSILON / 2, 0,
>> -1.0L, NOT_MINUS_LDBL_EPSILON_2, FE_INEXACT,
>> 0, 0)
>> @@ -372,10 +372,10 @@ TEST_FUNCS (long_double_predec_inexact, long double, --, , 0,
>> TEST_FUNCS (long_double_postdec_inexact, long double, , --, 0,
>> -LDBL_EPSILON / 2, NOT_1, FE_INEXACT,
>> 1, 0)
>> -#endif
>> TEST_FUNCS (complex_long_double_sub_overflow, _Complex long double, , -= LDBL_MAX, 0,
>> -LDBL_MAX, REAL_ISINF, FE_OVERFLOW | FE_INEXACT,
>> 0, 0)
>> +#endif
>> TEST_FUNCS (long_double_mul_invalid, long double, , *= __builtin_infl (), 0,
>> __builtin_infl (), __builtin_isinf, 0,
>> 0, FE_INVALID)
>> @@ -507,23 +507,23 @@ main (void)
>> ret |= test_main_int_div_double_inexact ();
>> ret |= test_main_complex_double_div_overflow ();
>> ret |= test_main_long_double_add_invalid ();
>> - ret |= test_main_long_double_add_overflow ();
>> #if LDBL_MANT_DIG != 106
>> + ret |= test_main_long_double_add_overflow ();
>> ret |= test_main_long_double_add_inexact ();
>> ret |= test_main_long_double_add_inexact_int ();
>> ret |= test_main_long_double_preinc_inexact ();
>> ret |= test_main_long_double_postinc_inexact ();
>> -#endif
>> ret |= test_main_complex_long_double_add_overflow ();
>> +#endif
>> ret |= test_main_long_double_sub_invalid ();
>> - ret |= test_main_long_double_sub_overflow ();
>> #if LDBL_MANT_DIG != 106
>> + ret |= test_main_long_double_sub_overflow ();
>> ret |= test_main_long_double_sub_inexact ();
>> ret |= test_main_long_double_sub_inexact_int ();
>> ret |= test_main_long_double_predec_inexact ();
>> ret |= test_main_long_double_postdec_inexact ();
>> -#endif
>> ret |= test_main_complex_long_double_sub_overflow ();
>> +#endif
>> ret |= test_main_long_double_mul_invalid ();
>> ret |= test_main_long_double_mul_overflow ();
>> ret |= test_main_long_double_mul_overflow_float ();
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-09-02 22:23 ` Adhemerval Zanella
@ 2014-09-03 14:01 ` Maciej W. Rozycki
2014-09-03 15:49 ` Joseph S. Myers
2014-09-04 18:40 ` Adhemerval Zanella
0 siblings, 2 replies; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-09-03 14:01 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: gcc-patches
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 <azanella@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 <macro@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>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-09-03 14:01 ` Maciej W. Rozycki
@ 2014-09-03 15:49 ` Joseph S. Myers
2014-09-04 18:40 ` Adhemerval Zanella
1 sibling, 0 replies; 18+ messages in thread
From: Joseph S. Myers @ 2014-09-03 15:49 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Adhemerval Zanella, gcc-patches
On Wed, 3 Sep 2014, Maciej W. Rozycki wrote:
> (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).
Support for SPE will depend on the C library just as soft-float support
will, because of the need to have trapping on exceptions other than
"inexact" enabled in the processor at all times with the kernel then using
the prctl settings to determine whether that trap is for emulation or to
produce SIGFPE.
(The relevant support is in glibc 2.19 for soft-float and e500, in the
form of __atomic_feholdexcept, __atomic_feclearexcept and
__atomic_feupdateenv functions. I intend to implement the GCC side -
conditional on being configured for glibc 2.19 or later on the target, as
specified with --with-glibc-version or detected by configure's examination
of target headers - once the hard-float support is in GCC. I believe the
support in question will be identical for soft-float and e500, since it
will be calling libc functions instead of manipulating processor state.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-09-03 14:01 ` Maciej W. Rozycki
2014-09-03 15:49 ` Joseph S. Myers
@ 2014-09-04 18:40 ` Adhemerval Zanella
2014-09-15 14:38 ` Maciej W. Rozycki
1 sibling, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2014-09-04 18:40 UTC (permalink / raw)
To: gcc-patches
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 <azanella@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 <macro@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>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV
2014-09-04 18:40 ` Adhemerval Zanella
@ 2014-09-15 14:38 ` Maciej W. Rozycki
2014-10-20 17:18 ` [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c Maciej W. Rozycki
0 siblings, 1 reply; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-09-15 14:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: gcc-patches
On Thu, 4 Sep 2014, Adhemerval Zanella wrote:
> > 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?
I missed your e-mail previously, sorry. Surely I don't! Thanks.
Maciej
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-09-15 14:38 ` Maciej W. Rozycki
@ 2014-10-20 17:18 ` Maciej W. Rozycki
2014-10-21 0:26 ` David Edelsohn
2014-11-14 21:02 ` [PING^2][PATCH] " Maciej W. Rozycki
0 siblings, 2 replies; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-10-20 17:18 UTC (permalink / raw)
To: gcc-patches; +Cc: Adhemerval Zanella, David Edelsohn
Hi,
I thought http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html would
be folded into PowerPC TARGET_ATOMIC_ASSIGN_EXPAND_FENV support, but I see
r216437 went without it. In that case would someone please review my
proposal as a separate change?
Thanks,
Maciej
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-10-20 17:18 ` [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c Maciej W. Rozycki
@ 2014-10-21 0:26 ` David Edelsohn
2014-10-21 1:49 ` Joseph S. Myers
2014-11-14 21:02 ` [PING^2][PATCH] " Maciej W. Rozycki
1 sibling, 1 reply; 18+ messages in thread
From: David Edelsohn @ 2014-10-21 0:26 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: GCC Patches, Adhemerval Zanella
On Mon, Oct 20, 2014 at 12:59 PM, Maciej W. Rozycki
<macro@codesourcery.com> wrote:
> Hi,
>
> I thought http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html would
> be folded into PowerPC TARGET_ATOMIC_ASSIGN_EXPAND_FENV support, but I see
> r216437 went without it. In that case would someone please review my
> proposal as a separate change?
The patch seems like a kludge work-around. Joseph suggested that full
support will require a newer GLIBC and detection in GCC.
Thanks, David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-10-21 0:26 ` David Edelsohn
@ 2014-10-21 1:49 ` Joseph S. Myers
2014-10-21 2:15 ` David Edelsohn
0 siblings, 1 reply; 18+ messages in thread
From: Joseph S. Myers @ 2014-10-21 1:49 UTC (permalink / raw)
To: David Edelsohn; +Cc: Maciej W. Rozycki, GCC Patches, Adhemerval Zanella
On Mon, 20 Oct 2014, David Edelsohn wrote:
> On Mon, Oct 20, 2014 at 12:59 PM, Maciej W. Rozycki
> <macro@codesourcery.com> wrote:
> > Hi,
> >
> > I thought http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html would
> > be folded into PowerPC TARGET_ATOMIC_ASSIGN_EXPAND_FENV support, but I see
> > r216437 went without it. In that case would someone please review my
> > proposal as a separate change?
>
> The patch seems like a kludge work-around. Joseph suggested that full
> support will require a newer GLIBC and detection in GCC.
No, it's support for soft-float and e500 in
TARGET_ATOMIC_ASSIGN_EXPAND_FENV that will need that (along with libgcc
changes to make libgcc's copies of the soft-fp functions into compat
symbols when they are available in glibc). That's nothing to do with the
timeout issue.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-10-21 1:49 ` Joseph S. Myers
@ 2014-10-21 2:15 ` David Edelsohn
2014-10-21 23:03 ` Maciej W. Rozycki
0 siblings, 1 reply; 18+ messages in thread
From: David Edelsohn @ 2014-10-21 2:15 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: Maciej W. Rozycki, GCC Patches, Adhemerval Zanella
On Mon, Oct 20, 2014 at 8:26 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Mon, 20 Oct 2014, David Edelsohn wrote:
>
>> On Mon, Oct 20, 2014 at 12:59 PM, Maciej W. Rozycki
>> <macro@codesourcery.com> wrote:
>> > Hi,
>> >
>> > I thought http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html would
>> > be folded into PowerPC TARGET_ATOMIC_ASSIGN_EXPAND_FENV support, but I see
>> > r216437 went without it. In that case would someone please review my
>> > proposal as a separate change?
>>
>> The patch seems like a kludge work-around. Joseph suggested that full
>> support will require a newer GLIBC and detection in GCC.
>
> No, it's support for soft-float and e500 in
> TARGET_ATOMIC_ASSIGN_EXPAND_FENV that will need that (along with libgcc
> changes to make libgcc's copies of the soft-fp functions into compat
> symbols when they are available in glibc). That's nothing to do with the
> timeout issue.
I can apply the patch, but I don't want to unilaterally decide to
change the timeout affecting all architectures.
Thanks, David
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-10-21 2:15 ` David Edelsohn
@ 2014-10-21 23:03 ` Maciej W. Rozycki
0 siblings, 0 replies; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-10-21 23:03 UTC (permalink / raw)
To: David Edelsohn; +Cc: Joseph S. Myers, GCC Patches, Adhemerval Zanella
David,
> >> > I thought http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html would
> >> > be folded into PowerPC TARGET_ATOMIC_ASSIGN_EXPAND_FENV support, but I see
> >> > r216437 went without it. In that case would someone please review my
> >> > proposal as a separate change?
> >>
> >> The patch seems like a kludge work-around. Joseph suggested that full
> >> support will require a newer GLIBC and detection in GCC.
> >
> > No, it's support for soft-float and e500 in
> > TARGET_ATOMIC_ASSIGN_EXPAND_FENV that will need that (along with libgcc
> > changes to make libgcc's copies of the soft-fp functions into compat
> > symbols when they are available in glibc). That's nothing to do with the
> > timeout issue.
>
> I can apply the patch, but I don't want to unilaterally decide to
> change the timeout affecting all architectures.
Understood, I only cc-ed you to keep you in the loop with changes somehow
related to Power targets and stuff you have been involved with rather than
seeking your approval.
Maciej
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PING^2][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-10-20 17:18 ` [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c Maciej W. Rozycki
2014-10-21 0:26 ` David Edelsohn
@ 2014-11-14 21:02 ` Maciej W. Rozycki
2014-11-17 10:06 ` Mike Stump
1 sibling, 1 reply; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-11-14 21:02 UTC (permalink / raw)
To: gcc-patches
Hi,
This patch:
http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html
is still waiting, please review.
Thanks,
Maciej
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-11-14 21:02 ` [PING^2][PATCH] " Maciej W. Rozycki
@ 2014-11-17 10:06 ` Mike Stump
2014-11-18 16:48 ` Maciej W. Rozycki
0 siblings, 1 reply; 18+ messages in thread
From: Mike Stump @ 2014-11-17 10:06 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: gcc-patches
On Nov 14, 2014, at 1:00 PM, Maciej W. Rozycki <macro@codesourcery.com> wrote:
> http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html
>
> is still waiting, please review.
Wait no more.
Ok.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PING^2][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c
2014-11-17 10:06 ` Mike Stump
@ 2014-11-18 16:48 ` Maciej W. Rozycki
0 siblings, 0 replies; 18+ messages in thread
From: Maciej W. Rozycki @ 2014-11-18 16:48 UTC (permalink / raw)
To: Mike Stump; +Cc: gcc-patches
On Mon, 17 Nov 2014, Mike Stump wrote:
> > http://gcc.gnu.org/ml/gcc-patches/2014-09/msg00242.html
> >
> > is still waiting, please review.
>
> Wait no more.
>
> Ok.
Applied now, thanks for your review.
Maciej
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-11-18 16:34 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-16 20:33 [PATCH] PowerPC: Implement TARGET_ATOMIC_ASSIGN_EXPAND_FENV David Edelsohn
2014-08-01 3:28 ` David Edelsohn
2014-08-01 15:31 ` Joseph S. Myers
2014-08-06 20:21 ` Adhemerval Zanella
2014-08-19 16:54 ` Adhemerval Zanella
2014-09-02 22:23 ` Adhemerval Zanella
2014-09-03 14:01 ` Maciej W. Rozycki
2014-09-03 15:49 ` Joseph S. Myers
2014-09-04 18:40 ` Adhemerval Zanella
2014-09-15 14:38 ` Maciej W. Rozycki
2014-10-20 17:18 ` [PING][PATCH] GCC/test: Set timeout factor for c11-atomic-exec-5.c Maciej W. Rozycki
2014-10-21 0:26 ` David Edelsohn
2014-10-21 1:49 ` Joseph S. Myers
2014-10-21 2:15 ` David Edelsohn
2014-10-21 23:03 ` Maciej W. Rozycki
2014-11-14 21:02 ` [PING^2][PATCH] " Maciej W. Rozycki
2014-11-17 10:06 ` Mike Stump
2014-11-18 16:48 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).