On Wed, 6 Jan 2021, Richard Sandiford wrote:
> PR98560 is about a case in which the vectoriser initially generates:
>
> mask_1 = a < 0;
> mask_2 = mask_1 & ...;
> res = VEC_COND_EXPR <mask_2, b, c>;
>
> The vectoriser thus expects res to be calculated using vcond_mask.
> However, we later manage to fold mask_2 to mask_1, leaving:
>
> mask_1 = a < 0;
> res = VEC_COND_EXPR <mask_1, b, c>;
>
> gimple-isel then required a combined vcond to exist.
>
> On most targets, it's not too onerous to provide all possible
> (compare x select) combinations. For each data mode, you just
> need to provide unsigned comparisons, signed comparisons, and
> floating-point comparisons, with the data mode and type of
> comparison uniquely determining the mode of the compared values.
> But for targets like SVE that support “unpacked” vectors,
> it's not that simple: the level of unpacking adds another
> degree of freedom.
>
> Rather than insist that the combined versions exist, I think
> we should be prepared to fall back to using separate comparisons
> and vcond_masks. I think that makes more sense on targets like
> AArch64 and AArch32 in which compares and selects are fundementally
> separate operations anyway.
Indeed the mask variants (thus being able to expand the comparison)
are more fundamental. I guess you're running into this path because
we did not consider using vcond_mask because of
if (used_vec_cond_exprs >= 2
&& (get_vcond_mask_icode (mode, TYPE_MODE (op0_type))
!= CODE_FOR_nothing)
&& expand_vec_cmp_expr_p (op0a_type, op0_type, tcode))
{
/* Keep the SSA name and use vcond_mask. */
tcode = TREE_CODE (op0);
}
not triggering? Which also means your patch fails to check/assert
that we can expand_vec_cmp_expr_p the separate compare?
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
It does feel like the function could need some refactoring ...
But OK - preferably with the assertion that we can actually
expand the compare (I suggest to do the expand_vec_cmp_expr_p
above unconditionally and have a 'global' cannot_expand_mask
flag defaulted to false and checked in the new path).
Thanks,
Richard.
> Richard
>
>
> gcc/
> PR tree-optimization/98560
> * gimple-isel.cc (gimple_expand_vec_cond_expr): If we fail to use
> IFN_VCOND{,U,EQ}, fall back on IFN_VCOND_MASK.
>
> gcc/testsuite/
> PR tree-optimization/98560
> * gcc.dg/vect/pr98560-1.c: New test.
> ---
> gcc/gimple-isel.cc | 9 ++++++++-
> gcc/testsuite/gcc.dg/vect/pr98560-1.c | 17 +++++++++++++++++
> 2 files changed, 25 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/vect/pr98560-1.c
>
> diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> index d40338ce4a2..9c07d79a86c 100644
> --- a/gcc/gimple-isel.cc
> +++ b/gcc/gimple-isel.cc
> @@ -254,7 +254,14 @@ gimple_expand_vec_cond_expr (gimple_stmt_iterator *gsi,
> }
> }
>
> - gcc_assert (icode != CODE_FOR_nothing);
> + if (icode == CODE_FOR_nothing)
> + {
> + gcc_assert (VECTOR_BOOLEAN_TYPE_P (TREE_TYPE (op0))
> + && (get_vcond_mask_icode (mode, TYPE_MODE (TREE_TYPE (op0)))
> + != CODE_FOR_nothing));
> + return gimple_build_call_internal (IFN_VCOND_MASK, 3, op0, op1, op2);
> + }
> +
> tree tcode_tree = build_int_cst (integer_type_node, tcode);
> return gimple_build_call_internal (unsignedp ? IFN_VCONDU : IFN_VCOND,
> 5, op0a, op0b, op1, op2, tcode_tree);
> diff --git a/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> new file mode 100644
> index 00000000000..2583fc48f8a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr98560-1.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3 -fno-tree-vrp -fno-tree-fre -fno-tree-pre
> -fno-code-hoisting -fvect-cost-model=dynamic" } */
> +/* { dg-additional-options "-msve-vector-bits=128" { target aarch64_sve } }
> */
> +
> +#include <stdint.h>
> +
> +void
> +f (uint16_t *restrict dst, uint32_t *restrict src1, float *restrict src2)
> +{
> + int i = 0;
> + for (int j = 0; j < 4; ++j)
> + {
> + uint16_t tmp = src1[i] >> 1;
> + dst[i] = (uint16_t) (src2[i] < 0 && i < 4 ? tmp : 1);
> + i += 1;
> + }
> +}
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
