Hi! What is going on with this patch, will anybody commit it?
This is also http://gcc.gnu.org/PR80631 apparently. The patch doesn't apply cleanly to current trunk due to the reduc_code -> reduc_fn changes. As it doesn't apply, I can't easily check what the patch generates on the PR80631 testcases vs. my thoughts on that; though, if it emits something more complicated for the simple cases, perhaps we could improve those (not handle it like COND_REDUCTION, but instead as former INTEGER_INDUC_COND_REDUCTION and just use a different constant instead of 0 if 0 isn't usable for the condition never matched value. We could check the value from the loop preheader edge of the induc_stmt phi and if it is constant and for REDUC_MAX (does that imply always positive step?) larger than minimum value of the type, we can use the minimum of the type as the value instead of zero. If REDUC_MIN and the initial constant is smaller than the maximum value of the type, we could use the maximum. Otherwise punt to what you're doing. Or instead of minimum or maximum check the value of initial_def, if it is constant, if that value is usable, we could even avoid the final COND_EXPR. Another thing is that is_nonwrapping_integer_induction has: /* Make sure the loop is integer based. */ if (TREE_CODE (base) != INTEGER_CST || TREE_CODE (step) != INTEGER_CST) return false; /* Check that the induction increments. */ if (tree_int_cst_sgn (step) == -1) return false; if (TYPE_OVERFLOW_UNDEFINED (lhs_type)) return true; First of all, I fail to see why we don't handle negative step, that can be done with REDUC_MIN instead of REDUC_MAX. And, I also fail to see why we need to require INTEGER_CST base if TYPE_OVERFLOW_UNDEFINED (lhs_type), in that case we know it will not wrap, so all we care about is if step * ni won't cover all iterations and we can use some value (type minimum or maximum) to denote the condition never true case. On Wed, Nov 22, 2017 at 05:57:08PM +0100, Kilian Verhetsel wrote: > 2017-11-21 Kilian Verhetsel <kilian.verhet...@uclouvain.be> Missing space before <. > gcc/ChangeLog: > PR testsuite/81179 > * tree-vect-loop.c > (vect_create_epilog_for_reduction): Fix the returned value for 8 spaces instead of a tab, note the (vect_create_epilog_for_reduction): should just go after space right after the filename. > INTEGER_INDUC_COND_REDUCTION whose last match occurred at > index 0. > (vectorizable_reduction): For INTEGER_INDUC_COND_REDUCTION, > pass the PHI statement that sets the induction variable to the > code generating the epilogue and check that no overflow will > occur. > > gcc/testsuite/ChangeLog: > * gcc.dg/vect/vect-iv-cond-reduc-overflow-1.c: New test for > overflows when compiling conditional reductions. > * gcc.dg/vect/vect-iv-cond-reduc-overflow-2.c: Likewise. Again, spaces instead of tab twice. > - if (STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) == COND_REDUCTION) > + if (STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) == COND_REDUCTION > + || STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) > + == INTEGER_INDUC_COND_REDUCTION) Formatting, == should be below STMT_VINFO on the previous line, for emacs users perhaps even better surrounded by ()s, like: if (STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) == COND_REDUCTION || (STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info) == INTEGER_INDUC_COND_REDUCTION)) (happens many times in the patch). Jakub
