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