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.

>  >>>         /* 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

Reply via email to