[GCC][PATCH][Aarch64] Add Bfloat16_t scalar type, vector types and machine modes to Aarch64 back-end [2/2]

[Richard Sandiford]

Stam Markianos-Wright <Stam.Markianos-Wright@arm.com> writes:
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index f57469b6e23..f40f6432fd4 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -21661,6 +21661,68 @@ aarch64_stack_protect_guard (void)
>    return NULL_TREE;
>  }
>  
> +/* Return the diagnostic message string if conversion from FROMTYPE to
> +   TOTYPE is not allowed, NULL otherwise.  */
> +
> +static const char *
> +aarch64_invalid_conversion (const_tree fromtype, const_tree totype)
> +{
> +  static char templ[100];
> +  if ((GET_MODE_INNER (TYPE_MODE (fromtype)) == BFmode
> +       || GET_MODE_INNER (TYPE_MODE (totype)) == BFmode)
> +       && TYPE_MODE (fromtype) != TYPE_MODE (totype))
> +  {
> +    snprintf (templ, sizeof (templ), \
> +      "incompatible types when assigning to type '%s' from type '%s'",
> +      IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (totype))),
> +      IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (fromtype))));
> +    return N_(templ);
> +  }
> +  /* Conversion allowed.  */
> +  return NULL;
> +}
> +

This won't handle translation properly.  We also have no guarantee that
the formatted string will fit in 100 characters since at least one of
the type names is unconstrained.  (Also, not all types have names.)

Unfortunately the interface of the current hook doesn't allow for good
diagnostics.  We'll just have to return a fixed string.

Formatting nit: braced block should be indented two spaces more
than the "if (...)".

Same comment for the other hooks.

> +/* Return the diagnostic message string if the unary operation OP is
> +   not permitted on TYPE, NULL otherwise.  */
> +
> +static const char *
> +aarch64_invalid_unary_op (int op, const_tree type)
> +{
> +  static char templ[100];
> +  /* Reject all single-operand operations on BFmode except for &.  */
> +  if (GET_MODE_INNER (TYPE_MODE (type)) == BFmode && op != ADDR_EXPR)
> +  {
> +    snprintf (templ, sizeof (templ),
> +      "operation not permitted on type '%s'",
> +      IDENTIFIER_POINTER (DECL_NAME (TYPE_NAME (type))));
> +    return N_(templ);
> +  }
> +  /* Operation allowed.  */
> +  return NULL;
> +}

The problem with testing TYPE_MODE is that we'll then miss things
that don't have a dedicated mode.  E.g. it'd be interesting to
test what happens for arithmetic on:

  typedef bfloat16_t v16bf __attribute__((vector_size(32)));

Probably better to use element_mode instead.

> diff --git a/gcc/testsuite/gcc.target/aarch64/bfloat16_scalar_typecheck.c b/gcc/testsuite/gcc.target/aarch64/bfloat16_scalar_typecheck.c
> new file mode 100644
> index 00000000000..6f6a6af9587
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/bfloat16_scalar_typecheck.c
> @@ -0,0 +1,83 @@
> +/* { dg-do compile { target { aarch64*-*-* } } } */
> +/* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } } */
> +/* { dg-options "-march=armv8.2-a+i8mm" } */

+bf16 rather than +i8mm.  But using:

/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
/* { dg-add-options arm_v8_2a_bf16_neon }  */

would be better.

