[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG23a96b84a8d9: [AArch64][SVE] Support implicit lax vector
conversions for SVE types (authored by joechrisellis).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.c
clang/test/Sema/attr-arm-sve-vector-bits.c
clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -270,7 +270,6 @@
TEST_CAST_COMMON(bool)
// Test the implicit conversion only applies to valid types
-fixed_int8_t to_fixed_int8_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_int8_t' (vector of {{[0-9]+}} 'signed char' values)}}
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')}}
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.c
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ fixed_int32_t fi32;
+ svint64_t si64;
+
+ // The implicit cast here should fail if -flax-vector-conversions=none, but pass
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis updated this revision to Diff 305494.
joechrisellis added a comment.
Remove failing test; it was checking that a conversion _failed_, although the
conversion should now _pass_ given the changes in this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.c
clang/test/Sema/attr-arm-sve-vector-bits.c
clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -270,7 +270,6 @@
TEST_CAST_COMMON(bool)
// Test the implicit conversion only applies to valid types
-fixed_int8_t to_fixed_int8_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_int8_t' (vector of {{[0-9]+}} 'signed char' values)}}
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')}}
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.c
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ fixed_int32_t fi32;
+ svint64_t si64;
+
+ // The implicit cast here should fail if -flax-vector-conversions=none, but pass if
+ //
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis updated this revision to Diff 304811.
joechrisellis marked an inline comment as done.
joechrisellis added a comment.
- Support C lax vector conversions.
- Test C lax vector conversions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.c
clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.c
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
}
}
- if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
- S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
- }
+ if (ToType->isSizelessBuiltinType() ||
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
fpetrogalli accepted this revision. fpetrogalli added a comment. This revision is now accepted and ready to land. Thank you @joechrisellis - LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91067/new/ https://reviews.llvm.org/D91067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis updated this revision to Diff 304469.
joechrisellis marked 3 inline comments as done.
joechrisellis added a comment.
Address @fpetrogalli's comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,34 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
}
}
- if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
- S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
- }
+ if (ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType())
+if (S.Context.areCompatibleSveTypes(FromType, ToType) ||
+S.Context.areLaxCompatibleSveTypes(FromType, ToType)) {
+ ICK = ICK_SVE_Vector_Conversion;
+ return true;
+}
// We can perform the conversion between vector types in the following cases:
// 1)vector types are equivalent AltiVec and GCC vector types
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8553,6 +8553,41 @@
IsValidCast(SecondType, FirstType);
}
+bool ASTContext::areLaxCompatibleSveTypes(QualType FirstType,
+ QualType SecondType) {
+ assert(((FirstType->isSizelessBuiltinType() && SecondType->isVectorType()) ||
+ (FirstType->isVectorType() && SecondType->isSizelessBuiltinType())) &&
+ "Expected SVE builtin type and vector type!");
+
+ auto IsLaxCompatible = [this](QualType FirstType, QualType SecondType) {
+if (!FirstType->getAs())
+ return false;
+
+const auto *VecTy = SecondType->getAs();
+if (VecTy &&
+VecTy->getVectorKind() == VectorType::SveFixedLengthDataVector) {
+ 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;
+
+ // If -flax-vector-conversions=integer is specified, the types are
+ // compatible if the elements are integer types.
+ if (LVCKind == LangOptions::LaxVectorConversionKind::Integer)
+return VecTy->getElementType().getCanonicalType()->isIntegerType() &&
+ FirstType->getSveEltType(*this)->isIntegerType();
+}
+
+return false;
+ };
+
+ return IsLaxCompatible(FirstType, SecondType) ||
+ IsLaxCompatible(SecondType, FirstType);
+}
+
bool ASTContext::hasDirectOwnershipQualifier(QualType Ty) const {
while
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis marked an inline comment as done.
joechrisellis added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:8563-8566
+if (const auto *BT = FirstType->getAs()) {
+ if (const auto *VT = SecondType->getAs()) {
+if (VT->getVectorKind() == VectorType::SveFixedLengthDataVector) {
+ const LangOptions::LaxVectorConversionKind LVCKind =
fpetrogalli wrote:
> May I ask to avoid this triple if statement? Given that `BT` is not used
> after being defined, I think the following form would be easier to understand:
>
> ```
> if (!FirstType->getAs())
> return false;
>
> const auto *VT = SecondType->getAs();
>
> if (VT && VT->getVectorKind() == VectorType::SveFixedLengthDataVector) {
>/// ...
> return ...
> }
>
> return false;
> ```
>
> May I ask you to give meaningful names to the variables? BT and VT are quite
> cryptic to me.
>
> Moreover.. are BT and VT really needed? You are asserting
> `FirstType->isSizelessBuiltinType() && SecondType->isVectorType()` ... the
> `getAs` calls should not fail, no? given that the lambda is local to this
> method, I wouldn't bother making it work for the generic case.
Simplified the code as per your suggestion, but the lambda here here serves a
purpose: it makes sure that `areLaxCompatibleSveTypes` has the same behaviour
irrespective of the ordering of the parameters. So we do actually need the if
statements inside the lambda.
Comment at: clang/lib/Sema/SemaCast.cpp:
if (srcIsVector || destIsVector) {
+// Scalable vectors can be cast to and from liberally.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType())
{
fpetrogalli wrote:
> This code path seems untested.
Thinking about it, this could do with being broken out into its own patch. Will
do this.
Comment at: clang/lib/Sema/SemaOverload.cpp:1650-1652
+ ICK = ICK_SVE_Vector_Conversion;
+ return true;
+}
fpetrogalli wrote:
> tabs!
Not sure where these came from, since I ran `clang-format` over the patch.
Think they should be gone now...
Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:19
+
+ // This explicit cast is always allowed, irrespective of the value of
-flax-vector-conversions.
+ ff32 = (fixed_float32_t)sf64;
fpetrogalli wrote:
> Why this one in particular? To me the comment would make more sense if saying
>
> ```
> // An explicit cast is always allowed, irrespective of the value of
> -flax-vector-conversions.
> ```
Will break this out into another patch as mentioned above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
fpetrogalli added a comment.
Hi @joechrisellis - thank you for this patch!
I have left a couple of comments.
Francesco
Comment at: clang/lib/AST/ASTContext.cpp:8563-8566
+if (const auto *BT = FirstType->getAs()) {
+ if (const auto *VT = SecondType->getAs()) {
+if (VT->getVectorKind() == VectorType::SveFixedLengthDataVector) {
+ const LangOptions::LaxVectorConversionKind LVCKind =
May I ask to avoid this triple if statement? Given that `BT` is not used after
being defined, I think the following form would be easier to understand:
```
if (!FirstType->getAs())
return false;
const auto *VT = SecondType->getAs();
if (VT && VT->getVectorKind() == VectorType::SveFixedLengthDataVector) {
/// ...
return ...
}
return false;
```
May I ask you to give meaningful names to the variables? BT and VT are quite
cryptic to me.
Moreover.. are BT and VT really needed? You are asserting
`FirstType->isSizelessBuiltinType() && SecondType->isVectorType()` ... the
`getAs` calls should not fail, no? given that the lambda is local to this
method, I wouldn't bother making it work for the generic case.
Comment at: clang/lib/Sema/SemaCast.cpp:
if (srcIsVector || destIsVector) {
+// Scalable vectors can be cast to and from liberally.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType())
{
This code path seems untested.
Comment at: clang/lib/Sema/SemaOverload.cpp:1650-1652
+ ICK = ICK_SVE_Vector_Conversion;
+ return true;
+}
tabs!
Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:19
+
+ // This explicit cast is always allowed, irrespective of the value of
-flax-vector-conversions.
+ ff32 = (fixed_float32_t)sf64;
Why this one in particular? To me the comment would make more sense if saying
```
// An explicit cast is always allowed, irrespective of the value of
-flax-vector-conversions.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis added inline comments. Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:15 +typedef svint32_t fixed_int32_t FIXED_ATTR; +typedef svint64_t fixed_int64_t FIXED_ATTR; + peterwaller-arm wrote: > I can't see any uses of fixed_float64_t and fixed_int64_t? Good spot! Removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91067/new/ https://reviews.llvm.org/D91067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis updated this revision to Diff 303838.
joechrisellis marked an inline comment as done.
joechrisellis added a comment.
Address @peterwaller-arm's comment regarding unused types.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91067/new/
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,42 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+
+void allowed_always() {
+ fixed_float32_t ff32;
+ svfloat64_t sf64;
+
+ // This explicit cast is always allowed, irrespective of the value of -flax-vector-conversions.
+ ff32 = (fixed_float32_t)sf64;
+}
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
}
}
- if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
- S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
- }
+ if (ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType())
+if (S.Context.areCompatibleSveTypes(FromType, ToType) ||
+S.Context.areLaxCompatibleSveTypes(FromType, ToType)) {
+ ICK = ICK_SVE_Vector_Conversion;
+ return true;
+}
// We can perform the conversion between vector types in the following cases:
// 1)vector types are equivalent AltiVec and GCC vector types
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2219,6 +2219,12 @@
bool destIsVector = DestType->isVectorType();
bool srcIsVector = SrcType->isVectorType();
if (srcIsVector || destIsVector) {
+// Scalable vectors can be cast to and from liberally.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType()) {
+ Kind = CK_BitCast;
+ return TC_Success;
+}
+
// The non-vector type, if any, must have integral type. This is
// the same rule that C vector casts use; note, however, that enum
// types are not integral in C++.
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8553,6 +8553,39 @@
IsValidCast(SecondType, FirstType);
}
+bool ASTContext::areLaxCompatibleSveTypes(QualType FirstType,
+ QualType SecondType) {
+ assert(((FirstType->isSizelessBuiltinType() && SecondType->isVectorType()) ||
+ (FirstType->isVectorType() && SecondType->isSizelessBuiltinType())) &&
+ "Expected SVE builtin type and vector type!");
+
+ auto IsLaxCompatible = [this](QualType FirstType, QualType SecondType) {
+if (const auto *BT = FirstType->getAs()) {
+ if (const auto *VT =
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
peterwaller-arm added a comment. fixed_float64_t appears in the commit message but also is unused. Comment at: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp:15 +typedef svint32_t fixed_int32_t FIXED_ATTR; +typedef svint64_t fixed_int64_t FIXED_ATTR; + I can't see any uses of fixed_float64_t and fixed_int64_t? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91067/new/ https://reviews.llvm.org/D91067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91067: [AArch64][SVE] Support implicit lax vector conversions for SVE types
joechrisellis created this revision.
joechrisellis added reviewers: peterwaller-arm, fpetrogalli, DavidTruby.
Herald added subscribers: cfe-commits, psnobl, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a reviewer: efriedma.
Herald added a project: clang.
joechrisellis requested review of this revision.
Lax vector conversions was behaving incorrectly for implicit casts
between scalable and fixed-length vector types. For example, this:
#include
#define N __ARM_FEATURE_SVE_BITS
#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
typedef svfloat32_t fixed_float32_t FIXED_ATTR;
typedef svfloat64_t fixed_float64_t FIXED_ATTR;
void allowed_depending() {
fixed_float32_t fs32;
svfloat64_t s64;
fs32 = s64;
}
... would fail because the vectors have differing lane sizes. This patch
implements the correct behaviour for
-flax-vector-conversions={none,all,integer}. Specifically:
- -flax-vector-conversions=none prevents all lax vector conversions between
scalable and fixed-sized vectors.
- -flax-vector-conversions=integer allows lax vector conversions between
scalable and fixed-size vectors whose element types are integers.
- -flax-vector-conversions=all allows all lax vector conversions between
scalable and fixed-size vectors (including those with floating point element
types).
The implicit conversions are implemented as bitcasts.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D91067
Files:
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
Index: clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
===
--- /dev/null
+++ clang/test/Sema/aarch64-sve-lax-vector-conversions.cpp
@@ -0,0 +1,44 @@
+// 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
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svfloat32_t fixed_float32_t FIXED_ATTR;
+typedef svfloat64_t fixed_float64_t FIXED_ATTR;
+typedef svint32_t fixed_int32_t FIXED_ATTR;
+typedef svint64_t fixed_int64_t FIXED_ATTR;
+
+void allowed_always() {
+ fixed_float32_t ff32;
+ svfloat64_t sf64;
+
+ // This explicit cast is always allowed, irrespective of the value of -flax-vector-conversions.
+ ff32 = (fixed_float32_t)sf64;
+}
+
+void allowed_with_integer_lax_conversions() {
+ 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}}
+}
+
+void allowed_with_all_lax_conversions() {
+ 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}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1644,11 +1644,12 @@
}
}
- if ((ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType()) &&
- S.Context.areCompatibleSveTypes(FromType, ToType)) {
-ICK = ICK_SVE_Vector_Conversion;
-return true;
- }
+ if (ToType->isSizelessBuiltinType() || FromType->isSizelessBuiltinType())
+if (S.Context.areCompatibleSveTypes(FromType, ToType) ||
+S.Context.areLaxCompatibleSveTypes(FromType, ToType)) {
+ ICK = ICK_SVE_Vector_Conversion;
+ return true;
+}
// We can perform the conversion between vector types in the following cases:
// 1)vector types are equivalent AltiVec and GCC vector types
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2219,6 +2219,12 @@
bool
