On Mon, 4 Dec 2017, Bin.Cheng wrote:
> On Mon, Dec 4, 2017 at 1:11 PM, Richard Biener <rguent...@suse.de> wrote:
> >
> > I've noticed we perform FP reduction association without the required
> > checks for associative math. I've added
> > gcc.dg/tree-ssa/loop-interchange-1b.c to cover this.
> >
> > I also noticed we happily interchange a loop with a reduction like
> >
> > sum = a[i] - sum;
> >
> > where a change in order of elements isn't ok. Unfortunately bwaves
> > is exactly a case where single_use != next_def (tried to simply remove
> > that case for now), because reassoc didn't have a chance to fix the
> > operand order. Thus this patch exports the relevant handling from
> > the vectorizer (for stage1 having a separate infrastructure gathering /
> > analyzing of reduction/induction infrastructure would be nice...)
> > and uses it from interchange. We then don't handle
> > gcc.dg/tree-ssa/loop-interchange-4.c anymore (similar vectorizer
> > missed-opt is PR65930). I didn't bother to split up the vectorizer
> > code further to implement relaxed validity checking but simply XFAILed
> > this testcase.
> >
> > Earlier I simplified allocation stuff in the main loop which is why
> > this part is included in this patch.
> >
> > Bootstrap running on x86_64-unknown-linux-gnu.
> >
> > I'll see to craft a testcase with the sum = a[i] - sum; mis-handling.
> >
> > Ok?
> Sure.
> Just for the record. There is also similar associative check in
> predcom. As you suggested, a path extraction/checking interface for
> associative checking would be great, given we have multiple users now.
Yeah. Note for interchange we don't need associativeness in the sense
as implemented by associative_tree_code, we need left-associativeness
which also MINUS_EXPR provides. So my check isn't perfect. I guess
we instead need
associative_tree_code ()
|| (code == MINUS_EXPR
&& use_p->use == gimple_assign_rhs1_ptr (single_use))
where we could also allow RDIV_EXPR and other left-associative
tree codes (but check_reduction_path would do the wrong thing
with those at the moment but it has MINUS_EXPR handling that
would support sum = x - (y - sum) which the above rejects).
So I'm retesting with
/* Check the reduction operation. We require a left-associative
operation.
For FP math we also need to be allowed to associate operations. */
enum tree_code code = gimple_assign_rhs_code (single_use);
if (gassign *ass = dyn_cast <gassign *> (single_use))
{
enum tree_code code = gimple_assign_rhs_code (ass);
if (! (associative_tree_code (code)
|| (code == MINUS_EXPR
&& use_p->use == gimple_assign_rhs1_ptr (ass)))
|| (FLOAT_TYPE_P (TREE_TYPE (var))
&& ! flag_associative_math))
return false;
}
else
return false;
which is more restrictive than before.
Richard.