> +
> +#include <arm_neon.h>
> +
> +bfloat16_t glob;
> +float is_a_float;
> +int n;
> +
> +bfloat16_t footest (bfloat16_t scalar0)
> +{
> +
> +  /* Initialisation  */
> +
> +  bfloat16_t scalar1 = 0.1; /* { dg-error "incompatible types when assigning to type 'bfloat16_t' from type 'double'" "" {target *-*-*} } */
> +  bfloat16_t scalar2 = 0;   /* { dg-error "incompatible types when assigning to type 'bfloat16_t' from type 'int'" "" {target *-*-*} } */
> +  bfloat16_t scalar3 = {}; /* { dg-error "empty scalar initializer" "" {target *-*-*} } */

Would also be worth testing { scalar0 }, { is_a_float } and { 0.1 }.

(For SVE the tests are divided between sizeless_1.c and gnu_vectors_1.c.
Most of the cases mentioned here are handled in gnu_vectors_1.c instead.)

> +
> +  float16_t initi_a = scalar1; /* { dg-error "incompatible types when assigning to type 'float16_t' from type 'bfloat16_t'" "" {target *-*-*} } */
> +  float16_t initi_b = { scalar1 }; /* { dg-error "incompatible types when assigning to type 'float16_t' from type 'bfloat16_t'" "" {target *-*-*} } */
> +
> +  /* Compound literals.  */
> +
> +  (bfloat16_t) {}; /* { dg-error "empty scalar initializer" "" {target *-*-*} } */
> +  (bfloat16_t) { scalar1 };

Same here.

> +
> +  (int) { scalar1 }; /* { dg-error "incompatible types when assigning to type 'int' from type 'bfloat16_t'" "" {target *-*-*} } */
> +
> +  /* Casting.  */
> +
> +  (void) scalar1;
> +  (bfloat16_t) scalar1;

Would be good to have some tests for invalid cases too.

> +
> +  /* Arrays and Structs.  */
> +
> +  typedef bfloat16_t array_type[2];
> +  extern bfloat16_t extern_array[];
> +
> +  bfloat16_t array[2];
> +  bfloat16_t zero_length_array[0];
> +  bfloat16_t empty_init_array[] = {};
> +  typedef bfloat16_t vla_type[n];
> +
> +  struct struct1 {
> +    bfloat16_t a;
> +  };
> +
> +  union union1 {
> +    bfloat16_t a;
> +  };
> +
> +  /* Assignments.  */
> +
> +  n = scalar1; /* { dg-error "incompatible types when assigning to type 'int' from type 'bfloat16_t'" "" {target *-*-*} } */
> +  is_a_float = scalar1; /* { dg-error "incompatible types when assigning to type 'float' from type 'bfloat16_t'" "" {target *-*-*} } */
> +  scalar1 = 0; /* { dg-error "incompatible types when assigning to type 'bfloat16_t' from type 'int'" "" {target *-*-*} } */
> +  scalar1 = 0.1; /* { dg-error "incompatible types when assigning to type 'bfloat16_t' from type 'double'" "" {target *-*-*} } */
> +  scalar1 = scalar2;

Would be good to test the other way too: "scalar1 = is_a_float",
"scalar1 = n".

> +
> +  /* Addressing and dereferencing.  */
> +
> +  bfloat16_t *bfloat_ptr = &scalar1;
> +  scalar1 = *bfloat_ptr;
> +
> +  /* Pointer assignment.  */
> +
> +  bfloat16_t *bfloat_ptr2 = bfloat_ptr;
> +
> +  /* Single-operand operation.  */
> +
> +  scalar1 = !glob; /* { dg-error "operation not permitted on type 'bfloat16_t'" "" {target *-*-*} } */

Would be good to test "+" and "-" as well -- "!" isn't really typical
for floats.

Other things worth testing for are:

- comparisons
- bfloats used as a condition (e.g. bfloat16 ? a : b)
- bfloats selected via ?:, including cases where the types don't match

> [...]
> diff --git a/gcc/testsuite/gcc.target/aarch64/bfloat16_vector_typecheck1.c b/gcc/testsuite/gcc.target/aarch64/bfloat16_vector_typecheck1.c

Very minor, but local aarch64 style seems to be to use foo_1, foo_2,
etc. rather than foo, foo1, etc., although things aren't very consistent.

Similar comments for these tests as for the scalar ones.

It would be good to have C++ tests too.  An extra thing to test there
is elementwise vector ? vector : vector.

Thanks,
Richard