[AArch64][SVE] Allow lax conversion between VLATs and GNU vectors

[joechrisellis]

Summary

Previously, lax conversions were only allowed between SVE vector-length
agnostic types and vector-length specific types. This meant that code
such as the following:

#include <arm_sve.h>
#define N __ARM_FEATURE_SVE_BITS
#define FIXED_ATTR __attribute__ ((vector_size (N/8)))
typedef float fixed_float32_t FIXED_ATTR;

void foo() {
    fixed_float32_t fs32;
    svfloat64_t s64;
    fs32 = s64;
}

was not allowed.

This patch makes a minor change to areLaxCompatibleSveTypes to allow for
lax conversions to be performed between SVE vector-length agnostic types
and GNU vectors.

[willlovett]

I found this one. The fix looks sensible, and I believe the code being tested is consistent with the spec, but I'll leave it to someone else to accept (once the tests are passing)

[joechrisellis]

Remove redundant tests from clang/test/Sema/attr-arm-sve-vector-bits.c.

[fpetrogalli]

This LGTM. May I ask to extend the commit message to add a reference to the paragraph in section "3.7.3.3", item 2 on page 23 of the specs version 00bet6?

[c-rhodes]

clang/test/Sema/attr-arm-sve-vector-bits.c

278–283

I don't think this can be removed. The ACLE states "Whenever __ARM_FEATURE_SVE_BITS==N, GNUT implicitly converts to VLAT and VLAT implicitly converts to GNUT.".

AFAIK lax vector conversions only apply to vectors of the same width, with GNU vectors for example the following is invalid regardless of lax vector conversions:

typedef int8_t int8x16_t __attribute__((vector_size(16)));
typedef int8_t int8x64_t __attribute__((vector_size(64)));

int8x16_t foo(int8x64_t x) { return x; }

[joechrisellis]

Address @c-rhodes's comments regarding lax conversion when __ARM_FEATURE_SVE_BITS != N for GNU vectors.

clang/test/Sema/attr-arm-sve-vector-bits.c

278–283

Great spot, didn't see that in the spec. I've re-added the test and added an extra condition to check if __ARM_FEATURE_SVE_BITS == N before allowing the lax conversion for GNU vectors. Not sure if this is the best way to do it, though. :)

[c-rhodes]

In D91696#2405082, @joechrisellis wrote:

Address @c-rhodes's comments regarding lax conversion when __ARM_FEATURE_SVE_BITS != N for GNU vectors.

Cheers, LGTM

clang/lib/AST/ASTContext.cpp

