On Thu, Nov 6, 2025 at 4:37 PM Robin Dapp <[email protected]> wrote:
>
> Hi,
>
> The gather/scatter relaxation patches introduced a bug with
> vect_use_strided_gather_scatters_p. I didn't want to pass
> supported_offset_vectype and supported scale all the way from
> vect_truncate_gather_scatter_offset and
> vect_use_strided_gather_scatters_p to get_load_store_type so
> just called vect_gather_scatter_fn_p again afterwards to determine
> the supported type and scale.
>
> However, this doesn't take into account that
> vect_use_strided_gather_scatters_p changes the offset type after
> verifying that we can use gather/scatter.
>
> The flow right now is
> - vect_use_strided_gather_scatters_p calls vect_check_gather_scatter
> with e.g. a char offset type.
> - We actually need/support a short vector offset type and
> vect_use_strided_gather_scatters_p fold converts the actual (scalar)
> char offset to a short offset.
> - We call vect_gather_scatter_fn_p with the new short offset instead of
> the original char one, thinking we need an even larger int offset type.
>
> The last call is obviously not identical to the ones we used to check
> gather/scatter in the first place and can fail if there is no offset
> vectype.
>
> There are several ways to fix this. The most obvious one is to bite the
> bullet and just add the supported_offset_vectype and supported_scale to
> all the intermediate functions. I wondered, however, if we need the
> offset conversion at all. As far as I can tell we don't ever use
> the scalar offset type and vect_get_strided_load_store_ops in particular
> uses offset_vectype. Thus, this patch removes the conversion.
>
> I bootstrapped and regtested this, before and after the relaxation
> patches, on x86 and power10. Regtested on aarch64 and riscv.
ISTR hitting this code path since the actual scalar offset we'll end
up using might be of different type.
But if test coverage doesn't show anything I'd say go ahead - simplification
is always good (and adding test coverage in case there's fallout).
OK.
Thanks,
Richard.
> Regards
> Robin
>
> gcc/ChangeLog:
>
> * tree-vect-stmts.cc (vect_use_strided_gather_scatters_p):
> Do not convert offset type.
> ---
> gcc/tree-vect-stmts.cc | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index d153544640a..a60e9a440d4 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1856,15 +1856,6 @@ vect_use_strided_gather_scatters_p (stmt_vec_info
> stmt_info, tree vectype,
> masked_p, gs_info, elsvals))
> return false;
> }
> - else
> - {
> - tree old_offset_type = TREE_TYPE (gs_info->offset);
> - tree new_offset_type = TREE_TYPE (gs_info->offset_vectype);
> -
> - gcc_assert (TYPE_PRECISION (new_offset_type)
> - >= TYPE_PRECISION (old_offset_type));
> - gs_info->offset = fold_convert (new_offset_type, gs_info->offset);
> - }
>
> if (!single_element_p
> && !targetm.vectorize.prefer_gather_scatter (TYPE_MODE (vectype),
> --
> 2.51.0
>
