On Thu, 23 Mar 2023, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs on aarch64-linux, because
> expand_vector_condition attempts to piecewise lower SVE
> d_3 = a_1(D) < b_2(D);
> _5 = VEC_COND_EXPR <d_3, c_4(D), d_3>;
> which isn't possible - nunits_for_known_piecewise_op ICEs but
> the rest of the code assumes constant number of elements too.
>
> expand_vector_condition attempts to find if a (rhs1) is a SSA_NAME
> for comparison and calls expand_vec_cond_expr_p (type, TREE_TYPE (a1), code)
> where a1 is one of the operands of the comparison and code is the comparison
> code. That one indeed isn't supported here, but what aarch64 SVE supports
> are the individual statements, comparison (expand_vec_cmp_expr_p) and
> expand_vec_cond_expr_p (type, TREE_TYPE (a), SSA_NAME), the latter because
> that function starts with
> if (VECTOR_BOOLEAN_TYPE_P (cmp_op_type)
> && get_vcond_mask_icode (TYPE_MODE (value_type),
> TYPE_MODE (cmp_op_type)) != CODE_FOR_nothing)
> return true;
>
> In an earlier version of the patch (in the PR), we did this
> if (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (a))
> && expand_vec_cond_expr_p (type, TREE_TYPE (a), ERROR_MARK))
> return true;
> before the code == SSA_NAME handling plus some further tweaks later.
> While that fixed the ICE, it broke quite a few tests on x86 and some on
> aarch64 too. The problem is that expand_vector_comparison doesn't lower
> comparisons which aren't supported and only feed VEC_COND_EXPR first operand
> and expand_vector_condition succeeds for those, so with the above mentioned
> change we'd verify the VEC_COND_EXPR is implementable using optab alone,
> but nothing would verify the tcc_comparison which relied on
> expand_vector_condition to verify.
Ah, indeed - all a bit twisty.
> So, the following patch instead queries whether optabs can handle the
> comparison and VEC_COND_EXPR together (if a (rhs1) is a comparison;
> otherwise as before it checks only the VEC_COND_EXPR) and if that fails,
> also checks whether the two operations could be supported individually
> and only if even that fails does the piecewise lowering.
>
> Bootstrapped/regtested on x86_64-linux, i686-linux and aarch64-linux, ok for
> trunk?
OK.
Thanks for digging into it.
Richard.
> 2023-03-23 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/109176
> * tree-vect-generic.cc (expand_vector_condition): If a has
> vector boolean type and is a comparison, also check if both
> the comparison and VEC_COND_EXPR could be successfully expanded
> individually.
>
> * gcc.target/aarch64/sve/pr109176.c: New test.
>
> --- gcc/tree-vect-generic.cc.jj 2023-03-21 13:28:21.354671095 +0100
> +++ gcc/tree-vect-generic.cc 2023-03-22 12:53:27.853986127 +0100
> @@ -1063,6 +1063,15 @@ expand_vector_condition (gimple_stmt_ite
> return true;
> }
>
> + /* If a has vector boolean type and is a comparison, above
> + expand_vec_cond_expr_p might fail, even if both the comparison and
> + VEC_COND_EXPR could be supported individually. See PR109176. */
> + if (a_is_comparison
> + && VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (a))
> + && expand_vec_cond_expr_p (type, TREE_TYPE (a), SSA_NAME)
> + && expand_vec_cmp_expr_p (TREE_TYPE (a1), TREE_TYPE (a), code))
> + return true;
> +
> /* Handle vector boolean types with bitmasks. If there is a comparison
> and we can expand the comparison into the vector boolean bitmask,
> or otherwise if it is compatible with type, we can transform
> --- gcc/testsuite/gcc.target/aarch64/sve/pr109176.c.jj 2023-03-22
> 12:19:21.672218631 +0100
> +++ gcc/testsuite/gcc.target/aarch64/sve/pr109176.c 2023-03-22
> 12:19:21.672218631 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/109176 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O2" } */
> +
> +#include <arm_sve.h>
> +
> +svbool_t
> +foo (svint8_t a, svint8_t b, svbool_t c)
> +{
> + svbool_t d = svcmplt_s8 (svptrue_pat_b8 (SV_ALL), a, b);
> + return svsel_b (d, c, d);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
