On Tue, Jul 21, 2015 at 8:42 PM, Sebastian Pop <seb...@gmail.com> 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