assert(((FirstType->isSizelessBuiltinType() && SecondType->isVectorType()) ||
          "Expected SVE builtin type and vector type!");
 
   auto IsLaxCompatible = [this](QualType FirstType, QualType SecondType) {
     if (!FirstType->getAs<BuiltinType>())
       return false;
 
     const auto *VecTy = SecondType->getAs<VectorType>();
     if (VecTy &&
-        VecTy->getVectorKind() == VectorType::SveFixedLengthDataVector) {
+        (VecTy->getVectorKind() == VectorType::SveFixedLengthDataVector ||
+         VecTy->getVectorKind() == VectorType::GenericVector)) {
       const LangOptions::LaxVectorConversionKind LVCKind =
           getLangOpts().getLaxVectorConversions();
 
+      // If __ARM_FEATURE_SVE_BITS != N do not allow GNU vector lax conversion.
+      // "Whenever __ARM_FEATURE_SVE_BITS==N, GNUT implicitly
+      // converts to VLAT and VLAT implicitly converts to GNUT."
+      // ACLE Spec Version 00bet6, 3.7.3.2. Behavior common to vectors and
+      // predicates.
+      if (VecTy->getVectorKind() == VectorType::GenericVector &&
+          getTypeSize(SecondType) != getLangOpts().ArmSveVectorBits)
+        return false;
+
       // If -flax-vector-conversions=all is specified, the types are
       // certainly compatible.
       if (LVCKind == LangOptions::LaxVectorConversionKind::All)
         return true;
 
       // If -flax-vector-conversions=integer is specified, the types are
       // compatible if the elements are integer types.
       if (LVCKind == LangOptions::LaxVectorConversionKind::Integer)

clang/test/Sema/aarch64-sve-lax-vector-conversions.c

 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=none -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-none %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=integer -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-integer %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=all -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-all %s
 
 // lax-vector-all-no-diagnostics
 
 #include <arm_sve.h>
 
 #define N __ARM_FEATURE_SVE_BITS
-#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define SVE_FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define GNU_FIXED_ATTR __attribute__((vector_size(N / 8)))
 
-typedef svfloat32_t fixed_float32_t FIXED_ATTR;
-typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svfloat32_t sve_fixed_float32_t SVE_FIXED_ATTR;
+typedef svint32_t sve_fixed_int32_t SVE_FIXED_ATTR;
+typedef float gnu_fixed_float32_t GNU_FIXED_ATTR;
+typedef int gnu_fixed_int32_t GNU_FIXED_ATTR;
 
-void allowed_with_integer_lax_conversions() {
-  fixed_int32_t fi32;
+void sve_allowed_with_integer_lax_conversions() {
+  sve_fixed_int32_t fi32;
   svint64_t si64;
 
   // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
   // -flax-vector-conversions={integer,all}.
   fi32 = si64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
   si64 = fi32;
   // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
 }
 
-void allowed_with_all_lax_conversions() {
-  fixed_float32_t ff32;
+void sve_allowed_with_all_lax_conversions() {
+  sve_fixed_float32_t ff32;
   svfloat64_t sf64;
 
   // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
   // -flax-vector-conversions=all.
   ff32 = sf64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
-  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  sf64 = ff32;
+  // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_integer_lax_conversions() {
+  gnu_fixed_int32_t fi32;
+  svint64_t si64;
+
+  // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+  // -flax-vector-conversions={integer,all}.
+  fi32 = si64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  si64 = fi32;
+  // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_all_lax_conversions() {
+  gnu_fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
+  // -flax-vector-conversions=all.
+  ff32 = sf64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
   sf64 = ff32;
   // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
   // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
 }

clang/test/Sema/attr-arm-sve-vector-bits.c

 TEST_CAST_VECTOR(float32)
 TEST_CAST_VECTOR(float64)
 TEST_CAST_VECTOR(bfloat16)
 TEST_CAST_COMMON(bool)
 
 // Test the implicit conversion only applies to valid types
 fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error-re {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (vector of {{[0-9]+}} 'unsigned char' values)}}
 
-svint64_t to_svint64_t__from_gnu_int32_t(gnu_int32_t x) { return x; } // expected-error-re {{returning 'gnu_int32_t' (vector of {{[0-9]+}} 'int32_t' values) from a function with incompatible result type 'svint64_t' (aka '__SVInt64_t')}}
-gnu_int32_t from_svint64_t__to_gnu_int32_t(svint64_t x) { return x; } // expected-error-re {{returning 'svint64_t' (aka '__SVInt64_t') from a function with incompatible result type 'gnu_int32_t' (vector of {{[0-9]+}} 'int32_t' values)}}
-
 // Test implicit conversion between SVE and GNU vector is invalid when
 // __ARM_FEATURE_SVE_BITS != N
 #if defined(__ARM_FEATURE_SVE_BITS) && __ARM_FEATURE_SVE_BITS == 512
 typedef int32_t int4 __attribute__((vector_size(16)));
 svint32_t badcast(int4 x) { return x; } // expected-error {{returning 'int4' (vector of 4 'int32_t' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 #endif
 
 // Test conversion between predicate and uint8 is invalid, both have the same
 // memory representation.
 fixed_bool_t to_fixed_bool_t__from_svuint8_t(svuint8_t x) { return x; } // expected-error-re {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_bool_t' (vector of {{[0-9]+}} 'unsigned char' values)}}
 
 // --------------------------------------------------------------------------//
 // Test the scalable and fixed-length types can be used interchangeably
 

clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp

 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=none -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-none %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=integer -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-integer %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=all -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify=lax-vector-all %s
 
 // lax-vector-all-no-diagnostics
 
 #include <arm_sve.h>
 
 #define N __ARM_FEATURE_SVE_BITS
-#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define SVE_FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+#define GNU_FIXED_ATTR __attribute__((vector_size(N / 8)))
 
-typedef svfloat32_t fixed_float32_t FIXED_ATTR;
-typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svfloat32_t sve_fixed_float32_t SVE_FIXED_ATTR;
+typedef svint32_t sve_fixed_int32_t SVE_FIXED_ATTR;
+typedef float gnu_fixed_float32_t GNU_FIXED_ATTR;
+typedef int gnu_fixed_int32_t GNU_FIXED_ATTR;
 
-void allowed_with_integer_lax_conversions() {
-  fixed_int32_t fi32;
+void sve_allowed_with_integer_lax_conversions() {
+  sve_fixed_int32_t fi32;
   svint64_t si64;
 
   // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
   // -flax-vector-conversions={integer,all}.
   fi32 = si64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
   si64 = fi32;
   // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
 }
 
-void allowed_with_all_lax_conversions() {
-  fixed_float32_t ff32;
+void sve_allowed_with_all_lax_conversions() {
+  sve_fixed_float32_t ff32;
   svfloat64_t sf64;
 
   // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
   // -flax-vector-conversions=all.
   ff32 = sf64;
-  // lax-vector-none-error@-1 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
-  // lax-vector-integer-error@-2 {{assigning to 'fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-none-error@-1 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'sve_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  sf64 = ff32;
+  // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_integer_lax_conversions() {
+  gnu_fixed_int32_t fi32;
+  svint64_t si64;
+
+  // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+  // -flax-vector-conversions={integer,all}.
+  fi32 = si64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_int32_t' (vector of 16 'int' values) from incompatible type}}
+  si64 = fi32;
+  // lax-vector-none-error@-1 {{assigning to 'svint64_t' (aka '__SVInt64_t') from incompatible type}}
+}
+
+void gnu_allowed_with_all_lax_conversions() {
+  gnu_fixed_float32_t ff32;
+  svfloat64_t sf64;
+
+  // The implicit cast here should fail if -flax-vector-conversions={none,integer}, but pass if
+  // -flax-vector-conversions=all.
+  ff32 = sf64;
+  // lax-vector-none-error@-1 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
+  // lax-vector-integer-error@-2 {{assigning to 'gnu_fixed_float32_t' (vector of 16 'float' values) from incompatible type}}
   sf64 = ff32;
   // lax-vector-none-error@-1 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
   // lax-vector-integer-error@-2 {{assigning to 'svfloat64_t' (aka '__SVFloat64_t') from incompatible type}}
 }