Re: [Patch, AArch64] Restructure arm_neon.h vector types' implementation.

[James Greenhalgh <james.greenhalgh@arm.com>]

On Fri, Jun 27, 2014 at 04:32:19PM +0100, Tejas Belagod wrote:
> +/* Internal scalar builtin types.  These types are used to support
> +   neon intrinsic builtins.  They are _not_ user-visible types.  Therefore
> +   the mangling for these types are implementation defined.  */
> +const char *aarch64_scalar_builtin_types[] = {
> +  "__builtin_aarch64_simd_qi",
> +  "__builtin_aarch64_simd_hi",
> +  "__builtin_aarch64_simd_si",
> +  "__builtin_aarch64_simd_sf",
> +  "__builtin_aarch64_simd_di",
> +  "__builtin_aarch64_simd_df",
> +  "__builtin_aarch64_simd_poly8",
> +  "__builtin_aarch64_simd_poly16",
> +  "__builtin_aarch64_simd_poly64",
> +  "__builtin_aarch64_simd_poly128",
> +  "__builtin_aarch64_simd_ti",
> +  "__builtin_aarch64_simd_uqi",
> +  "__builtin_aarch64_simd_uhi",
> +  "__builtin_aarch64_simd_usi",
> +  "__builtin_aarch64_simd_udi",
> +  "__builtin_aarch64_simd_ei",
> +  "__builtin_aarch64_simd_oi",
> +  "__builtin_aarch64_simd_ci",
> +  "__builtin_aarch64_simd_xi",
> +  NULL
> +};
<snip>
> +static const char *
> +aarch64_mangle_builtin_scalar_type (const_tree type)
> +{
> +  int i = 0;
> +
> +  while (aarch64_scalar_builtin_types[i] != NULL)
>      {
> -      default:
> -	eltype = aarch64_build_scalar_type (GET_MODE_INNER (mode),
> -					    unsigned_p, poly_p);
> -	return build_vector_type_for_mode (eltype, mode);
> -	break;
> -      VECTOR_TYPES
> -   }
> +      const char *name = aarch64_scalar_builtin_types[i];
> +
> +      if (TREE_CODE (TYPE_NAME (type)) == TYPE_DECL
> +	  && DECL_NAME (TYPE_NAME (type))
> +	  && !strcmp (IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (type))), name))
> +	return aarch64_scalar_builtin_types[i];
> +      i++;
> +    }
> +  return NULL;
>  }
<snip>
> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
> index 3ed8a98..50d294e 100644
> --- a/gcc/config/aarch64/arm_neon.h
> +++ b/gcc/config/aarch64/arm_neon.h
> @@ -32,66 +32,45 @@
> +typedef __Poly8_t poly8_t;
> +typedef __Poly16_t poly16_t;
> +typedef __Poly64_t poly64_t;
> +typedef __Poly128_t poly128_t;

This looks wrong to me. The type which eventually becomes poly8_t in
arm_neon.h has "__Poly8_t" as its internal type name.

When you go through the loop in aarch64_mangle_builtin_scalar_type you'll
be checking in aarch64_scalar_builtin_types for a string matching
"__Poly8_t" and won't find it, so we'll end up with default mangling for this
type.

One question I have is, if for all the backend types we define we want the
mangled name to be:

  <strlen (type)><type>

then why do we not just return that and save the string comparisons?

I can see some argument for future flexibility, but in that case we will need
to rewrite this code anyway. Is there some other hole in my reasoning?

Thanks,
James