On Wed, Nov 8, 2023 at 5:18 PM Robin Dapp <rdapp....@gmail.com> wrote:
>
> Hi,
>
> as Tamar reported in PR112406 we still ICE on aarch64 in SPEC2017
> when creating COND_OPs in ifcvt.
>
> The problem is that we fail to deduce the mask's type from the statement
> vectype and then end up with a non-matching mask in expand. This patch
> checks if the current op is equal to the mask operand and, if so, uses
> the truth type from the stmt_vectype. Is that a valid approach?
>
> Bootstrapped and regtested on aarch64, x86 is running.
>
> Besides, the testcase is Tamar's reduced example, originally from
> SPEC. I hope it's ok to include it as is (as imagick is open source
> anyway).
For the fortran testcase we don't even run into this but hit an
internal def and assert on
gcc_assert (STMT_VINFO_VEC_STMTS (def_stmt_info).length () == ncopies);
I think this shows missing handling of .COND_* in the bool pattern recognition
as we get the 'bool' condition as boolean data vector rather than a mask. The
same is true for the testcase with the invariant condition. This causes us to
select the wrong vector type here. The "easiest" might be to look at
how COND_EXPR is handled in vect_recog_bool_pattern and friends and
handle .COND_* IFNs the same for the mask operand.
Richard.
> Regards
> Robin
>
> gcc/ChangeLog:
>
> PR middle-end/112406
>
> * tree-vect-stmts.cc (vect_get_vec_defs_for_operand): Handle
> masks of conditional ops.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/pr112406.c: New test.
> ---
> gcc/testsuite/gcc.dg/pr112406.c | 37 +++++++++++++++++++++++++++++++++
> gcc/tree-vect-stmts.cc | 20 +++++++++++++++++-
> 2 files changed, 56 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/pr112406.c
>
> diff --git a/gcc/testsuite/gcc.dg/pr112406.c b/gcc/testsuite/gcc.dg/pr112406.c
> new file mode 100644
> index 00000000000..46459c68c4a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr112406.c
> @@ -0,0 +1,37 @@
> +/* { dg-do compile { target { aarch64*-*-* } } } */
> +/* { dg-options "-march=armv8-a+sve -w -Ofast" } */
> +
> +typedef struct {
> + int red
> +} MagickPixelPacket;
> +
> +GetImageChannelMoments_image, GetImageChannelMoments_image_0,
> + GetImageChannelMoments___trans_tmp_1, GetImageChannelMoments_M11_0,
> + GetImageChannelMoments_pixel_3, GetImageChannelMoments_y,
> + GetImageChannelMoments_p;
> +
> +double GetImageChannelMoments_M00_0, GetImageChannelMoments_M00_1,
> + GetImageChannelMoments_M01_1;
> +
> +MagickPixelPacket GetImageChannelMoments_pixel;
> +
> +SetMagickPixelPacket(int color, MagickPixelPacket *pixel) {
> + pixel->red = color;
> +}
> +
> +GetImageChannelMoments() {
> + for (; GetImageChannelMoments_y; GetImageChannelMoments_y++) {
> + SetMagickPixelPacket(GetImageChannelMoments_p,
> + &GetImageChannelMoments_pixel);
> + GetImageChannelMoments_M00_1 += GetImageChannelMoments_pixel.red;
> + if (GetImageChannelMoments_image)
> + GetImageChannelMoments_M00_1++;
> + GetImageChannelMoments_M01_1 +=
> + GetImageChannelMoments_y * GetImageChannelMoments_pixel_3;
> + if (GetImageChannelMoments_image_0)
> + GetImageChannelMoments_M00_0++;
> + GetImageChannelMoments_M01_1 +=
> + GetImageChannelMoments_y * GetImageChannelMoments_p++;
> + }
> + GetImageChannelMoments___trans_tmp_1 = atan(GetImageChannelMoments_M11_0);
> +}
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 65883e04ad7..6793b01bf44 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1238,10 +1238,28 @@ vect_get_vec_defs_for_operand (vec_info *vinfo,
> stmt_vec_info stmt_vinfo,
> tree stmt_vectype = STMT_VINFO_VECTYPE (stmt_vinfo);
> tree vector_type;
>
> + /* For a COND_OP the mask operand's type must not be deduced from the
> + scalar type but from the statement's vectype. */
> + bool use_stmt_vectype = false;
> + gcall *call;
> + if ((call = dyn_cast <gcall *> (STMT_VINFO_STMT (stmt_vinfo)))
> + && gimple_call_internal_p (call))
> + {
> + internal_fn ifn = gimple_call_internal_fn (call);
> + int mask_idx = -1;
> + if (ifn != IFN_LAST
> + && (mask_idx = internal_fn_mask_index (ifn)) != -1)
> + {
> + tree maskop = gimple_call_arg (call, mask_idx);
> + if (op == maskop)
> + use_stmt_vectype = true;
> + }
> + }
> +
> if (vectype)
> vector_type = vectype;
> else if (VECT_SCALAR_BOOLEAN_TYPE_P (TREE_TYPE (op))
> - && VECTOR_BOOLEAN_TYPE_P (stmt_vectype))
> + && (use_stmt_vectype || VECTOR_BOOLEAN_TYPE_P (stmt_vectype)))
> vector_type = truth_type_for (stmt_vectype);
> else
> vector_type = get_vectype_for_scalar_type (loop_vinfo, TREE_TYPE
> (op));
> --
> 2.41.0
