On Tue, Jul 21, 2015 at 8:42 PM, Sebastian Pop <[email protected]> wrote:
> Tom de Vries wrote:
>> Fix reduction safety checks
>>
>> * graphite-sese-to-poly.c (is_reduction_operation_p): Limit
>> flag_associative_math to SCALAR_FLOAT_TYPE_P. Honour
>> TYPE_OVERFLOW_TRAPS and TYPE_OVERFLOW_WRAPS for INTEGRAL_TYPE_P.
>> Only allow wrapping fixed-point otherwise.
>> (build_poly_scop): Always call
>> rewrite_commutative_reductions_out_of_ssa.
>
> The changes to graphite look good to me.
+ if (SCALAR_FLOAT_TYPE_P (type))
+ return flag_associative_math;
+
why only scalar floats? Please use FLOAT_TYPE_P.
+ if (INTEGRAL_TYPE_P (type))
+ return (!TYPE_OVERFLOW_TRAPS (type)
+ && TYPE_OVERFLOW_WRAPS (type));
it cannot both wrap and trap thus TYPE_OVERFLOW_WRAPS is enough.
I'm sure you'll disable quite some parallelization this way... (the
routine is modeled after
the vectorizers IIRC, so it would be affected as well). Yeah - I see
you modify autopar
testcases. Please instead XFAIL the existing ones and add variants
with unsigned
reductions. Adding -fwrapv isn't a good solution either.
Can you think of a testcase that breaks btw?
The "proper" solution (see other passes) is to rewrite the reduction
to a wrapping
one (cast to unsigned for the reduction op).
+ return (FIXED_POINT_TYPE_P (type)
+ && FIXED_POINT_TYPE_OVERFLOW_WRAPS_P (type));
why? Simply return false here instead?
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 9145dbf..e014be2 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -2613,16 +2613,30 @@ vect_is_simple_reduction_1 (loop_vec_info
loop_info, gimple phi,
"reduction: unsafe fp math optimization: ");
return NULL;
}
- else if (INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_TRAPS (type)
- && check_reduction)
+ else if (INTEGRAL_TYPE_P (type) && check_reduction)
{
...
You didn't need to adjust any testcases? That's probably because the
checking above is
not always executed (see PR66623 for a related testcase). The code
needs refactoring.
And we need a way-out, that is, we do _not_ want to not vectorize
signed reductions.
So you need to fix code generation instead.
+/* Nonzero if fixed-point type TYPE wraps at overflow.
+
+ GCC support of fixed-point types as specified by the draft technical report
+ (N1169 draft of ISO/IEC DTR 18037) is incomplete: Pragmas to
control overflow
+ and rounding behaviors are not implemented.
+
+ So, if not saturating, we assume modular wrap-around (see Annex E.4 Modwrap
+ overflow). */
+
+#define FIXED_POINT_TYPE_OVERFLOW_WRAPS_P(TYPE) \
+ (NON_SAT_FIXED_POINT_TYPE_P (TYPE))
somebody with knowledge about fixed-point types needs to review this.
I suggest to
leave fixed-point changes out from the initial patch submission.
Thanks,
Richard.
> Thanks,
> Sebastian