[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 ↗

(On Diff #306043)

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 ↗

(On Diff #306043)

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 -flax-vector-conversions=all is specified, the types are
       // certainly compatible.
       if (LVCKind == LangOptions::LaxVectorConversionKind::All)
         return true;
 

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/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}}
 }