On 31/01/2024 14:03, Richard Biener wrote:
On Wed, 31 Jan 2024, Richard Biener wrote:

On Wed, 31 Jan 2024, Andre Vieira (lists) wrote:



On 31/01/2024 12:13, Richard Biener wrote:
On Wed, 31 Jan 2024, Richard Biener wrote:

On Tue, 30 Jan 2024, Andre Vieira wrote:


This patch adds stmt_vec_info to TARGET_SIMD_CLONE_USABLE to make sure the
target can reject a simd_clone based on the vector mode it is using.
This is needed because for VLS SVE vectorization the vectorizer accepts
Advanced SIMD simd clones when vectorizing using SVE types because the
simdlens
might match.  This will cause type errors later on.

Other targets do not currently need to use this argument.

Can you instead pass down the mode?

Thinking about that again the cgraph_simd_clone info in the clone
should have sufficient information to disambiguate.  If it doesn't
then we should amend it.

Richard.

Hi Richard,

Thanks for the review, I don't think cgraph_simd_clone_info is the right place
to pass down this information, since this is information about the caller
rather than the simdclone itself. What we are trying to achieve here is making
the vectorizer being able to accept or reject simdclones based on the ISA we
are vectorizing for. To distinguish between SVE and Advanced SIMD ISAs we use
modes, I am also not sure that's ideal but it is what we currently use. So to
answer your earlier question, yes I can also pass down mode if that's
preferable.

Note cgraph_simd_clone_info has simdlen and we seem to check elsewhere
whether that's POLY or constant.  I wonder how aarch64_sve_mode_p
comes into play here which in the end classifies VLS SVE modes as
non-SVE?

Maybe it's just a bit non-obvious as you key on mangling:

  static int
-aarch64_simd_clone_usable (struct cgraph_node *node)
+aarch64_simd_clone_usable (struct cgraph_node *node, stmt_vec_info
stmt_vinfo)
  {
    switch (node->simdclone->vecsize_mangle)
      {
      case 'n':
        if (!TARGET_SIMD)
         return -1;
+      if (STMT_VINFO_VECTYPE (stmt_vinfo)
+         && aarch64_sve_mode_p (TYPE_MODE (STMT_VINFO_VECTYPE
(stmt_vinfo))))
+       return -1;

?  What does 'n' mean?  It's documented as

   /* The mangling character for a given vector size.  This is used
      to determine the ISA mangling bit as specified in the Intel
      Vector ABI.  */
   unsigned char vecsize_mangle;

I'll update the comment, but yeh 'n' is for Advanced SIMD, 's' is for SVE.

which is slightly misleading.

Reply via email to