[clang][AArch64][SVE] Implicit conversions for vector-scalar operations

[DavidTruby]

Summary

This patch allows the same implicit conversions for vector-scalar
operations in SVE that are allowed for NEON.

Depends on D126377

[c-rhodes]

clang/lib/Sema/SemaChecking.cpp

13532–13554

a lot of this is duplicated by what you've added below

13580

this is the same as Target defined on line 13473

clang/lib/Sema/SemaExpr.cpp

10266

how about:

if (const auto *VT = VectorTy->getAs<VectorType>()) {
  assert(!isa<ExtVectorType>(VT) &&
         "ExtVectorTypes should not be handled here!");
  ...

?

10269–10270

VectorEltTy =
    VectorTy->castAs<BuiltinType>()->getSveEltType(S.getASTContext());

10272

nit: fixed-length

clang/test/CodeGen/aarch64-sve-vector-arith-ops.c

336

took me a moment to realise lit -> literal, maybe add an underscore to make it clearer? I.e. add_i8_i_lit

clang/test/Sema/aarch64-sve-vector-scalar-ops.c

1 ↗

(On Diff #431989)

There's so few tests in here is it not worth add them to clang/test/Sema/aarch64-sve-vector-arith-ops.c?

5 ↗

(On Diff #431989)

what's NEON needed for in this test?

12 ↗

(On Diff #431989)

unused

17 ↗

(On Diff #431989)

there's already a test for this in clang/test/Sema/aarch64-sve-vector-arith-ops.c.

[DavidTruby]

clang/lib/Sema/SemaChecking.cpp

13580

The control flow in this function is a little odd, but it isn't necessarily the same target; we could have entered one or more of the if statements above that modify the target, and we need to inspect the original target here.

[DavidTruby]

Clean up code based on review comments
Move additional tests to existing file

[c-rhodes]

@DavidTruby thanks for updating, just one last comment otherwise LGTM

clang/lib/Sema/SemaChecking.cpp

13580

The control flow in this function is a little odd, but it isn't necessarily the same target; we could have entered one or more of the if statements above that modify the target, and we need to inspect the original target here.

Ah ofc, I missed that, thanks for clarifying.

13589–13593

can a test be added for this?

[DavidTruby]

Remove redundant code

clang/lib/Sema/SemaChecking.cpp

13589–13593

It appears this code doesn't actually do anything as this condition is covered elsewhere (and tested below) so I've just removed it

[c-rhodes]

clang/lib/Sema/SemaChecking.cpp

13589–13593

It appears this code doesn't actually do anything as this condition is covered elsewhere (and tested below) so I've just removed it

Great! Thanks for investigating, can this code also be removed from the generic vector version this was copied from then? Just curious I won't let that hold up this patch.

[DavidTruby]

clang/lib/Sema/SemaChecking.cpp

13589–13593

I don't think so, I believe the reason this isn't necessary here is that vector -> scalar casts are forbidden for SVE vectors but are allowed in specific cases for fixed width vectors, this warning is here for cases where it's allowed but nonsensical. That's at least my understanding

clang/lib/Sema/SemaChecking.cpp

static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
 
   // Check implicit casts from Objective-C collection literals to specialized
   // collection types, e.g., NSArray<NSString *> *.
   if (auto *ArrayLiteral = dyn_cast<ObjCArrayLiteral>(E))
     checkObjCArrayLiteral(S, QualType(Target, 0), ArrayLiteral);
   else if (auto *DictionaryLiteral = dyn_cast<ObjCDictionaryLiteral>(E))
     checkObjCDictionaryLiteral(S, QualType(Target, 0), DictionaryLiteral);
 
   // Strip vector types.
   if (isa<VectorType>(Source)) {
     if (Target->isVLSTBuiltinType() &&
         (S.Context.areCompatibleSveTypes(QualType(Target, 0),
                                          QualType(Source, 0)) ||
          S.Context.areLaxCompatibleSveTypes(QualType(Target, 0),
                                             QualType(Source, 0))))
       return;
 
     if (!isa<VectorType>(Target)) {
       if (S.SourceMgr.isInSystemMacro(CC))
         return;
       return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
     }
 
     // If the vector cast is cast between two vectors of the same size, it is
     // a bitcast, not a conversion.
     if (S.Context.getTypeSize(Source) == S.Context.getTypeSize(Target))
       return;
 
     Source = cast<VectorType>(Source)->getElementType().getTypePtr();
     Target = cast<VectorType>(Target)->getElementType().getTypePtr();
   }
   if (auto VecTy = dyn_cast<VectorType>(Target))
     Target = VecTy->getElementType().getTypePtr();
 
   // Strip complex types.
   if (isa<ComplexType>(Source)) {
     if (!isa<ComplexType>(Target)) {
       if (S.SourceMgr.isInSystemMacro(CC) || Target->isBooleanType())
         return;
 
       return DiagnoseImpCast(S, E, T, CC,
                              S.getLangOpts().CPlusPlus
                                  ? diag::err_impcast_complex_scalar
                                  : diag::warn_impcast_complex_scalar);
     }
 
     Source = cast<ComplexType>(Source)->getElementType().getTypePtr();
     Target = cast<ComplexType>(Target)->getElementType().getTypePtr();
   }
 
   const BuiltinType *SourceBT = dyn_cast<BuiltinType>(Source);
   const BuiltinType *TargetBT = dyn_cast<BuiltinType>(Target);
 
+  // Strip SVE vector types
+  if (SourceBT && SourceBT->isVLSTBuiltinType()) {
+    // Need the original target type for vector type checks
+    const Type *OriginalTarget = S.Context.getCanonicalType(T).getTypePtr();
+    // Handle conversion from scalable to fixed when msve-vector-bits is
+    // specified
+    if (S.Context.areCompatibleSveTypes(QualType(OriginalTarget, 0),
+                                        QualType(Source, 0)) ||
+        S.Context.areLaxCompatibleSveTypes(QualType(OriginalTarget, 0),
+                                           QualType(Source, 0)))
+      return;
+
+    // If the vector cast is cast between two vectors of the same size, it is
+    // a bitcast, not a conversion.
+    if (S.Context.getTypeSize(Source) == S.Context.getTypeSize(Target))
+      return;
+
+    Source = SourceBT->getSveEltType(S.Context).getTypePtr();
+  }
+
+  if (TargetBT && TargetBT->isVLSTBuiltinType())
+    Target = TargetBT->getSveEltType(S.Context).getTypePtr();
+
   // If the source is floating point...
   if (SourceBT && SourceBT->isFloatingPoint()) {
     // ...and the target is floating point...
     if (TargetBT && TargetBT->isFloatingPoint()) {
       // ...then warn if we're dropping FP rank.
 
       int Order = S.getASTContext().getFloatingTypeSemanticOrder(
           QualType(SourceBT, 0), QualType(TargetBT, 0));

clang/lib/Sema/SemaExpr.cpp

 /// Attempt to convert and splat Scalar into a vector whose types matches
 /// Vector following GCC conversion rules. The rule is that implicit
 /// conversion can occur when Scalar can be casted to match Vector's element
 /// type without causing truncation of Scalar.
 static bool tryGCCVectorConvertAndSplat(Sema &S, ExprResult *Scalar,
                                         ExprResult *Vector) {
   QualType ScalarTy = Scalar->get()->getType().getUnqualifiedType();
   QualType VectorTy = Vector->get()->getType().getUnqualifiedType();
-  const auto *VT = VectorTy->castAs<VectorType>();
+  QualType VectorEltTy;
 
+  if (const auto *VT = VectorTy->getAs<VectorType>()) {
     assert(!isa<ExtVectorType>(VT) &&
            "ExtVectorTypes should not be handled here!");
-
-  QualType VectorEltTy = VT->getElementType();
+    VectorEltTy = VT->getElementType();
+  } else if (VectorTy->isVLSTBuiltinType()) {
+    VectorEltTy =
+        VectorTy->castAs<BuiltinType>()->getSveEltType(S.getASTContext());
+  } else {
+    llvm_unreachable("Only Fixed-Length and SVE Vector types are handled here");
+  }
 
   // Reject cases where the vector element type or the scalar element type are
   // not integral or floating point types.
   if (!VectorEltTy->isArithmeticType() || !ScalarTy->isArithmeticType())
     return true;
 
   // The conversion to apply to the scalar before splatting it,
   // if necessary.
if ((OperationKind == ACK_Arithmetic) &&
     Diag(Loc, DiagID) << LHSType << RHSType << LHS.get()->getSourceRange()
                       << RHS.get()->getSourceRange();
     return QualType();
   }
 
   if (Context.hasSameType(LHSType, RHSType))
     return LHSType;
 
-  auto tryScalableVectorConvert = [this](ExprResult *Src, QualType SrcType,
-                                         QualType DestType) {
-    const QualType DestBaseType = DestType->getSveEltType(Context);
-    if (DestBaseType->getUnqualifiedDesugaredType() ==
-        SrcType->getUnqualifiedDesugaredType()) {
-      unsigned DiagID = diag::err_typecheck_invalid_operands;
-      if (!tryVectorConvertAndSplat(*this, Src, SrcType, DestBaseType, DestType,
-                                    DiagID))
-        return DestType;
-    }
-    return QualType();
-  };
-
   if (LHSType->isVLSTBuiltinType() && !RHSType->isVLSTBuiltinType()) {
-    auto DestType = tryScalableVectorConvert(&RHS, RHSType, LHSType);
-    if (DestType == QualType())
-      return InvalidOperands(Loc, LHS, RHS);
-    return DestType;
+    if (!tryGCCVectorConvertAndSplat(*this, &RHS, &LHS))
+      return LHSType;
+  }
+  if (RHSType->isVLSTBuiltinType() && !LHSType->isVLSTBuiltinType()) {
+    if (LHS.get()->isLValue() ||
+        !tryGCCVectorConvertAndSplat(*this, &LHS, &RHS))
+      return RHSType;
   }
 
   if ((!LHSType->isVLSTBuiltinType() && !LHSType->isRealType()) ||
       (!RHSType->isVLSTBuiltinType() && !RHSType->isRealType())) {
     Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar)
         << LHSType << RHSType << LHS.get()->getSourceRange()
         << RHS.get()->getSourceRange();
     return QualType();

clang/test/CodeGen/aarch64-sve-vector-arith-ops.c

 // CHECK-NEXT:    [[SPLAT_SPLAT:%.*]] = shufflevector <vscale x 2 x double> [[SPLAT_SPLATINSERT]], <vscale x 2 x double> poison, <vscale x 2 x i32> zeroinitializer
 // CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], [[SPLAT_SPLAT]]
 // CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
 //
 svfloat64_t add_scalar_f64(svfloat64_t a, double b) {
   return a + b;
 }
 
+// CHECK-LABEL: @add_i8_i_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_i_lit(svint8_t a) {
+  return a + 0;
+}
+
+// CHECK-LABEL: @add_i8_il_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_il_lit(svint8_t a) {
+  return a + 0l;
+}
+
+// CHECK-LABEL: @add_i8_ill_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_ill_lit(svint8_t a) {
+  return a + 0ll;
+}
+
+// CHECK-LABEL: @add_i8_u_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_u_lit(svint8_t a) {
+  return a + 0u;
+}
+
+// CHECK-LABEL: @add_i8_ul_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_ul_lit(svint8_t a) {
+  return a + 0ul;
+}
+
+// CHECK-LABEL: @add_i8_ull_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = add <vscale x 16 x i8> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 16 x i8> [[ADD]]
+//
+svint8_t add_i8_ull_lit(svint8_t a) {
+  return a + 0ull;
+}
+
+// CHECK-LABEL: @add_f64_i_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_i_lit(svfloat64_t a) {
+  return a + 0;
+}
+
+// CHECK-LABEL: @add_f64_il_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_il_lit(svfloat64_t a) {
+  return a + 0l;
+}
+
+// CHECK-LABEL: @add_f64_ill_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_ill_lit(svfloat64_t a) {
+  return a + 0ll;
+}
+
+// CHECK-LABEL: @add_f64_u_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_u_lit(svfloat64_t a) {
+  return a + 0u;
+}
+
+// CHECK-LABEL: @add_f64_ul_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_ul_lit(svfloat64_t a) {
+  return a + 0ul;
+}
+
+// CHECK-LABEL: @add_f64_ull_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_ull_lit(svfloat64_t a) {
+  return a + 0ull;
+}
+
+// CHECK-LABEL: @add_f64_f_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_f_lit(svfloat64_t a) {
+  return a + 0.f;
+}
+
+// CHECK-LABEL: @add_f64_d_lit(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[ADD:%.*]] = fadd <vscale x 2 x double> [[A:%.*]], zeroinitializer
+// CHECK-NEXT:    ret <vscale x 2 x double> [[ADD]]
+//
+svfloat64_t add_f64_d_lit(svfloat64_t a) {
+  return a + 0.;
+}
+
 // SUBTRACTION
 
 // CHECK-LABEL: @sub_i8(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:    [[SUB:%.*]] = sub <vscale x 16 x i8> [[A:%.*]], [[B:%.*]]
 // CHECK-NEXT:    ret <vscale x 16 x i8> [[SUB]]
 //
 svint8_t sub_i8(svint8_t a, svint8_t b) {

clang/test/Sema/aarch64-sve-vector-arith-ops.c

void mod(svint8_t i8, svint16_t i16, svint32_t i32, svint64_t i64,
   (void)(s % i32); // expected-error{{invalid operands to binary expression}}
   (void)(s % i64); // expected-error{{invalid operands to binary expression}}
   (void)(s % u8);  // expected-error{{invalid operands to binary expression}}
   (void)(s % u16); // expected-error{{invalid operands to binary expression}}
   (void)(s % u32); // expected-error{{invalid operands to binary expression}}
   (void)(s % f16); // expected-error{{invalid operands to binary expression}}
   (void)(s % f32); // expected-error{{invalid operands to binary expression}}
 }
+
+svint8_t svi8(svint8_t a) {
+  return a + 256; // expected-error{{cannot convert between scalar type 'int' and vector type 'svint8_t' (aka '__SVInt8_t') as implicit conversion would cause truncation}}
+}
+
+svint8_t svi8_128(svint8_t a) {
+  return a + 128; // expected-warning{{implicit conversion from 'int' to 'svint8_t' (aka '__SVInt8_t') changes value from 128 to -128}}
+}

clang/test/Sema/sizeless-1.c

void func(int sel) {
   !init_int8;       // expected-error {{invalid argument type 'svint8_t'}}
   *init_int8;       // expected-error {{indirection requires pointer operand}}
   __real init_int8; // expected-error {{invalid type 'svint8_t'}}
   __imag init_int8; // expected-error {{invalid type 'svint8_t'}}
 
   local_int8 &&init_int8;  // expected-error {{invalid operands to binary expression}}
   local_int8 || init_int8; // expected-error {{invalid operands to binary expression}}
 
-  local_int8 + 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 - 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 * 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 / 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 % 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 & 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 | 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 ^ 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 < 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 <= 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 == 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 != 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 >= 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 > 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 && 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 || 0; // expected-error {{invalid operands to binary expression}}
-
   if (local_int8) { // expected-error {{statement requires expression of scalar type}}
   }
   while (local_int8) { // expected-error {{statement requires expression of scalar type}}
   }
   do { // expected-error {{statement requires expression of scalar type}}
   } while (local_int8);
   switch (local_int8) { // expected-error {{statement requires expression of integer type}}
   default:;

clang/test/SemaCXX/sizeless-1.cpp

#endif
   !init_int8;       // expected-error {{invalid argument type 'svint8_t'}}
   *init_int8;       // expected-error {{indirection requires pointer operand}}
   __real init_int8; // expected-error {{invalid type 'svint8_t'}}
   __imag init_int8; // expected-error {{invalid type 'svint8_t'}}
 
   local_int8 &&init_int8;  // expected-error {{invalid operands to binary expression}} expected-error {{not contextually convertible}}
   local_int8 || init_int8; // expected-error {{invalid operands to binary expression}} expected-error {{not contextually convertible}}
 
-  local_int8 + 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 - 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 * 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 / 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 % 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 & 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 | 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 ^ 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 < 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 <= 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 == 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 != 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 >= 0; // expected-error {{invalid operands to binary expression}}
-  local_int8 > 0;  // expected-error {{invalid operands to binary expression}}
-  local_int8 && 0; // expected-error {{invalid operands to binary expression}} expected-error {{not contextually convertible}}
-  local_int8 || 0; // expected-error {{invalid operands to binary expression}} expected-error {{not contextually convertible}}
-
   if (local_int8) { // expected-error {{not contextually convertible to 'bool'}}
   }
   while (local_int8) { // expected-error {{not contextually convertible to 'bool'}}
   }
   do {
   } while (local_int8); // expected-error {{not contextually convertible to 'bool'}}
   switch (local_int8) { // expected-error {{statement requires expression of integer type}}
   default:;