On Fri, Apr 14, 2023 at 11:09 AM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> On Fri, Apr 14, 2023 at 10:43 AM Andre Vieira (lists)
> <andre.simoesdiasvie...@arm.com> wrote:
> >
> > Resending this to everyone (sorry for the double send Richard).
> >
> > On 14/04/2023 09:15, Andre Vieira (lists) wrote:
> > >
> > >
> > > On 14/04/2023 07:55, Richard Biener wrote:
> > >> On Thu, Apr 13, 2023 at 4:25 PM Andre Vieira (lists)
> > >> <andre.simoesdiasvie...@arm.com> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On 13/04/2023 15:00, Richard Biener wrote:
> > >>>> On Thu, Apr 13, 2023 at 3:00 PM Andre Vieira (lists) via Gcc-patches
> > >>>> <gcc-patches@gcc.gnu.org> wrote:
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>
> > >>> But that's not it, I've been looking at it, and there is code in place
> > >>> that does what I expected which is defer the choice of vectype for simd
> > >>> clones until vectorizable_simd_clone_call, unfortunately it has a
> > >>> mistaken assumption that simdclones don't return :/
> > >>
> > >> I think that's not it - when the SIMD clone returns a vector we have to
> > >> determine the vector type in this function. We cannot defer this.
> > >
> > > What's 'this function' here, do you mean we have to determine the
> > > vectype in 'vect_get_vector_types_for_stmt' &
> > > 'vect_determine_vf_for_stmt' ?
>
> Yes.
>
> > Because at that time we don't yet know
> > > what clone we will be using, this choice is done inside
> > > vectorizable_simd_clone_call. In fact, to choose the simd clone, we need
> > > to know the vf as that has to be a multiple of the chosen clone's
> > > simdlen. So we simply can't use the simdclone's types (as that depends
> > > on the simdlen) to choose the vf because the choice of simdlend depends
> > > on the vf. And there was already code in place to handle this,
> > > unfortunately that code was wrong and had the wrong assumption that
> > > simdclones didn't return (probably was true at some point and bitrotted).
>
> But to compute the VF we need to know the vector types! We're only
> calling vectorizable_* when the VF is final. That said, the code you quote:
>
> > >>
> > >>> see vect_get_vector_types_for_stmt:
> > >>> ...
> > >>> if (gimple_get_lhs (stmt) == NULL_TREE
>
> is just for the case of a function without return value. For this case
> it's OK to do nothing - 'vectype' is the vector type of all vector defs
> a stmt produces.
>
> For calls with a LHS it should fall through to generic code doing
> get_vectype_for_scalar_type on the LHS type.
Isn't just the target selector wrong in these tests?
/* { dg-final { scan-tree-dump-times {[\n\r] [^\n]* = foo\.simdclone}
2 "vect" { target { { ! avx_runtime } && { ! { i686*-*-* && { ! lp64 }
} } } } } } */
/* { dg-final { scan-tree-dump-times {[\n\r] [^\n]* = foo\.simdclone}
3 "vect" { target { avx_runtime && { ! { i686*-*-* && { ! lp64 } } } }
} } } */
/* { dg-final { scan-tree-dump-times {[\n\r] [^\n]* = foo\.simdclone}
4 "vect" { target { i686*-*-* && { ! lp64 } } } } } */
the i686-*-* should be { i?86-*-* x86_64-*-* } && { ! lp64 } to
properly check x86_64 -m32?
I'm going to patch that.
Richard.
> > >>> /* MASK_STORE has no lhs, but is ok. */
> > >>> && !gimple_call_internal_p (stmt, IFN_MASK_STORE))
> > >>> {
> > >>> if (is_a <gcall *> (stmt))
> > >>> {
> > >>> /* Ignore calls with no lhs. These must be calls to
> > >>> #pragma omp simd functions, and what vectorization
> > factor
> > >>> it really needs can't be determined until
> > >>> vectorizable_simd_clone_call. */
> > >>> if (dump_enabled_p ())
> > >>> dump_printf_loc (MSG_NOTE, vect_location,
> > >>> "defer to SIMD clone analysis.\n");
> > >>> return opt_result::success ();
> > >>> }
> > >>>
> > >>> return opt_result::failure_at (stmt,
> > >>> "not vectorized: irregular
> > >>> stmt.%G", stmt);
> > >>> }
> > >>> ...
> > >>>
> > >>> I'm working on a patch.
> > >>>>
> > >>>>> Kind Regards,
> > >>>>> Andre
