On Mon, Jul 28, 2025 at 3:50 PM Andrew Stubbs <a...@baylibre.com> wrote: > > On 28/07/2025 10:46, Richard Biener wrote: > > On Fri, Jul 25, 2025 at 6:37 PM Andrew Stubbs <a...@baylibre.com> wrote: > >> > >> On 23/07/2025 16:23, Richard Biener wrote: > >>>>> That said, the hook is a bit black/white - whether the target prefers > >>>>> a gather/scatter over N piecewise operations with equal stride depends > >>>>> at least on the vector mode. On x86_64 for V2DImode definitely no > >>>>> gather, for V16SFmode it probably depends (V64QImode gather isn't > >>>>> supported). > >>>> > >>>> I expected this one might come up. What features would you like to see > >>>> in a hook? Should it be the mode, or the vectype? I see also masked_p > >>>> that might be relevant to some architecture? > >>> > >>> The mode, the (possibly constant) stride and the group size > >> > >> How is this updated patch? > >> > >> I couldn't figure out how to get the stride, so I've used the "scale", > >> which matches what various other hooks and instructions get. > >> > >> The hook is now called after the call to > >> vect_use_strided_gather_scatters_p, which partly means that the hook > >> doesn't get called in cases where gather/scatter would never have been > >> allowed anyway, but mostly just because the gs_info isn't populated > >> until after than call. > > > > +DEFHOOK > > +(prefer_gather_scatter, > > + "This hook returns TRUE if gather loads or scatter stores are cheaper > > on\n\ > > +this target than a sequence of elementwise loads or stores. ", > > + bool, > > + (machine_mode mode, int scale, unsigned int group_size), > > + hook_bool_void_false) > > > > hook_bool_void_false looks like a wrong initializer. You probably need > > to implement the default. I also miss mentioning of SCALE and GROUP_SIZE > > and MODE in the description and what they are. > > Yes, sorry, that was not meant to be left like that. Fixed now. > > > - && single_element_p > > && SLP_TREE_LANES (slp_node) == 1 > > && loop_vinfo > > && vect_use_strided_gather_scatters_p (stmt_info, loop_vinfo, > > - masked_p, gs_info, elsvals)) > > + masked_p, gs_info, elsvals) > > + && (targetm.vectorize.prefer_gather_scatter (TYPE_MODE (vectype), > > + gs_info->scale, > > group_size) > > + || single_element_p)) > > > > I'd have prefered to move the check into vect_use_strided_gather_scatters_p > > since it logically belongs there. > > I think the attached is logically the same as before, with the exception > that it now also outputs the dump message on all of the "true" paths, > but none of the "false" paths (previously, the message would be skipped > when vect_truncate_gather_scatter_offset return true). > > OK?
OK. Thanks, Richard. > > Thanks for the review. > > Andrew
