> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: Monday, May 11, 2026 8:17 PM
> To: Cui, Lili <[email protected]>
> Cc: [email protected]; Liu, Hongtao <[email protected]>
> Subject: Re: [PATCH 1/2] tree-optimization/vect: Allow commutative operand
> swap for IFN in SLP reduction
>
> > --- a/gcc/tree-vect-slp.cc
> > +++ b/gcc/tree-vect-slp.cc
> > @@ -1401,7 +1401,14 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned
> char *swap,
> > && STMT_VINFO_REDUC_IDX (stmt_info) != -1
> > && rhs_code.is_tree_code ()
> > && commutative_tree_code (tree_code (rhs_code))
> > - && first_reduc_idx == 1 - STMT_VINFO_REDUC_IDX
> (stmt_info)))
> > + && first_reduc_idx == 1 - STMT_VINFO_REDUC_IDX
> (stmt_info))
> > + && !(first_reduc_idx != -1
> > + && STMT_VINFO_REDUC_IDX (stmt_info) != -1
> > + && rhs_code.is_internal_fn ()
> > + && first_commutative_argument (internal_fn (rhs_code)) >=
> 0
> > + && (first_reduc_idx + STMT_VINFO_REDUC_IDX (stmt_info)
> > + == 2 * first_commutative_argument (
> > + internal_fn (rhs_code)) + 1)))
>
> I suppose
>
> first_reduc_idx == first_commutative_argument (...) + 1
> - STMT_VINFO_REDUC_IDX (stmt_info)
>
> would have worked as well. These are basically sanity checks that the indexes
> are either equal or the current one is "the other".
>
> Note that there is first_commutative_argument (code_helper, type) that we
> could have used to unify the tree-code and internal-fn paths. 'type' would be
> the return type aka TREE_TYPE (lhs).
>
> The patch is OK, but it would be nice to do the unification.
>
Hi Richard,
Thanks for the review and suggestions!
You're right. I've simplified the implementation to use the unified
first_commutative_argument(code_helper, tree) interface, which handles both
tree codes and internal functions in one path.
The updated check now looks like:
@@ -1391,6 +1391,7 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char
*swap,
}
else
{
+ int comm_arg;
if (first_reduc_idx != STMT_VINFO_REDUC_IDX (stmt_info)
/* For SLP reduction groups the index isn't necessarily
uniform but only that of the first stmt matters. */
@@ -1399,9 +1400,9 @@ vect_build_slp_tree_1 (vec_info *vinfo, unsigned char
*swap,
&& REDUC_GROUP_FIRST_ELEMENT (stmt_info))
&& !(first_reduc_idx != -1
&& STMT_VINFO_REDUC_IDX (stmt_info) != -1
- && rhs_code.is_tree_code ()
- && commutative_tree_code (tree_code (rhs_code))
- && first_reduc_idx == 1 - STMT_VINFO_REDUC_IDX (stmt_info)))
+ && (comm_arg = first_commutative_argument
+ (rhs_code, TREE_TYPE (gimple_get_lhs
(stmt)))) >= 0
+ && (first_reduc_idx == 2 * comm_arg + 1 -
STMT_VINFO_REDUC_IDX (stmt_info))))
This unifies the tree-code and IFN paths. The formula first_reduc_idx == 2 *
comm_arg + 1 - STMT_VINFO_REDUC_IDX verifies that the two indices are either
equal or represent "the other" commutative operand position:
For tree codes (PLUS_EXPR, etc.):
- comm_arg = 0
- Formula: first_reduc_idx == 1 - STMT_VINFO_REDUC_IDX
- Example: reduc_idx 0 and 1 satisfy 0 == 1-1 or 1 == 1-0 ✓
For internal functions (.COND_ADD, etc.):
- comm_arg = 1
- Formula: first_reduc_idx == 3 - STMT_VINFO_REDUC_IDX
- Example: .COND_ADD(cond, arg1, arg2, else) where reduc_idx 1 and 2
satisfy 1 == 3-2 or 2 == 3-1 ✓
Bootstrapped and retested on x86_64-linux-gnu. Performance improvements not
changed.
Does this look better?
Thanks,
Lili
> Thanks,
> Richard.
