On Tue, 9 Jan 2024, Tamar Christina wrote:
> > > -
> > > - gimple_seq_add_seq (&stmts, tem);
> > > -
> > > - scalar_res = gimple_build (&stmts, CFN_EXTRACT_LAST, scalar_type,
> > > - mask, vec_lhs_phi);
> > > + scalar_res = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE
> > (vectype),
> > > + vec_lhs_phi, bitstart);
> >
> > So bitstart is always zero? I wonder why using CFN_VEC_EXTRACT over
> > BIT_FIELD_REF here which wouldn't need any additional target support.
> >
>
> Ok, how about...
>
> ---
>
> I was generating the vector reverse mask without checking if the target
> actually supported such an operation.
>
> This patch changes it to if the bitstart is 0 then use BIT_FIELD_REF instead
> to extract the first element since this is supported by all targets.
>
> This is good for now since masks always come from whilelo. But in the future
> when masks can come from other sources we will need the old code back.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no issues with --enable-checking=release --enable-lto
> --with-build-config=bootstrap-O3 --enable-checking=yes,rtl,extra.
> tested on cross cc1 for amdgcn-amdhsa and issue fixed.
>
> Ok for master?
OK.
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> PR tree-optimization/113199
> * tree-vect-loop.cc (vectorizable_live_operation_1): Use
> BIT_FIELD_REF.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/113199
> * gcc.target/gcn/pr113199.c: New test.
>
> --- inline copy of patch ---
>
> diff --git a/gcc/testsuite/gcc.target/gcn/pr113199.c
> b/gcc/testsuite/gcc.target/gcn/pr113199.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..8a641e5536e80e207ca0163cac66c0f4f6ca93f7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/gcn/pr113199.c
> @@ -0,0 +1,44 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O2" } */
> +
> +typedef long unsigned int size_t;
> +typedef int wchar_t;
> +struct tm
> +{
> + int tm_mon;
> + int tm_year;
> +};
> +int abs (int);
> +struct lc_time_T { const char *month[12]; };
> +struct __locale_t * __get_current_locale (void) { }
> +const struct lc_time_T * __get_time_locale (struct __locale_t *locale) { }
> +const wchar_t * __ctloc (wchar_t *buf, const char *elem, size_t *len_ret) {
> return buf; }
> +size_t
> +__strftime (wchar_t *s, size_t maxsize, const wchar_t *format,
> + const struct tm *tim_p, struct __locale_t *locale)
> +{
> + size_t count = 0;
> + const wchar_t *ctloc;
> + wchar_t ctlocbuf[256];
> + size_t i, ctloclen;
> + const struct lc_time_T *_CurrentTimeLocale = __get_time_locale (locale);
> + {
> + switch (*format)
> + {
> + case L'B':
> + (ctloc = __ctloc (ctlocbuf, _CurrentTimeLocale->month[tim_p->tm_mon],
> &ctloclen));
> + for (i = 0; i < ctloclen; i++)
> + {
> + if (count < maxsize - 1)
> + s[count++] = ctloc[i];
> + else
> + return 0;
> + {
> + int century = tim_p->tm_year >= 0
> + ? tim_p->tm_year / 100 + 1900 / 100
> + : abs (tim_p->tm_year + 1900) / 100;
> + }
> + }
> + }
> + }
> +}
> diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc
> index
> 37f1be1101ffae779214056a0886411e0683e887..39b1161309d8ff8bfe88ee26df9147df0af0a58c
> 100644
> --- a/gcc/tree-vect-loop.cc
> +++ b/gcc/tree-vect-loop.cc
> @@ -10592,7 +10592,17 @@ vectorizable_live_operation_1 (loop_vec_info
> loop_vinfo,
>
> gimple_seq stmts = NULL;
> tree new_tree;
> - if (LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo))
> +
> + /* If bitstart is 0 then we can use a BIT_FIELD_REF */
> + if (integer_zerop (bitstart))
> + {
> + tree scalar_res = gimple_build (&stmts, BIT_FIELD_REF, TREE_TYPE
> (vectype),
> + vec_lhs_phi, bitsize, bitstart);
> +
> + /* Convert the extracted vector element to the scalar type. */
> + new_tree = gimple_convert (&stmts, lhs_type, scalar_res);
> + }
> + else if (LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo))
> {
> /* Emit:
>
> @@ -10618,12 +10628,6 @@ vectorizable_live_operation_1 (loop_vec_info
> loop_vinfo,
> tree last_index = gimple_build (&stmts, PLUS_EXPR, TREE_TYPE (len),
> len, bias_minus_one);
>
> - /* This needs to implement extraction of the first index, but not sure
> - how the LEN stuff works. At the moment we shouldn't get here since
> - there's no LEN support for early breaks. But guard this so there's
> - no incorrect codegen. */
> - gcc_assert (!LOOP_VINFO_EARLY_BREAKS (loop_vinfo));
> -
> /* SCALAR_RES = VEC_EXTRACT <VEC_LHS, LEN + BIAS - 1>. */
> tree scalar_res
> = gimple_build (&stmts, CFN_VEC_EXTRACT, TREE_TYPE (vectype),
> @@ -10648,32 +10652,6 @@ vectorizable_live_operation_1 (loop_vec_info
> loop_vinfo,
> &LOOP_VINFO_MASKS (loop_vinfo),
> 1, vectype, 0);
> tree scalar_res;
> -
> - /* For an inverted control flow with early breaks we want EXTRACT_FIRST
> - instead of EXTRACT_LAST. Emulate by reversing the vector and mask. */
> - if (restart_loop && LOOP_VINFO_EARLY_BREAKS (loop_vinfo))
> - {
> - /* First create the permuted mask. */
> - tree perm_mask = perm_mask_for_reverse (TREE_TYPE (mask));
> - tree perm_dest = copy_ssa_name (mask);
> - gimple *perm_stmt
> - = gimple_build_assign (perm_dest, VEC_PERM_EXPR, mask,
> - mask, perm_mask);
> - vect_finish_stmt_generation (loop_vinfo, stmt_info, perm_stmt,
> - &gsi);
> - mask = perm_dest;
> -
> - /* Then permute the vector contents. */
> - tree perm_elem = perm_mask_for_reverse (vectype);
> - perm_dest = copy_ssa_name (vec_lhs_phi);
> - perm_stmt
> - = gimple_build_assign (perm_dest, VEC_PERM_EXPR, vec_lhs_phi,
> - vec_lhs_phi, perm_elem);
> - vect_finish_stmt_generation (loop_vinfo, stmt_info, perm_stmt,
> - &gsi);
> - vec_lhs_phi = perm_dest;
> - }
> -
> gimple_seq_add_seq (&stmts, tem);
>
> scalar_res = gimple_build (&stmts, CFN_EXTRACT_LAST, scalar_type,
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
