[PATCH] D155495: [RISCV][AArch64][IRGen] Add a special case to CodeGenFunction::EmitCall for scalable vector return being coerced to fixed vector.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

Thanks Craig this is a better solution, LLVM cleans up the store/load coercion 
nicely but better not to create them in the first place if possible. LGTM cheers




Comment at: clang/lib/CodeGen/CGCall.cpp:5749
+  // intrinsic to perform the conversion.
+  if (auto *FixedDst = 
dyn_cast(ConvertType(RetTy))) {
+llvm::Value *V = CI;

nit: RetIRTy


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155495/new/

https://reviews.llvm.org/D155495

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155222: [RISCV][AArch64][IRGen] Add scalable->fixed as a special case in CreateCoercedStore.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

It's been a while since I've looked at this code, what about this wouldnt work 
for predicates? I seem to recall fixed predicates using i8 vs i1 for scalable, 
is that issue? Happy with this regardless, just curious.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155222/new/

https://reviews.llvm.org/D155222

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155222: [RISCV][AArch64][IRGen] Add scalable->fixed as a special case in CreateCoercedStore.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM cheers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155222/new/

https://reviews.llvm.org/D155222

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155220: [IRGen] Remove 'Sve' from the name of some IR names that are shared with RISC-V now.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM cheers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155220/new/

https://reviews.llvm.org/D155220

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144613: [RISCV] Properly diagnose mixing RVV scalable vectors with GNU vectors.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

One minor nit but otherwise LGTM




Comment at: clang/lib/Sema/SemaExpr.cpp:10769-10770
 
   // Expressions containing GNU and SVE (fixed or sizeless) vectors are invalid
   // since the ambiguity can affect the ABI.
+  auto IsSveRVVGnuConversion = [](QualType FirstType, QualType SecondType,

nit: update comment?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144613/new/

https://reviews.llvm.org/D144613

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130984: [clang][AArch64][SVE] Add unary +/- operators for SVE types

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM, just one minor comment




Comment at: clang/test/CodeGen/aarch64-sve-vector-arith-ops.c:1654
+
+// UNARY PROMOTION
+

should we add FP tests as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130984/new/

https://reviews.llvm.org/D130984

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129476: [AArch64][SVE] Prefer SIMD variant of clast[ab]

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes closed this revision.
c-rhodes marked an inline comment as done.
c-rhodes added a comment.

In D129476#3645667 , @efriedma wrote:

> Please update the comment, then LGTM

Done, cheers Eli.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129476/new/

https://reviews.llvm.org/D129476

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129476: [AArch64][SVE] Prefer SIMD variant of clast[ab]

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D129476#3643382 , @efriedma wrote:

> Are you saying that it's faster to use clasta targeting a float register, 
> then move the result to an integer register, rather than use the integer form 
> directly?  Or is the issue just that we want to split the operations in case 
> we can simplify the resulting bitcast?

The former, for the most part. If clast[ab] is inside a loop and is a 
loop-carried dependency it's considerably faster. If it's a straight 
bitcast-to-fp + clast[ab] + bitcast-to-int then that costs a cycle or two more 
depending on the micro-architecture, but in slightly more complicated code from 
what I've observed the SIMD with bitcasts is as fast as the integer variant, 
if not faster.

> Do we expect SelectionDAG to combine a clasta+bitcast to a single 
> instruction?  Do we have test coverage for that?

No, there'll be a mov to an integer register.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129476/new/

https://reviews.llvm.org/D129476

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129476: [AArch64][SVE] Prefer SIMD variant of clast[ab]

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 443594.
c-rhodes added a comment.

Add full patch context.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129476/new/

https://reviews.llvm.org/D129476

Files:
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll

Index: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll
@@ -0,0 +1,44 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=instcombine -S < %s | FileCheck %s
+
+target triple = "aarch64"
+
+define i16 @clastb_n_i16( %pg, i16 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i16(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i16 [[A:%.*]] to half
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call half @llvm.aarch64.sve.clastb.n.nxv8f16( [[PG:%.*]], half [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast half [[TMP3]] to i16
+; CHECK-NEXT:ret i16 [[TMP4]]
+;
+  %out = call i16 @llvm.aarch64.sve.clastb.n.nxv8i16( %pg, i16 %a,  %b)
+  ret i16 %out
+}
+
+define i32 @clastb_n_i32( %pg, i32 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i32(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i32 [[A:%.*]] to float
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call float @llvm.aarch64.sve.clastb.n.nxv4f32( [[PG:%.*]], float [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast float [[TMP3]] to i32
+; CHECK-NEXT:ret i32 [[TMP4]]
+;
+  %out = call i32 @llvm.aarch64.sve.clastb.n.nxv4i32( %pg, i32 %a,  %b)
+  ret i32 %out
+}
+
+define i64 @clastb_n_i64( %pg, i64 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i64(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i64 [[A:%.*]] to double
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call double @llvm.aarch64.sve.clastb.n.nxv2f64( [[PG:%.*]], double [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast double [[TMP3]] to i64
+; CHECK-NEXT:ret i64 [[TMP4]]
+;
+  %out = call i64 @llvm.aarch64.sve.clastb.n.nxv2i64( %pg, i64 %a,  %b)
+  ret i64 %out
+}
+
+declare i16 @llvm.aarch64.sve.clastb.n.nxv8i16(, i16, )
+declare i32 @llvm.aarch64.sve.clastb.n.nxv4i32(, i32, )
+declare i64 @llvm.aarch64.sve.clastb.n.nxv2i64(, i64, )
Index: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -796,6 +796,44 @@
   return IC.replaceInstUsesWith(II, Extract);
 }
 
+static Optional instCombineSVECondLast(InstCombiner ,
+  IntrinsicInst ) {
+  // Replace scalar integer CLAST[AB] intrinsic with optimal SIMD variant.
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint();
+  Value *Pg = II.getArgOperand(0);
+  Value *Fallback = II.getArgOperand(1);
+  Value *Vec = II.getArgOperand(2);
+  Type *Ty = II.getType();
+
+  if (!Ty->isIntegerTy())
+return None;
+
+  Type *FPTy;
+  switch (cast(Ty)->getBitWidth()) {
+  default:
+return None;
+  case 16:
+FPTy = Builder.getHalfTy();
+break;
+  case 32:
+FPTy = Builder.getFloatTy();
+break;
+  case 64:
+FPTy = Builder.getDoubleTy();
+break;
+  }
+
+  Value *FPFallBack = Builder.CreateBitCast(Fallback, FPTy);
+  auto *FPVTy = VectorType::get(
+  FPTy, cast(Vec->getType())->getElementCount());
+  Value *FPVec = Builder.CreateBitCast(Vec, FPVTy);
+  auto *FPII = Builder.CreateIntrinsic(II.getIntrinsicID(), {FPVec->getType()},
+   {Pg, FPFallBack, FPVec});
+  Value *FPIItoInt = Builder.CreateBitCast(FPII, II.getType());
+  return IC.replaceInstUsesWith(II, FPIItoInt);
+}
+
 static Optional instCombineRDFFR(InstCombiner ,
 IntrinsicInst ) {
   LLVMContext  = II.getContext();
@@ -1294,6 +1332,9 @@
   case Intrinsic::aarch64_sve_lasta:
   case Intrinsic::aarch64_sve_lastb:
 return instCombineSVELast(IC, II);
+  case Intrinsic::aarch64_sve_clasta_n:
+  case Intrinsic::aarch64_sve_clastb_n:
+return instCombineSVECondLast(IC, II);
   case Intrinsic::aarch64_sve_cntd:
 return instCombineSVECntElts(IC, II, 2);
   case Intrinsic::aarch64_sve_cntw:
Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
===
--- clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
@@ -215,14 +215,20 @@
 // CHECK-LABEL: @test_svclastb_n_s16(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:   

[PATCH] D129476: [AArch64][SVE] Prefer SIMD variant of clast[ab]

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: paulwalker-arm, bsmith, peterwaller-arm, DavidTruby.
Herald added subscribers: psnobl, hiraditya, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: All.
c-rhodes requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

The scalar variant with GPR source/dest has considerably higher latency
than the SIMD scalar variant across a variety of micro-architectures:

  Core   ScalarSIMD
  
  Neoverse V1 9 cyc  3 cyc
  Neoverse N2 8 cyc  3 cyc
  Cortex A510 8 cyc  4 cyc


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129476

Files:
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll

Index: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-clast.ll
@@ -0,0 +1,44 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=instcombine -S < %s | FileCheck %s
+
+target triple = "aarch64"
+
+define i16 @clastb_n_i16( %pg, i16 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i16(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i16 [[A:%.*]] to half
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call half @llvm.aarch64.sve.clastb.n.nxv8f16( [[PG:%.*]], half [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast half [[TMP3]] to i16
+; CHECK-NEXT:ret i16 [[TMP4]]
+;
+  %out = call i16 @llvm.aarch64.sve.clastb.n.nxv8i16( %pg, i16 %a,  %b)
+  ret i16 %out
+}
+
+define i32 @clastb_n_i32( %pg, i32 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i32(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i32 [[A:%.*]] to float
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call float @llvm.aarch64.sve.clastb.n.nxv4f32( [[PG:%.*]], float [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast float [[TMP3]] to i32
+; CHECK-NEXT:ret i32 [[TMP4]]
+;
+  %out = call i32 @llvm.aarch64.sve.clastb.n.nxv4i32( %pg, i32 %a,  %b)
+  ret i32 %out
+}
+
+define i64 @clastb_n_i64( %pg, i64 %a,  %b) {
+; CHECK-LABEL: @clastb_n_i64(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i64 [[A:%.*]] to double
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast  [[B:%.*]] to 
+; CHECK-NEXT:[[TMP3:%.*]] = call double @llvm.aarch64.sve.clastb.n.nxv2f64( [[PG:%.*]], double [[TMP1]],  [[TMP2]])
+; CHECK-NEXT:[[TMP4:%.*]] = bitcast double [[TMP3]] to i64
+; CHECK-NEXT:ret i64 [[TMP4]]
+;
+  %out = call i64 @llvm.aarch64.sve.clastb.n.nxv2i64( %pg, i64 %a,  %b)
+  ret i64 %out
+}
+
+declare i16 @llvm.aarch64.sve.clastb.n.nxv8i16(, i16, )
+declare i32 @llvm.aarch64.sve.clastb.n.nxv4i32(, i32, )
+declare i64 @llvm.aarch64.sve.clastb.n.nxv2i64(, i64, )
Index: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -796,6 +796,44 @@
   return IC.replaceInstUsesWith(II, Extract);
 }
 
+static Optional instCombineSVECondLast(InstCombiner ,
+  IntrinsicInst ) {
+  // Replace scalar integer CLAST[AB] intrinsic with optimal SIMD variant.
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint();
+  Value *Pg = II.getArgOperand(0);
+  Value *Fallback = II.getArgOperand(1);
+  Value *Vec = II.getArgOperand(2);
+  Type *Ty = II.getType();
+
+  if (!Ty->isIntegerTy())
+return None;
+
+  Type *FPTy;
+  switch (cast(Ty)->getBitWidth()) {
+  default:
+return None;
+  case 16:
+FPTy = Builder.getHalfTy();
+break;
+  case 32:
+FPTy = Builder.getFloatTy();
+break;
+  case 64:
+FPTy = Builder.getDoubleTy();
+break;
+  }
+
+  Value *FPFallBack = Builder.CreateBitCast(Fallback, FPTy);
+  auto *FPVTy = VectorType::get(
+  FPTy, cast(Vec->getType())->getElementCount());
+  Value *FPVec = Builder.CreateBitCast(Vec, FPVTy);
+  auto *FPII = Builder.CreateIntrinsic(II.getIntrinsicID(), {FPVec->getType()},
+   {Pg, FPFallBack, FPVec});
+  Value *FPIItoInt = Builder.CreateBitCast(FPII, II.getType());
+  return IC.replaceInstUsesWith(II, FPIItoInt);
+}
+
 static Optional instCombineRDFFR(InstCombiner ,
 IntrinsicInst ) {
   LLVMContext  = II.getContext();
@@ -1294,6 +1332,9 @@
   case Intrinsic::aarch64_sve_lasta:
   case Intrinsic::aarch64_sve_lastb:
 return instCombineSVELast(IC, II);
+  case Intrinsic::aarch64_sve_clasta_n:
+  case 

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/SemaChecking.cpp:13591-13595
+if (!Target->isVLSTBuiltinType() && !isa(OriginalTarget)) {
+  if (S.SourceMgr.isInSystemMacro(CC))
+return;
+  return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
+}

DavidTruby wrote:
> c-rhodes wrote:
> > can a test be added for this?
> It appears this code doesn't actually do anything as this condition is 
> covered elsewhere (and tested below) so I've just removed it
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126380/new/

https://reviews.llvm.org/D126380

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

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




Comment at: clang/lib/Sema/SemaChecking.cpp:13591-13595
+if (!Target->isVLSTBuiltinType() && !isa(OriginalTarget)) {
+  if (S.SourceMgr.isInSystemMacro(CC))
+return;
+  return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
+}

can a test be added for this?



Comment at: clang/lib/Sema/SemaChecking.cpp:13582
+// 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

DavidTruby wrote:
> c-rhodes wrote:
> > this is the same as `Target` defined on line 13473
> 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.
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126380/new/

https://reviews.llvm.org/D126380

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126377: [clang][AArch64][SVE] Improve diagnostics for SVE operators

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126377/new/

https://reviews.llvm.org/D126377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126377: [clang][AArch64][SVE] Improve diagnostics for SVE operators

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/test/Sema/aarch64-sve-vector-arith-ops.c:23
-  (void)(i8 + f64); // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0);   // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0l);  // expected-error{{invalid operands to binary expression}}

DavidTruby wrote:
> c-rhodes wrote:
> > I think these vector + imm tests should be removed in D126380 but fine to 
> > keep here if it's easier
> Ah yes I missed this, you're correct but I will be pushing the two patches at 
> the same time so I guess it doesn't matter much except for correct history? 
> Happy to change it though.
> Ah yes I missed this, you're correct but I will be pushing the two patches at 
> the same time so I guess it doesn't matter much except for correct history? 
> Happy to change it though.

Yeah no worries keep them in here, thanks for clarifying


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126377/new/

https://reviews.llvm.org/D126377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13534-13556
   // Strip vector types.
   if (isa(Source)) {
 if (Target->isVLSTBuiltinType() &&
 (S.Context.areCompatibleSveTypes(QualType(Target, 0),
  QualType(Source, 0)) ||
  S.Context.areLaxCompatibleSveTypes(QualType(Target, 0),
 QualType(Source, 0

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



Comment at: clang/lib/Sema/SemaChecking.cpp:13582
+// 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

this is the same as `Target` defined on line 13473



Comment at: clang/lib/Sema/SemaExpr.cpp:10267
+
+  if (VT) {
+VectorEltTy = VT->getElementType();

how about:

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

?



Comment at: clang/lib/Sema/SemaExpr.cpp:10270-10271
+  } else if (VectorTy->isVLSTBuiltinType()) {
+const auto *BuiltinTy = VectorTy->castAs();
+VectorEltTy = BuiltinTy->getSveEltType(S.getASTContext());
+  } else {

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



Comment at: clang/lib/Sema/SemaExpr.cpp:10273
+  } else {
+llvm_unreachable("Only Fixed-Length and SVE Vector types are handled 
here");
+  }

nit: fixed-length



Comment at: clang/test/CodeGen/aarch64-sve-vector-arith-ops.c:336
+//
+svint8_t add_i8_ilit(svint8_t a) {
+  return a + 0;

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



Comment at: clang/test/Sema/aarch64-sve-vector-scalar-ops.c:1
+// RUN: %clang_cc1 -verify -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +neon -fallow-half-arguments-and-returns -fsyntax-only %s
+

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



Comment at: clang/test/Sema/aarch64-sve-vector-scalar-ops.c:5
+
+#include 
+#include 

what's NEON needed for in this test?



Comment at: clang/test/Sema/aarch64-sve-vector-scalar-ops.c:12
+
+void add_nonscalar(svint32_t i32, struct T a32, svbool_t b) {
+  (void)(i32 + a32); // expected-error{{cannot convert between vector and 
non-scalar values ('svint32_t' (aka '__SVInt32_t') and 'struct T')}}

unused



Comment at: clang/test/Sema/aarch64-sve-vector-scalar-ops.c:17
+void add_bool(svbool_t b) {
+  (void)(b + b); // expected-error{{invalid operands to binary expression 
('svbool_t' (aka '__SVBool_t') and 'svbool_t')}}
+}

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126380/new/

https://reviews.llvm.org/D126380

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126377: [clang][AArch64][SVE] Improve diagnostics for SVE operators

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10609-10614
-  if (RHSType->isVLSTBuiltinType() && !LHSType->isVLSTBuiltinType()) {
-auto DestType = tryScalableVectorConvert((IsCompAssign ? nullptr : ),
- LHSType, RHSType);
-if (DestType == QualType())
-  return InvalidOperands(Loc, LHS, RHS);
-return DestType;

why is this removed?



Comment at: clang/lib/Sema/SemaExpr.cpp:10614
+  (!RHSType->isVLSTBuiltinType() && !RHSType->isRealType())) {
+Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar)
+<< LHSType << RHSType << LHS.get()->getSourceRange()

missing a test for this?



Comment at: clang/test/Sema/aarch64-sve-vector-arith-ops.c:23
-  (void)(i8 + f64); // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0);   // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0l);  // expected-error{{invalid operands to binary expression}}

I think these vector + imm tests should be removed in D126380 but fine to keep 
here if it's easier


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126377/new/

https://reviews.llvm.org/D126377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

can the diagnostics improvements be broke out into separate patches? I think it 
would make this easier to review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124860/new/

https://reviews.llvm.org/D124860

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG698584f89b8f: [IR] Remove unbounded as possible value for 
vscale_range minimum (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D113294?vs=392002=392312#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -210,7 +210,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:482
+return std::pair(
+LangOpts.VScaleMin ? LangOpts.VScaleMin : 1, 16);
+

paulwalker-arm wrote:
> This part is no longer needed because to get here you already know 
> `LangOpts.VScaleMin==0 && LangOpts.VScaleMax==0`. 
> This part is no longer needed because to get here you already know 
> `LangOpts.VScaleMin==0 && LangOpts.VScaleMax==0`. 

Ah of course, I'll fix before committing, cheers!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 392002.
c-rhodes added a comment.

Revert `LangOpt.VScaleMin` default to 0.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
@@ -117,7 +117,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+  assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+  if (LangOpts.VScaleMax)
 return std::pair(LangOpts.VScaleMin,
  LangOpts.VScaleMax);
+
   if (hasFeature("sve"))

paulwalker-arm wrote:
> c-rhodes wrote:
> > paulwalker-arm wrote:
> > > This looks like a change of behaviour to me.  Previously the command line 
> > > flags would override the "sve" default but now that only happens when the 
> > > user specifies a maximum value.  That means the interface can no longer 
> > > be used to force truly width agnostic values.
> > > This looks like a change of behaviour to me.  Previously the command line 
> > > flags would override the "sve" default but now that only happens when the 
> > > user specifies a maximum value.  That means the interface can no longer 
> > > be used to force truly width agnostic values.
> > 
> > I think the issue here is the default of 1 for min would always trigger `if 
> > (LangOpts.VScaleMin || LangOpts.VScaleMax)` overriding the SVE default. 
> > Perhaps the default can be removed from the driver option and handled here, 
> > i.e.
> > 
> > ```
> > if (LangOpts.VScaleMin || LangOpts.VScaleMax)
> > return std::pair(LangOpts.VScaleMin ? 
> > LangOpts.VScaleMin : 1,
> >  LangOpts.VScaleMax);
> > ```
> > 
> > 
> Is this enough?  I'm not sure it'll work because `LangOpts.VScaleMin` 
> defaults to 1 and thus you'll always end up passing the first check, unless 
> the user specifically uses `-mvscale-min=0` which they cannot because that'll 
> result in `diag::err_cc1_unbounded_vscale_min`.
> 
> Do we need to link the LangOpt defaults to the attribute defaults?  I'm think 
> that having the LangOpts default to zero is a good way to represent "value is 
> unspecified" with regards to the `LangOpts.VScaleMin`.
> Is this enough?  I'm not sure it'll work because `LangOpts.VScaleMin` 
> defaults to 1 and thus you'll always end up passing the first check, unless 
> the user specifically uses `-mvscale-min=0` which they cannot because that'll 
> result in `diag::err_cc1_unbounded_vscale_min`.

It works, I removed the default from the driver option:

```diff --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3b85b15739d4..3ec90d42d6ab 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3338,7 +3338,7 @@ def msve_vector_bits_EQ : Joined<["-"], 
"msve-vector-bits=">, Group,
-  MarshallingInfoInt, "1">;
+  MarshallingInfoInt>;```

and updated `clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c`.

> Do we need to link the LangOpt defaults to the attribute defaults?  I'm think 
> that having the LangOpts default to zero is a good way to represent "value is 
> unspecified" with regards to the `LangOpts.VScaleMin`.

I'm not sure the LangOpt.VScaleMin default is having any affect, if it were the 
last CHECK-NONE test in `clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c` 
wouldn't pass. I'll revert it back to zero anyway.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added a comment.

In D113294#3170007 , @paulwalker-arm 
wrote:

> I agree, it's the change to VScaleMin that has caused the issue.  If the 
> LangOpts default can remain as 0 and you can still achieve what you're after 
> then that would be perfect.

Not sure why this was an issue when I first looked at it, fresh eyes maybe, 
fixed now. Thanks for raising.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 391668.
c-rhodes added a comment.

Revert to previous behaviour where both the min/max Clang flags override SVE.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
+++ 

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+  assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+  if (LangOpts.VScaleMax)
 return std::pair(LangOpts.VScaleMin,
  LangOpts.VScaleMax);
+
   if (hasFeature("sve"))

paulwalker-arm wrote:
> This looks like a change of behaviour to me.  Previously the command line 
> flags would override the "sve" default but now that only happens when the 
> user specifies a maximum value.  That means the interface can no longer be 
> used to force truly width agnostic values.
> This looks like a change of behaviour to me.  Previously the command line 
> flags would override the "sve" default but now that only happens when the 
> user specifies a maximum value.  That means the interface can no longer be 
> used to force truly width agnostic values.

I think the issue here is the default of 1 for min would always trigger `if 
(LangOpts.VScaleMin || LangOpts.VScaleMax)` overriding the SVE default. Perhaps 
the default can be removed from the driver option and handled here, i.e.

```
if (LangOpts.VScaleMin || LangOpts.VScaleMax)
return std::pair(LangOpts.VScaleMin ? 
LangOpts.VScaleMin : 1,
 LangOpts.VScaleMax);
```




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 4 inline comments as done.
c-rhodes added a comment.

In D113294#3115354 , @sdesmalen wrote:

> It would be nice if the LLVM interfaces could be updated as well (could also 
> be done in a follow-up patch)

Thanks for reviewing, I'll create a follow-up patch to update the interfaces.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 385732.
c-rhodes added a comment.

Address comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113294/new/

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
@@ -117,7 +117,7 @@
   ret void
 }
 
-attributes #0 = 

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, bsmith, paulwalker-arm.
Herald added subscribers: ctetreau, dexonsmith, dang, jdoerfert, hiraditya.
c-rhodes requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

The default for min is changed to 1. The behaviour of -mvscale-{min,max}
in Clang is also changed such that 16 is the max vscale when targeting
SVE and no max is specified.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum cannot be unbounded
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = 

[PATCH] D109489: [OptParser] NFC: Remove unused template arg 'name' from bool opt

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c8ff4032e2b: [OptParser] NFC: Remove unused template arg 
name from bool opt (authored by c-rhodes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109489/new/

https://reviews.llvm.org/D109489

Files:
  clang/include/clang/Driver/Options.td
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -214,7 +214,7 @@
 }
 
 // Implementation detail of BoolOption.
-class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", 
OPT_"#other_name#")";
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -407,8 +407,7 @@
 Default default>
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag,
+   other.ValueAsCode, other.RecordName>,
 ImpliedByAnyOf {}
 
 // Generates TableGen records for two command line flags that control the same


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -214,7 +214,7 @@
 }
 
 // Implementation detail of BoolOption.
-class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", OPT_"#other_name#")";
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -407,8 +407,7 @@
 Default default>
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag,
+   other.ValueAsCode, other.RecordName>,
 ImpliedByAnyOf {}
 
 // Generates TableGen records for two command line flags that control the same
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109489: [OptParser] NFC: Remove unused template arg 'name' from bool opt

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D109489#2991261 , @jansvoboda11 
wrote:

> LGTM.
>
> To give more background, I think my intention was to add an assertion that 
> `name` and `other_name` are the same (except for the `no_` part) and I was 
> waiting for TableGen assertions to land. But I don't think that's too 
> valuable, since the callers already ensure that by some other mechanism. This 
> should be fine to remove.

No worries thanks for clarifying!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109489/new/

https://reviews.llvm.org/D109489

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109489: [OptParser] NFC: Remove unused template arg 'name' from bool opt

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

@jansvoboda11 this came up in D109359  with 
warning:

  llvm/include/llvm/Option/OptParser.td:217:91: warning: unused template 
argument: MarshallingInfoBooleanFlag:name

I see you've changed worked on this in the past, wasn't sure if it's intended 
for `name` not to be used so thought I'd post the patch and let you take a look.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109489/new/

https://reviews.llvm.org/D109489

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109489: [OptParser] NFC: Remove unused template arg 'name' from bool opt

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added a reviewer: jansvoboda11.
Herald added a subscriber: dang.
c-rhodes requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Identified in D109359 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109489

Files:
  clang/include/clang/Driver/Options.td
  llvm/include/llvm/Option/OptParser.td


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -214,7 +214,7 @@
 }
 
 // Implementation detail of BoolOption.
-class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", 
OPT_"#other_name#")";
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -407,8 +407,7 @@
 Default default>
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag,
+   other.ValueAsCode, other.RecordName>,
 ImpliedByAnyOf {}
 
 // Generates TableGen records for two command line flags that control the same


Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -214,7 +214,7 @@
 }
 
 // Implementation detail of BoolOption.
-class MarshallingInfoBooleanFlag
   : MarshallingInfoFlag {
   code Normalizer = "makeBooleanOptionNormalizer("#value#", "#other_value#", OPT_"#other_name#")";
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -407,8 +407,7 @@
 Default default>
   : Flag<["-"], flag.Spelling>, Flags, HelpText,
 MarshallingInfoBooleanFlag,
+   other.ValueAsCode, other.RecordName>,
 ImpliedByAnyOf {}
 
 // Generates TableGen records for two command line flags that control the same
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109137: [clang] NFC: Remove duplicate DependentSizedMatrixType methods

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9722e8ff9eab: [clang] NFC: Remove duplicate 
DependentSizedMatrixType methods (authored by c-rhodes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109137/new/

https://reviews.llvm.org/D109137

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3524,14 +3524,10 @@
Expr *ColumnExpr, SourceLocation loc);
 
 public:
-  QualType getElementType() const { return ElementType; }
   Expr *getRowExpr() const { return RowExpr; }
   Expr *getColumnExpr() const { return ColumnExpr; }
   SourceLocation getAttributeLoc() const { return loc; }
 
-  bool isSugared() const { return false; }
-  QualType desugar() const { return QualType(this, 0); }
-
   static bool classof(const Type *T) {
 return T->getTypeClass() == DependentSizedMatrix;
   }


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3524,14 +3524,10 @@
Expr *ColumnExpr, SourceLocation loc);
 
 public:
-  QualType getElementType() const { return ElementType; }
   Expr *getRowExpr() const { return RowExpr; }
   Expr *getColumnExpr() const { return ColumnExpr; }
   SourceLocation getAttributeLoc() const { return loc; }
 
-  bool isSugared() const { return false; }
-  QualType desugar() const { return QualType(this, 0); }
-
   static bool classof(const Type *T) {
 return T->getTypeClass() == DependentSizedMatrix;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D109137: [clang] NFC: Remove duplicate DependentSizedMatrixType methods

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added a reviewer: fhahn.
c-rhodes requested review of this revision.
Herald added a project: clang.

Inherited from MatrixType.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109137

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3524,14 +3524,10 @@
Expr *ColumnExpr, SourceLocation loc);
 
 public:
-  QualType getElementType() const { return ElementType; }
   Expr *getRowExpr() const { return RowExpr; }
   Expr *getColumnExpr() const { return ColumnExpr; }
   SourceLocation getAttributeLoc() const { return loc; }
 
-  bool isSugared() const { return false; }
-  QualType desugar() const { return QualType(this, 0); }
-
   static bool classof(const Type *T) {
 return T->getTypeClass() == DependentSizedMatrix;
   }


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3524,14 +3524,10 @@
Expr *ColumnExpr, SourceLocation loc);
 
 public:
-  QualType getElementType() const { return ElementType; }
   Expr *getRowExpr() const { return RowExpr; }
   Expr *getColumnExpr() const { return ColumnExpr; }
   SourceLocation getAttributeLoc() const { return loc; }
 
-  bool isSugared() const { return false; }
-  QualType desugar() const { return QualType(this, 0); }
-
   static bool classof(const Type *T) {
 return T->getTypeClass() == DependentSizedMatrix;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107719: [Clang][AST][NFC] Resolve FIXME: Remove unused QualType ElementType member from the ASTContext class.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D107719#2945656 , @gAlfonso-bit 
wrote:

> Not sure about the timing of base patch, but maybe this patch is also 
> candidate for llvm 13 (backport)?

What's the value in backporting it to 13? It's NFC


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107719/new/

https://reviews.llvm.org/D107719

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106860: [clang][AArch64][SVE] Avoid going through memory for fixed/scalable predicate casts

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

thanks @bsmith, just left one minor nit but otherwise LGTM




Comment at: clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c:10
 typedef svint32_t fixed_int32_t __attribute__((arm_sve_vector_bits(N)));
+typedef svint64_t fixed_int64_t __attribute__((arm_sve_vector_bits(N)));
 typedef svfloat64_t fixed_float64_t __attribute__((arm_sve_vector_bits(N)));

nit: unused?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106860/new/

https://reviews.llvm.org/D106860

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106860: [clang][AArch64][SVE] Avoid going through memory for fixed/scalable predicate casts

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2110-2129
 // Perform VLAT <-> VLST bitcast through memory.
 // TODO: since the llvm.experimental.vector.{insert,extract} intrinsics
 //   require the element types of the vectors to be the same, we
 //   need to keep this around for casting between predicates, or more
 //   generally for bitcasts between VLAT <-> VLST where the element
 //   types of the vectors are not the same, until we figure out a 
better
 //   way of doing these casts.

bsmith wrote:
> c-rhodes wrote:
> > With the predicate casting now using the intrinsics I don't think this is 
> > needed any longer. Perhaps we should add an unreachable above if the 
> > element type doesn't match?
> Don't we still need this for casting between vectors with different element 
> types, or are these guaranteed to not hit this code path?
> Don't we still need this for casting between vectors with different element 
> types, or are these guaranteed to not hit this code path?

Apologies I was wrong, you're right we'll still need it for casting between 
vectors with different element types to support lax vector conversions, 
although it's no longer tested. Might be worth adding a codegen test for ~:

```svint64_t lax_cast(fixed_int32_t t) {
return t;
}```

and the comment could also be updated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106860/new/

https://reviews.llvm.org/D106860

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106860: [clang][AArch64][SVE] Avoid going through memory for fixed/scalable predicate casts

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2065
 if (const auto *FixedSrc = dyn_cast(SrcTy)) {
   if (const auto *ScalableDst = dyn_cast(DstTy)) 
{
+// If we are casting a fixed i8 vector to a scalable 16 x i1 predicate

should const be dropped now the type might change?



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2110-2129
 // Perform VLAT <-> VLST bitcast through memory.
 // TODO: since the llvm.experimental.vector.{insert,extract} intrinsics
 //   require the element types of the vectors to be the same, we
 //   need to keep this around for casting between predicates, or more
 //   generally for bitcasts between VLAT <-> VLST where the element
 //   types of the vectors are not the same, until we figure out a 
better
 //   way of doing these casts.

With the predicate casting now using the intrinsics I don't think this is 
needed any longer. Perhaps we should add an unreachable above if the element 
type doesn't match?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106860/new/

https://reviews.llvm.org/D106860

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106333: [AArch64][SVE] Handle svbool_t VLST <-> VLAT/GNUT conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:8673
 
+/// getSVETypeSize - Return SVE vector or perdicate register size.
+static uint64_t getSVETypeSize(ASTContext , const BuiltinType *Ty) {

s/perdicate/predicate



Comment at: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp:13-14
 typedef svfloat32_t sve_fixed_float32_t SVE_FIXED_ATTR;
 typedef svint32_t sve_fixed_int32_t SVE_FIXED_ATTR;
+typedef svbool_t sve_fixed_bool_t SVE_FIXED_ATTR;
 typedef float gnu_fixed_float32_t GNU_FIXED_ATTR;

unused?



Comment at: clang/test/SemaCXX/aarch64-sve-lax-vector-conversions.cpp:17
 typedef int gnu_fixed_int32_t GNU_FIXED_ATTR;
+typedef int8_t gnu_fixed_bool_t GNU_BOOL_FIXED_ATTR;
 

also unused?



Comment at: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp:32
+
+// Test implicit casts between VLA and VLS perdicates
+svbool_t to_svbool_t(fixed_bool_t x) { return x; }

s/perdicates/predicates


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106333/new/

https://reviews.llvm.org/D106333

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100297: [AArch64][SVE] Always use overloaded methods instead of preprocessor macro.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100297/new/

https://reviews.llvm.org/D100297

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM thanks




Comment at: clang/lib/Sema/SemaChecking.cpp:12054
   // Strip vector types.
-  if (isa(Source)) {
+  if (auto *SourceVT = dyn_cast(Source)) {
+if (Target->isVLSTBuiltinType()) {

nit: can this warning be fixed before landing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97053/new/

https://reviews.llvm.org/D97053

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D97053#2575791 , @craig.topper 
wrote:

> Is this change specific to fixed vectors declared with arm_sve_vector_bits or 
> any of the subclasses of VectorType? If it allows the others, how do we know 
> for sure that there are enough bits in the scalable type for the fixed 
> vector. I ask because RISCV is also using sizeless builtin types for our 
> vectors as of D92715 .

Thanks for pointing out that out Craig, I wasn't aware RISCV is also using 
sizeless builtins now, I hadn't considered that.




Comment at: clang/lib/Sema/SemaChecking.cpp:12055
+  if (auto *SourceVT = dyn_cast(Source)) {
+if (Target->isSizelessBuiltinType()) {
+  auto SourceVectorKind = SourceVT->getVectorKind();

I suppose we could tighten this further by replacing `isSizelessBuiltinType` 
with `isVLSTBuiltinType`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97053/new/

https://reviews.llvm.org/D97053

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: 
clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 

joechrisellis wrote:
> c-rhodes wrote:
> > I wonder if we can just add `-Wconversion` to the existing Sema tests?
> > * clang/test/Sema/attr-arm-sve-vector-bits.c
> > * clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
> > 
> `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// 
> expected-error ...` and such, so I'm reluctant to make changes to that 
> because it seems a little weird to mix up expected errors and non-expected 
> warnings.
> 
> I suppose we could cat this test into `attr-arm-sve-vector-bits.cpp`, though, 
> if you would prefer?  
> `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// 
> expected-error ...` and such, so I'm reluctant to make changes to that 
> because it seems a little weird to mix up expected errors and non-expected 
> warnings.
> 

I sort of see what you mean but unless there's a good reason (which I can't 
really think of) then I think it's preferential to introducing another test for 
this attribute. There's already a bunch of tests with this exact logic being 
tested here the only difference is `-Wconversion` is on.

To be honest I don't have strong feelings about it so it can stay as a separate 
test if you wish, but if it does I think this can be reduced to a single test 
for predicates and one extra test for a data vector like int8 or something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97053/new/

https://reviews.llvm.org/D97053

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: 
clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 

I wonder if we can just add `-Wconversion` to the existing Sema tests?
* clang/test/Sema/attr-arm-sve-vector-bits.c
* clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97053/new/

https://reviews.llvm.org/D97053

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94290: [clang][AArch64][SVE] Avoid going through memory for coerced VLST return values

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

I've left one minor nit but looks otherwise looks fine to me




Comment at: clang/lib/CodeGen/CGCall.cpp:1273
+if (auto *FixedSrc =
+dyn_cast(Src.getElementType())) {
+  if (ScalableDst->getElementType() == FixedSrc->getElementType()) {

nit: `s/Src.getElementType()/SrcTy`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94290/new/

https://reviews.llvm.org/D94290

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92762: [clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2674
+  // perform the conversion.
+  if (Ty->getAs()) {
+auto *Coerced = Fn->getArg(FirstIRArg);

joechrisellis wrote:
> c-rhodes wrote:
> > Do we want to check `VT->getVectorKind() == 
> > VectorType::SveFixedLengthDataVector` and 
> > `isa(Coerced->getType());`?
> Do we not also want to account for predicate vectors here?
> Do we not also want to account for predicate vectors here?

I think so, but as mentioned on D92761 it'll require more thought since the 
insert/extract intrinsics require the element type to be identical which they 
aren't for predicates (`i8` for VLST and `i1` for scalable).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92762/new/

https://reviews.llvm.org/D92762

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92762: [clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CGCall.cpp:2706
+
ArgVals.push_back(ParamValue::forDirect(Builder.CreateExtractVector(
+VecTyTo, Coerced, Zero, "castScalableSve")));
+break;

nit: I know we've used `castSve` for `Name` in a couple of 
places already and it's not very descriptive, but I think it describes the type 
being cast to, in which case this should be `castFixedSve`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92762/new/

https://reviews.llvm.org/D92762

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92762: [clang][AArch64][SVE] Avoid going through memory for coerced VLST arguments

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2671-2673
+  // See if this is a VLST coerced to a VLAT at the function boundary and
+  // the types match up. If so, use llvm.experimental.vector.extract to
+  // perform the conversion.

this is slightly confusing since the coercion done in TargetInfo is from fixed 
-> scalable so VLSTs are represented as scalable vectors in functions 
args/return, yet this is casting back to fixed in the function prolog using 
`llvm.experimental.vector.extract` like you mention in the commit message, 
could this comment clarify that?



Comment at: clang/lib/CodeGen/CGCall.cpp:2674
+  // perform the conversion.
+  if (Ty->getAs()) {
+auto *Coerced = Fn->getArg(FirstIRArg);

Do we want to check `VT->getVectorKind() == 
VectorType::SveFixedLengthDataVector` and 
`isa(Coerced->getType());`?



Comment at: clang/lib/CodeGen/CGCall.cpp:2681
+VecTyFrom->getElementType() == VecTyTo->getElementType()) {
+  llvm::Value *Zero = llvm::Constant::getNullValue(this->CGM.Int64Ty);
+

`this->` can be dropped?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92762/new/

https://reviews.llvm.org/D92762

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92761: [clang][AArch64][SVE] Avoid going through memory for VLAT <-> VLST casts

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

Left a couple of nits but mostly LGTM, cheers




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2025-2051
 // Perform VLAT <-> VLST bitcast through memory.
 if ((isa(SrcTy) &&
  isa(DstTy)) ||
 (isa(SrcTy) &&
  isa(DstTy))) {
   if (const CallExpr *CE = dyn_cast(E)) {
 // Call expressions can't have a scalar return unless the return type

nit: it might be worth adding a comment stating we need to keep this around for 
casting between predicates, until we figure out a better way of doing that.  
The insert/extract intrinsics you've added require the element type to be 
identical and we represent fixed predicates with i8, whereas scalable 
predicates are represented as ``. 



Comment at: llvm/include/llvm/IR/IRBuilder.h:925
   }
 
+  CallInst *CreateExtractVector(Type *DstType, Value *SrcVec, Value *Idx,

`/// Create a call to the experimental.vector.extract intrinsic.`



Comment at: llvm/include/llvm/IR/IRBuilder.h:932
+  }
+
+  CallInst *CreateInsertVector(Type *DstType, Value *SrcVec, Value *SubVec,

`/// Create a call to the experimental.vector.insert intrinsic.`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92761/new/

https://reviews.llvm.org/D92761

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.

In D91696#2405082 , @joechrisellis 
wrote:

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

Cheers, LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91696/new/

https://reviews.llvm.org/D91696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:278-283
-// 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

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; }```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91696/new/

https://reviews.llvm.org/D91696

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91262: [AArch64][SVE] Allow C-style casts between fixed-size and scalable vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM

I've left one minor comment, if that suggestion works it should be fine to fix 
it before committing




Comment at: clang/lib/Sema/SemaExpr.cpp:7210-7212
+const auto *BuiltinTy = FirstType->getAs();
+if (!BuiltinTy || !BuiltinTy->isSizelessBuiltinType())
+  return false;

nit: I think you can just do:
```
if (!FirstType->isSizelessBuiltinType())
  return false;```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91262/new/

https://reviews.llvm.org/D91262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91262: [AArch64][SVE] Allow C-style casts between fixed-size and scalable vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:2217-2218
 
-  // Allow reinterpret_casts between vectors of the same size and
-  // between vectors and integers of the same size.
   bool destIsVector = DestType->isVectorType();

joechrisellis wrote:
> c-rhodes wrote:
> > nit: not sure we need to change this
> I changed this because the code changes mean that the original comment is 
> incomplete. I.e., we now allow for reinterpret casts VLAT <-> VLST, which is 
> not captured by the comment above.
> I changed this because the code changes mean that the original comment is 
> incomplete. I.e., we now allow for reinterpret casts VLAT <-> VLST, which is 
> not captured by the comment above.

It's a minor point but the reason I raised it is we have to be conscious of the 
fact most people probably don't care about SVE and this is changing an existing 
comment to put it first and foremost.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91262/new/

https://reviews.llvm.org/D91262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91262: [AArch64][SVE] Allow C-style casts between fixed-size and scalable vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:2217-2218
 
-  // Allow reinterpret_casts between vectors of the same size and
-  // between vectors and integers of the same size.
   bool destIsVector = DestType->isVectorType();

nit: not sure we need to change this



Comment at: clang/lib/Sema/SemaCast.cpp:2763-2767
+  if (SrcType->isVectorType() || DestType->isVectorType())
+if (Self.isValidSveBitcast(SrcType, DestType)) {
+  Kind = CK_BitCast;
+  return;
+}

I think braces are recommended on the outer if as well, see: 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

Although I suppose it could also be written as:

```
if ((SrcType->isVectorType() || DestType->isVectorType()) &&
Self.isValidSveBitcast(SrcType, DestType)) {
  Kind = CK_BitCast;
  return;
}```



Comment at: clang/lib/Sema/SemaCast.cpp:-2227
+// Allow bitcasting if either the source or destination is a scalable
+// vector.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType()) 
{
+  Kind = CK_BitCast;
+  return TC_Success;
+}

joechrisellis wrote:
> c-rhodes wrote:
> > This is a bit ambiguous, it'll allow bitcasting between things like a fixed 
> > predicate and any sizeless type which I don't think makes sense. I think 
> > it'll also allow bitcasting between any scalable and GNU vectors regardless 
> > of vector length, e.g.:
> > 
> > ```
> > typedef int32_t int32x4_t __attribute__((vector_size(16)));
> > 
> > void foo() {
> > svint32_t s32;
> > int32x4_t g32;
> > g32 = (int32x4_t)s32;
> > s32 = (svint32_t)g32;
> > }
> > ```
> > 
> > the above should only work when `-msve-vector-bits=128` but will currently 
> > be allowed for any N.
> Ah, you're dead right. I think the next diff should fix this, but if there 
> are any more inconsistencies/ambiguities I'll fix them too. :) 
> Ah, you're dead right. I think the next diff should fix this, but if there 
> are any more inconsistencies/ambiguities I'll fix them too. :) 

I suppose it's outside of the scope of this ticket, but I think we'll still 
need to address support for explicit conversions between scalable vectors and 
GNU vectors like in the example I gave.



Comment at: clang/lib/Sema/SemaExpr.cpp:7209-7210
+  auto ValidScalableConversion = [](QualType FirstType, QualType SecondType) {
+if (!FirstType->getAs())
+  return false;
+

Missing a check for `isSizelessBuiltinType`



Comment at: clang/test/SemaCXX/aarch64-sve-explicit-casts-fixed-size.cpp:22-68
+#define TESTCASE(from, to) \
+void from##_to_##to() {\
+from a;\
+to b;  \
+   \
+b = (to) a;\
+}

nit: could simplify this a little further by wrapping the macro, e.g. (moving 
existing `TESTCASE` to `CAST`):
```#define TESTCASE(ty1, ty2) \
CAST(ty1, ty2) \
CAST(ty2, ty1)```



Comment at: clang/test/SemaCXX/aarch64-sve-explicit-casts-fixed-size.cpp:24-25
+void from##_to_##to() {\
+from a;\
+to b;  \
+   \

nit: can just pass these as args `void from##_to_##to(from a, to b) {\`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91262/new/

https://reviews.llvm.org/D91262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91262: [AArch64][SVE] Allow C-style casts between fixed-size and scalable vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

@joechrisellis thanks for the patch Joe, I've added a few comments. I also 
noticed this only covers C++, do we want to test C as well?




Comment at: clang/lib/Sema/SemaCast.cpp:-2227
+// Allow bitcasting if either the source or destination is a scalable
+// vector.
+if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType()) 
{
+  Kind = CK_BitCast;
+  return TC_Success;
+}

This is a bit ambiguous, it'll allow bitcasting between things like a fixed 
predicate and any sizeless type which I don't think makes sense. I think it'll 
also allow bitcasting between any scalable and GNU vectors regardless of vector 
length, e.g.:

```
typedef int32_t int32x4_t __attribute__((vector_size(16)));

void foo() {
svint32_t s32;
int32x4_t g32;
g32 = (int32x4_t)s32;
s32 = (svint32_t)g32;
}
```

the above should only work when `-msve-vector-bits=128` but will currently be 
allowed for any N.



Comment at: clang/test/Sema/aarch64-sve-explicit-casts-fixed-size.cpp:1
+// 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 %s
+

C++ sema tests should be under `clang/test/SemaCXX/`



Comment at: clang/test/Sema/aarch64-sve-explicit-casts.cpp:1-36
+// 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 %s
+
+#include 
+
+// SVE types cannot be C-style casted to one another.
+// "To avoid any ambiguity [between the two operations], the ACLE does not 
allow C-style casting from one
+// vector type to another." ~ACLE Section 3.4 (Vector Types)

`clang/test/SemaCXX/sizeless-1.cpp` already contains a test checking C-style 
casts between distinct sizeless types aren't supported, I'm not sure if adding 
this is necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91262/new/

https://reviews.llvm.org/D91262

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3032
+The vector width is specified by
+``vectorize_width(_value_[, fixed|scalable])``, where __value__ is a positive
+integer and the type of vectorization can be specified with an optional

nit: `s/__value__/_value_/`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89031/new/

https://reviews.llvm.org/D89031

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
c-rhodes marked an inline comment as done.
Closed by commit rG58d3f0ea4972: [clang][aarch64] Address various fixed-length 
SVE vector operations (authored by c-rhodes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88233/new/

https://reviews.llvm.org/D88233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

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
@@ -123,13 +123,56 @@
 void f(int c) {
   fixed_int8_t fs8;
   svint8_t ss8;
+  gnu_int8_t gs8;
 
+  // Check conditional expressions where the result is ambiguous are
+  // ill-formed.
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
 
-  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? gs8 : ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? ss8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  sel = c ? gs8 : fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  // Check binary expressions where the result is ambiguous are ill-formed.
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 + fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 += fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 += ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 += ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 += fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 == fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 == gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 == ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 == gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 == ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 == fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 & fs8; // expected-error {{invalid operands to binary expression}}
+  ss8 = ss8 & gs8; // expected-error {{invalid operands to binary expression}}
+
+  fs8 = fs8 & ss8; // expected-error {{invalid operands to binary expression}}
+  fs8 = fs8 & gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 & ss8; // expected-error {{invalid operands to binary expression}}
+  gs8 = gs8 & fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
 }
 
 // --//
@@ -268,3 +311,78 @@
 

[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 2 inline comments as done.
c-rhodes added a comment.

In D88233#2359022 , @fpetrogalli wrote:

> Hi @c-rhodes, Peter asked me to take a look at this. LGTM, I only have minor 
> stuff.
>
> In the commit message:
>
>> Arm C Language Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE 
>> [1].
>
> It seems that a reference to [1] is missing:  
> https://developer.arm.com/documentation/100987/latest
> Also, please note that the current version is 00bet6. Nothing seem to have 
> changed from 00bet5 to 00bet6 in terms of this patch, but I think it is worth 
> keeping it up to date with the specs numbering until it is merged into master.
>
> Please fix the commit message before submitting.

I didn't realise 00bet6 had been released, thanks for pointing that out.

Thanks for reviewing!




Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:141-158
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and 
sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in 
expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and 
sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in 
expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in 
expression, result is ambiguous}}

fpetrogalli wrote:
> Nit: should you test more binary operators other than just `+`, like you have 
> done for the vector initialization tests?
> Nit: should you test more binary operators other than just +, like you have 
> done for the vector initialization tests?

Added tests for a couple more operators.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88233/new/

https://reviews.llvm.org/D88233

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 301603.
c-rhodes edited the summary of this revision.
c-rhodes added a comment.

Address comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88233/new/

https://reviews.llvm.org/D88233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

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
@@ -123,13 +123,56 @@
 void f(int c) {
   fixed_int8_t fs8;
   svint8_t ss8;
+  gnu_int8_t gs8;
 
+  // Check conditional expressions where the result is ambiguous are
+  // ill-formed.
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
 
-  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? gs8 : ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? ss8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  sel = c ? gs8 : fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  // Check binary expressions where the result is ambiguous are ill-formed.
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 + fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 += fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 += ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 += ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 += fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 == fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 == gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 == ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 == gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 == ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 == fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 = ss8 & fs8; // expected-error {{invalid operands to binary expression}}
+  ss8 = ss8 & gs8; // expected-error {{invalid operands to binary expression}}
+
+  fs8 = fs8 & ss8; // expected-error {{invalid operands to binary expression}}
+  fs8 = fs8 & gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 & ss8; // expected-error {{invalid operands to binary expression}}
+  gs8 = gs8 & fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
 }
 
 // --//
@@ -268,3 +311,78 @@
 TEST_CALL(int32)
 TEST_CALL(float64)
 TEST_CALL(bool)
+
+// --//
+// Vector 

[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, efriedma, rsandifo-arm.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
c-rhodes requested review of this revision.

This patch adds tests and support for operations on SVE vectors created
by the 'arm_sve_vector_bits' attribute, described by the Arm C Language
Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

This covers the following:

- VLSTs support the same forms of element-wise initialization as GNU vectors.
- VLSTs support the same built-in C and C++ operators as GNU vectors.
- Conditional and binary expressions containing GNU and SVE vectors (fixed or 
sizeless) are invalid since the ambiguity around the result type affects the 
ABI.

No functional changes were required to support vector initialization and
operators. The functional changes are to address unsupported conditional and
binary expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

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
@@ -123,13 +123,38 @@
 void f(int c) {
   fixed_int8_t fs8;
   svint8_t ss8;
+  gnu_int8_t gs8;
 
+  // Check conditional expressions where the result is ambiguous are
+  // ill-formed.
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
 
-  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? gs8 : ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? ss8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  sel = c ? gs8 : fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  // Check binary expressions where the result is ambiguous are ill-formed.
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 + fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 += fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 += ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 += ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 += fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
 }
 
 // --//
@@ -268,3 +293,78 @@
 TEST_CALL(int32)
 TEST_CALL(float64)
 TEST_CALL(bool)
+
+// --//
+// Vector initialization
+
+#if defined(__ARM_FEATURE_SVE_BITS) && __ARM_FEATURE_SVE_BITS == 256
+
+typedef svint32_t int32x8 __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat64_t float64x4 __attribute__((arm_sve_vector_bits(N)));
+
+int32x8 foo = {1, 2, 3, 4, 5, 6, 7, 8};
+int32x8 foo2 = {1, 2, 3, 4, 5, 6, 7, 8, 9}; // expected-warning{{excess elements in vector initializer}}
+
+float64x4 bar = {1.0, 2.0, 3.0, 4.0};
+float64x4 bar2 = {1.0, 2.0, 3.0, 4.0, 5.0}; // 

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9218f9283802: [clang][aarch64] ACLE: Support implicit casts 
between GNU and SVE vectors (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D87607?vs=292162=292441#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,14 +1,26 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
+#include 
+
 #define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
 
 template struct S { T var; };
 
 S s;
 
+// Test implicit casts between VLA and VLS vectors
 svint8_t to_svint8_t(fixed_int8_t x) { return x; }
 fixed_int8_t from_svint8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLA vectors
+svint8_t to_svint8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_svint8_t__to_gnu_int8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLS vectors
+fixed_int8_t to_fixed_int8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_fixed_int8_t__to_gnu_int8_t(fixed_int8_t x) { return x; }
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
@@ -1,11 +1,16 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+
+#include 
 
 #define N __ARM_FEATURE_SVE_BITS
 
+typedef __fp16 float16_t;
+typedef float float32_t;
+typedef double float64_t;
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 typedef __SVInt32_t svint32_t;
@@ -19,6 +24,7 @@
 typedef __SVFloat64_t svfloat64_t;
 
 #if defined(__ARM_FEATURE_SVE_BF16)
+typedef __bf16 bfloat16_t;
 typedef __SVBFloat16_t svbfloat16_t;
 #endif
 
@@ -43,6 +49,23 @@
 
 typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
 
+// GNU vector types
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
+typedef int16_t gnu_int16_t __attribute__((vector_size(N / 8)));
+typedef int32_t gnu_int32_t __attribute__((vector_size(N / 8)));
+typedef int64_t gnu_int64_t __attribute__((vector_size(N / 8)));
+
+typedef uint8_t gnu_uint8_t __attribute__((vector_size(N / 8)));

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:237
+// 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

efriedma wrote:
> This test isn't checking what it says it is; int4 and svint64_t have 
> different element types.
> This test isn't checking what it says it is; int4 and svint64_t have 
> different element types.

Good spot! Fixed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 292162.
c-rhodes added a comment.

Address comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,14 +1,26 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
+#include 
+
 #define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
 
 template struct S { T var; };
 
 S s;
 
+// Test implicit casts between VLA and VLS vectors
 svint8_t to_svint8_t(fixed_int8_t x) { return x; }
 fixed_int8_t from_svint8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLA vectors
+svint8_t to_svint8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_svint8_t__to_gnu_int8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLS vectors
+fixed_int8_t to_fixed_int8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_fixed_int8_t__to_gnu_int8_t(fixed_int8_t x) { return x; }
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
@@ -1,11 +1,16 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+
+#include 
 
 #define N __ARM_FEATURE_SVE_BITS
 
+typedef __fp16 float16_t;
+typedef float float32_t;
+typedef double float64_t;
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 typedef __SVInt32_t svint32_t;
@@ -19,6 +24,7 @@
 typedef __SVFloat64_t svfloat64_t;
 
 #if defined(__ARM_FEATURE_SVE_BF16)
+typedef __bf16 bfloat16_t;
 typedef __SVBFloat16_t svbfloat16_t;
 #endif
 
@@ -43,6 +49,23 @@
 
 typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
 
+// GNU vector types
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
+typedef int16_t gnu_int16_t __attribute__((vector_size(N / 8)));
+typedef int32_t gnu_int32_t __attribute__((vector_size(N / 8)));
+typedef int64_t gnu_int64_t __attribute__((vector_size(N / 8)));
+
+typedef uint8_t gnu_uint8_t __attribute__((vector_size(N / 8)));
+typedef uint16_t gnu_uint16_t __attribute__((vector_size(N / 8)));
+typedef uint32_t gnu_uint32_t __attribute__((vector_size(N / 8)));
+typedef uint64_t gnu_uint64_t __attribute__((vector_size(N / 8)));
+
+typedef float16_t gnu_float16_t 

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:8522
+  return VT->getElementType().getCanonicalType() ==
+ getBuiltinVectorTypeInfo(BT).ElementType;
   }

efriedma wrote:
> c-rhodes wrote:
> > efriedma wrote:
> > > We allow casting SVE fixed-width vectors only if the element type is 
> > > identical, but we allow casting to GNU fixed-width vectors if the vector 
> > > width is the same?  That seems sort of loose; could allow weird casts, 
> > > particularly when bool vectors are involved.
> > > We allow casting SVE fixed-width vectors only if the element type is 
> > > identical, but we allow casting to GNU fixed-width vectors if the vector 
> > > width is the same? That seems sort of loose; could allow weird casts, 
> > > particularly when bool vectors are involved.
> > 
> > Fixed-length SVE vectors are created with the attribute and the vector 
> > width must match `-msve-vector-bits`, which should guarantee consistent 
> > vector widths for a given input. This is checking the vector width is the 
> > same and the element type matches when casting between GNU and SVE vectors, 
> > although since neither of those types are created by the attribute it's 
> > necessary to check the vector width to validate `__ARM_FEATURE_SVE_BITS==N`.
> > 
> > Predicates are treated differently in the ACLE and it doesn't mention 
> > supporting casts between GNU bool vectors and SVE VLS/VLA predicates. I'm 
> > also not sure the GNU vector_size extension is defined for bool elements 
> > yet, but I did notice Simon Moll is working on this (D81083).
> I think you need to check both the size and the element type here?
> I think you need to check both the size and the element type here?

Compare the size of a scalable and fixed-length vector?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:233-234
 
+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)}}
+

c-rhodes wrote:
> I expected similar diagnostics when casting between GNU and fixed-length SVE 
> vectors where the element type doesn't match but the vector size is equal but 
> that wasn't the case. Since no functional changes were necessary to support 
> casting between these types I tried the following:
> ```#include 
> 
> #define N 128
> 
> typedef float float32_t;
> typedef double float64_t;
> 
> typedef float32_t gnu_float32_t __attribute__((vector_size(N/8)));
> typedef float64_t gnu_float64_t __attribute__((vector_size(N/8)));
> 
> gnu_float32_t foo(gnu_float64_t x) { return (gnu_float32_t)x; }
> gnu_float64_t bar(gnu_float32_t x) { return (gnu_float64_t)x; }```
> 
> It seems Clang considers this implicit cast valid whereas GCC doesn't, even 
> with lax vector conversions enabled.
I pasted the wrong example, the explicit cast shouldn't be there:
```#include 

#define N 128

typedef float float32_t;
typedef double float64_t;

typedef float32_t gnu_float32_t __attribute__((vector_size(N/8)));
typedef float64_t gnu_float64_t __attribute__((vector_size(N/8)));

gnu_float32_t foo(gnu_float64_t x) { return x; }
gnu_float64_t bar(gnu_float32_t x) { return x; }```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:233-234
 
+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)}}
+

I expected similar diagnostics when casting between GNU and fixed-length SVE 
vectors where the element type doesn't match but the vector size is equal but 
that wasn't the case. Since no functional changes were necessary to support 
casting between these types I tried the following:
```#include 

#define N 128

typedef float float32_t;
typedef double float64_t;

typedef float32_t gnu_float32_t __attribute__((vector_size(N/8)));
typedef float64_t gnu_float64_t __attribute__((vector_size(N/8)));

gnu_float32_t foo(gnu_float64_t x) { return (gnu_float32_t)x; }
gnu_float64_t bar(gnu_float32_t x) { return (gnu_float64_t)x; }```

It seems Clang considers this implicit cast valid whereas GCC doesn't, even 
with lax vector conversions enabled.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 291891.
c-rhodes edited the summary of this revision.
c-rhodes added a comment.

Added tests for implicit casts between GNU and fixed-length SVE vectors. I was 
planning to create a separate patch for this but since it's only tests and no 
functional changes I've included it in this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,14 +1,26 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
+#include 
+
 #define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
 
 template struct S { T var; };
 
 S s;
 
+// Test implicit casts between VLA and VLS vectors
 svint8_t to_svint8_t(fixed_int8_t x) { return x; }
 fixed_int8_t from_svint8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLA vectors
+svint8_t to_svint8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_svint8_t__to_gnu_int8_t(svint8_t x) { return x; }
+
+// Test implicit casts between GNU and VLS vectors
+fixed_int8_t to_fixed_int8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_fixed_int8_t__to_gnu_int8_t(fixed_int8_t x) { return x; }
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
@@ -1,11 +1,16 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+
+#include 
 
 #define N __ARM_FEATURE_SVE_BITS
 
+typedef __fp16 float16_t;
+typedef float float32_t;
+typedef double float64_t;
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 typedef __SVInt32_t svint32_t;
@@ -19,6 +24,7 @@
 typedef __SVFloat64_t svfloat64_t;
 
 #if defined(__ARM_FEATURE_SVE_BF16)
+typedef __bf16 bfloat16_t;
 typedef __SVBFloat16_t svbfloat16_t;
 #endif
 
@@ -43,6 +49,23 @@
 
 typedef svbool_t fixed_bool_t __attribute__((arm_sve_vector_bits(N)));
 
+// GNU vector types
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
+typedef int16_t gnu_int16_t __attribute__((vector_size(N / 8)));
+typedef int32_t gnu_int32_t __attribute__((vector_size(N / 8)));
+typedef int64_t gnu_int64_t __attribute__((vector_size(N / 8)));
+
+typedef uint8_t gnu_uint8_t __attribute__((vector_size(N / 8)));

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:8522
+  return VT->getElementType().getCanonicalType() ==
+ getBuiltinVectorTypeInfo(BT).ElementType;
   }

efriedma wrote:
> We allow casting SVE fixed-width vectors only if the element type is 
> identical, but we allow casting to GNU fixed-width vectors if the vector 
> width is the same?  That seems sort of loose; could allow weird casts, 
> particularly when bool vectors are involved.
> We allow casting SVE fixed-width vectors only if the element type is 
> identical, but we allow casting to GNU fixed-width vectors if the vector 
> width is the same? That seems sort of loose; could allow weird casts, 
> particularly when bool vectors are involved.

Fixed-length SVE vectors are created with the attribute and the vector width 
must match `-msve-vector-bits`, which should guarantee consistent vector widths 
for a given input. This is checking the vector width is the same and the 
element type matches when casting between GNU and SVE vectors, although since 
neither of those types are created by the attribute it's necessary to check the 
vector width to validate `__ARM_FEATURE_SVE_BITS==N`.

Predicates are treated differently in the ACLE and it doesn't mention 
supporting casts between GNU bool vectors and SVE VLS/VLA predicates. I'm also 
not sure the GNU vector_size extension is defined for bool elements yet, but I 
did notice Simon Moll is working on this (D81083).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87607/new/

https://reviews.llvm.org/D87607

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, efriedma, rsandifo-arm.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
c-rhodes requested review of this revision.

This patch adds support for implicit casting between GNU vectors and SVE
vectors when `__ARM_FEATURE_SVE_BITS==N`, as defined by the Arm C
Language Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

This behavior makes it possible to use GNU vectors with ACLE functions
that operate on VLAT. For example:

  typedef int8_t vec __attribute__((vector_size(32)));
  vec f(vec x) { return svasrd_x(svptrue_b8(), x, 1); }

The `__ARM_FEATURE_SVE_VECTOR_OPERATORS` feature macro indicates
interoperability with the GNU vector extension. This is the first patch
providing support for this feature, which once complete will be enabled
by the `-msve-vector-bits` flag, as the `__ARM_FEATURE_SVE_BITS` feature
currently is.

[1] https://developer.arm.com/documentation/100987/latest


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87607

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
+#include 
+
 #define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+typedef int8_t gnu_int8_t __attribute__((vector_size(N / 8)));
 
 template struct S { T var; };
 
@@ -12,3 +15,6 @@
 
 svint8_t to_svint8_t(fixed_int8_t x) { return x; }
 fixed_int8_t from_svint8_t(svint8_t x) { return x; }
+
+svint8_t to_svint8_t__from_gnu_int8_t(gnu_int8_t x) { return x; }
+gnu_int8_t from_svint8_t__to_gnu_int8_t(svint8_t x) { return x; }
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
@@ -1,11 +1,16 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=128 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=256 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -ffreestanding -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
+
+#include 
 
 #define N __ARM_FEATURE_SVE_BITS
 
+typedef __fp16 float16_t;
+typedef float float32_t;
+typedef double float64_t;
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 typedef __SVInt32_t svint32_t;
@@ -19,6 +24,7 @@
 typedef __SVFloat64_t svfloat64_t;
 
 #if defined(__ARM_FEATURE_SVE_BF16)
+typedef __bf16 bfloat16_t;
 typedef __SVBFloat16_t svbfloat16_t;
 #endif
 
@@ -43,6 +49,23 @@
 
 typedef svbool_t fixed_bool_t 

[PATCH] D87463: [clang][aarch64] Fix mangling of bfloat16 neon vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D87463#2267613 , @stuij wrote:

> I'm not sure what the procedure is to get patches into LLVM 11, but I think 
> there's still time. Could you try to get this in there as well @c-rhodes ?

Cherry-picked into LLVM 11, see: https://bugs.llvm.org/show_bug.cgi?id=47490


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87463/new/

https://reviews.llvm.org/D87463

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/AST/Type.cpp:2324
 // scalable and fixed-length vectors.
-return Ctx.UnsignedCharTy;
-  case BuiltinType::SveInt16:
-return Ctx.ShortTy;
-  case BuiltinType::SveUint16:
-return Ctx.UnsignedShortTy;
-  case BuiltinType::SveInt32:
-return Ctx.IntTy;
-  case BuiltinType::SveUint32:
-return Ctx.UnsignedIntTy;
-  case BuiltinType::SveInt64:
-return Ctx.LongTy;
-  case BuiltinType::SveUint64:
-return Ctx.UnsignedLongTy;
-  case BuiltinType::SveFloat16:
-return Ctx.Float16Ty;
-  case BuiltinType::SveBFloat16:
-return Ctx.BFloat16Ty;
-  case BuiltinType::SveFloat32:
-return Ctx.FloatTy;
-  case BuiltinType::SveFloat64:
-return Ctx.DoubleTy;
-  }
+return Ctx.getIntTypeForBitwidth(8, /*Signed=*/0);
+  else

efriedma wrote:
> For 8-bit types, not sure you're getting anything by using 
> getIntTypeForBitwidth instead of Ctx.UnsignedCharTy; there isn't actually any 
> possible variation. I don't care much either way, though.
> For 8-bit types, not sure you're getting anything by using 
> getIntTypeForBitwidth instead of Ctx.UnsignedCharTy; there isn't actually any 
> possible variation. I don't care much either way, though.

Yeah you're right, may as well keep `Ctx.UnsignedCharTy`, I changed it before 
pushing. Thanks for reviewing!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87358/new/

https://reviews.llvm.org/D87358

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87463: [clang][aarch64] Fix mangling of bfloat16 neon vectors

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcabd60c26b5d: [clang][aarch64] Fix mangling of bfloat16 neon 
vectors (authored by c-rhodes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87463/new/

https://reviews.llvm.org/D87463

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-neon-vectors.cpp


Index: clang/test/CodeGenCXX/mangle-neon-vectors.cpp
===
--- clang/test/CodeGenCXX/mangle-neon-vectors.cpp
+++ clang/test/CodeGenCXX/mangle-neon-vectors.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple armv7-apple-ios -target-feature +neon  %s 
-emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios -target-feature +neon %s -emit-llvm 
-o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon %s -emit-llvm 
-o - | FileCheck %s --check-prefix=CHECK-AARCH64
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon 
-target-feature +bf16 %s -emit-llvm -o - | FileCheck %s 
--check-prefix=CHECK-AARCH64-BF16
 
 typedef float float32_t;
 typedef double float64_t;
@@ -14,6 +15,10 @@
 #endif
 typedef unsigned __INT64_TYPE__ uint64_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __bf16 bfloat16_t;
+#endif
+
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
@@ -28,6 +33,10 @@
 typedef __attribute__((neon_polyvector_type(16))) poly8_t  poly8x16_t;
 typedef __attribute__((neon_polyvector_type(8)))  poly16_t poly16x8_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __attribute__((neon_vector_type(4))) __bf16 bfloat16x4_t;
+#endif
+
 // CHECK: 16__simd64_int32_t
 // CHECK-AARCH64: 11__Int32x2_t
 void f1(int32x2_t v) { }
@@ -72,3 +81,8 @@
 // CHECK-AARCH64: 13__Float64x2_t
 void f11(float64x2_t v) { }
 #endif
+
+#if defined(__ARM_FEATURE_BF16)
+// CHECK-AARCH64-BF16: 14__Bfloat16x4_t
+void f12(bfloat16x4_t v) {}
+#endif
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3275,7 +3275,7 @@
   case BuiltinType::Double:
 return "Float64";
   case BuiltinType::BFloat16:
-return "BFloat16";
+return "Bfloat16";
   default:
 llvm_unreachable("Unexpected vector element base type");
   }


Index: clang/test/CodeGenCXX/mangle-neon-vectors.cpp
===
--- clang/test/CodeGenCXX/mangle-neon-vectors.cpp
+++ clang/test/CodeGenCXX/mangle-neon-vectors.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple armv7-apple-ios -target-feature +neon  %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios -target-feature +neon %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-AARCH64
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon -target-feature +bf16 %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-AARCH64-BF16
 
 typedef float float32_t;
 typedef double float64_t;
@@ -14,6 +15,10 @@
 #endif
 typedef unsigned __INT64_TYPE__ uint64_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __bf16 bfloat16_t;
+#endif
+
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
@@ -28,6 +33,10 @@
 typedef __attribute__((neon_polyvector_type(16))) poly8_t  poly8x16_t;
 typedef __attribute__((neon_polyvector_type(8)))  poly16_t poly16x8_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __attribute__((neon_vector_type(4))) __bf16 bfloat16x4_t;
+#endif
+
 // CHECK: 16__simd64_int32_t
 // CHECK-AARCH64: 11__Int32x2_t
 void f1(int32x2_t v) { }
@@ -72,3 +81,8 @@
 // CHECK-AARCH64: 13__Float64x2_t
 void f11(float64x2_t v) { }
 #endif
+
+#if defined(__ARM_FEATURE_BF16)
+// CHECK-AARCH64-BF16: 14__Bfloat16x4_t
+void f12(bfloat16x4_t v) {}
+#endif
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3275,7 +3275,7 @@
   case BuiltinType::Double:
 return "Float64";
   case BuiltinType::BFloat16:
-return "BFloat16";
+return "Bfloat16";
   default:
 llvm_unreachable("Unexpected vector element base type");
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG002f5ab3b171: [clang][aarch64] Fix ILP32 ABI for 
arm_sve_vector_bits (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D87358?vs=290974=291168#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87358/new/

https://reviews.llvm.org/D87358

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c


Index: clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-512
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=1024 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-1024
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=2048 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-2048
+// RUN: %clang_cc1 -triple aarch64_32-unknown-darwin -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ILP32
 
 #include 
 
@@ -579,3 +580,11 @@
 // CHECK-2048-NEXT:  %local_arr_f64 = alloca [3 x <32 x double>], align 16
 // CHECK-2048-NEXT:  %local_arr_bf16 = alloca [3 x <128 x bfloat>], align 16
 // CHECK-2048-NEXT:  %local_arr_bool = alloca [3 x <32 x i8>], align 2
+
+//===--===//
+// ILP32 ABI
+//===--===//
+// CHECK-ILP32: @global_i32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_i64 = global <8 x i64> zeroinitializer, align 16
+// CHECK-ILP32: @global_u32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_u64 = global <8 x i64> zeroinitializer, align 16
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5627,7 +5627,7 @@
   ResType = llvm::ScalableVectorType::get(
   llvm::Type::getInt64Ty(getVMContext()), 2);
   break;
-case BuiltinType::Float16:
+case BuiltinType::Half:
   ResType = llvm::ScalableVectorType::get(
   llvm::Type::getHalfTy(getVMContext()), 8);
   break;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2317,38 +2317,13 @@
   assert(isVLSTBuiltinType() && "unsupported type!");
 
   const BuiltinType *BTy = getAs();
-  switch (BTy->getKind()) {
-  default:
-llvm_unreachable("Unknown builtin SVE type!");
-  case BuiltinType::SveInt8:
-return Ctx.SignedCharTy;
-  case BuiltinType::SveUint8:
-  case BuiltinType::SveBool:
+  if (BTy->getKind() == BuiltinType::SveBool)
 // Represent predicates as i8 rather than i1 to avoid any layout issues.
 // The type is bitcasted to a scalable predicate type when casting between
 // scalable and fixed-length vectors.
 return Ctx.UnsignedCharTy;
-  case BuiltinType::SveInt16:
-return Ctx.ShortTy;
-  case BuiltinType::SveUint16:
-return Ctx.UnsignedShortTy;
-  case BuiltinType::SveInt32:
-return Ctx.IntTy;
-  case BuiltinType::SveUint32:
-return Ctx.UnsignedIntTy;
-  case BuiltinType::SveInt64:
-return Ctx.LongTy;
-  case BuiltinType::SveUint64:
-return Ctx.UnsignedLongTy;
-  case BuiltinType::SveFloat16:
-return Ctx.Float16Ty;
-  case BuiltinType::SveBFloat16:
-return Ctx.BFloat16Ty;
-  case BuiltinType::SveFloat32:
-return Ctx.FloatTy;
-  case BuiltinType::SveFloat64:
-return Ctx.DoubleTy;
-  }
+  else
+return Ctx.getBuiltinVectorTypeInfo(BTy).ElementType;
 }
 
 bool QualType::isPODType(const ASTContext ) const {
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3388,7 +3388,7 @@
   case BuiltinType::ULong:
 TypeName = "__SVUint64_t";
 break;
-  case BuiltinType::Float16:
+  case BuiltinType::Half:
 TypeName = "__SVFloat16_t";
 break;
   case BuiltinType::Float:


Index: clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
===
--- 

[PATCH] D87463: [clang][aarch64] Fix mangling of bfloat16 neon vectors

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, SjoerdMeijer, stuij.
Herald added subscribers: danielkiss, kristof.beyls.
Herald added a project: clang.
c-rhodes requested review of this revision.

The AAPCS64 specifies the internal type is used for c++ mangling. For
bfloat16 it was defined as `BFloat16` when it should be `Bfloat16`, i.e.
lowercase 'f'.

For more information, see:

https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-support-for-advanced-simd-extensions


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87463

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-neon-vectors.cpp


Index: clang/test/CodeGenCXX/mangle-neon-vectors.cpp
===
--- clang/test/CodeGenCXX/mangle-neon-vectors.cpp
+++ clang/test/CodeGenCXX/mangle-neon-vectors.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple armv7-apple-ios -target-feature +neon  %s 
-emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios -target-feature +neon %s -emit-llvm 
-o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon %s -emit-llvm 
-o - | FileCheck %s --check-prefix=CHECK-AARCH64
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon 
-target-feature +bf16 %s -emit-llvm -o - | FileCheck %s 
--check-prefix=CHECK-AARCH64-BF16
 
 typedef float float32_t;
 typedef double float64_t;
@@ -14,6 +15,10 @@
 #endif
 typedef unsigned __INT64_TYPE__ uint64_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __bf16 bfloat16_t;
+#endif
+
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
@@ -28,6 +33,10 @@
 typedef __attribute__((neon_polyvector_type(16))) poly8_t  poly8x16_t;
 typedef __attribute__((neon_polyvector_type(8)))  poly16_t poly16x8_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __attribute__((neon_vector_type(4))) __bf16 bfloat16x4_t;
+#endif
+
 // CHECK: 16__simd64_int32_t
 // CHECK-AARCH64: 11__Int32x2_t
 void f1(int32x2_t v) { }
@@ -72,3 +81,8 @@
 // CHECK-AARCH64: 13__Float64x2_t
 void f11(float64x2_t v) { }
 #endif
+
+#if defined(__ARM_FEATURE_BF16)
+// CHECK-AARCH64-BF16: 14__Bfloat16x4_t
+void f12(bfloat16x4_t v) {}
+#endif
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3275,7 +3275,7 @@
   case BuiltinType::Double:
 return "Float64";
   case BuiltinType::BFloat16:
-return "BFloat16";
+return "Bfloat16";
   default:
 llvm_unreachable("Unexpected vector element base type");
   }


Index: clang/test/CodeGenCXX/mangle-neon-vectors.cpp
===
--- clang/test/CodeGenCXX/mangle-neon-vectors.cpp
+++ clang/test/CodeGenCXX/mangle-neon-vectors.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple armv7-apple-ios -target-feature +neon  %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios -target-feature +neon %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-AARCH64
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon -target-feature +bf16 %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-AARCH64-BF16
 
 typedef float float32_t;
 typedef double float64_t;
@@ -14,6 +15,10 @@
 #endif
 typedef unsigned __INT64_TYPE__ uint64_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __bf16 bfloat16_t;
+#endif
+
 typedef __attribute__((neon_vector_type(2))) int int32x2_t;
 typedef __attribute__((neon_vector_type(4))) int int32x4_t;
 typedef __attribute__((neon_vector_type(1))) uint64_t uint64x1_t;
@@ -28,6 +33,10 @@
 typedef __attribute__((neon_polyvector_type(16))) poly8_t  poly8x16_t;
 typedef __attribute__((neon_polyvector_type(8)))  poly16_t poly16x8_t;
 
+#if defined(__ARM_FEATURE_BF16)
+typedef __attribute__((neon_vector_type(4))) __bf16 bfloat16x4_t;
+#endif
+
 // CHECK: 16__simd64_int32_t
 // CHECK-AARCH64: 11__Int32x2_t
 void f1(int32x2_t v) { }
@@ -72,3 +81,8 @@
 // CHECK-AARCH64: 13__Float64x2_t
 void f11(float64x2_t v) { }
 #endif
+
+#if defined(__ARM_FEATURE_BF16)
+// CHECK-AARCH64-BF16: 14__Bfloat16x4_t
+void f12(bfloat16x4_t v) {}
+#endif
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3275,7 +3275,7 @@
   case BuiltinType::Double:
 return "Float64";
   case BuiltinType::BFloat16:
-return "BFloat16";
+return "Bfloat16";
   default:
 llvm_unreachable("Unexpected vector element base type");
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: clang/lib/AST/Type.cpp:2339
   case BuiltinType::SveInt32:
-return Ctx.IntTy;
+return IsILP32 ? Ctx.LongTy : Ctx.IntTy;
   case BuiltinType::SveUint32:

efriedma wrote:
> sdesmalen wrote:
> > Rather than comparing with a specific triple, how about getting the 
> > information from `TargetInfo`, i.e.
> > 
> >   case BuiltinType::SveInt32:
> > Ctx.getTargetInfo().getLongWidth() == 32 ? Ctx.LongTy : Ctx.IntTy
> The type that actually corresponds to int64_t is TargetInfo::getInt64Type().  
> Not sure if you need that here.
I realised `ASTContext::getIntTypeForBitwidth` handles this and noticed 
`getBuiltinVectorTypeInfo` which Sander recently added makes use of it so I've 
updated it to use that instead for all types except predicates since we still 
need to represent them as i8.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87358/new/

https://reviews.llvm.org/D87358

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 290974.
c-rhodes edited the summary of this revision.
c-rhodes added a comment.

- Get the element type of scalable vectors from `getBuiltinVectorTypeInfo` when 
creating VLS types. This fixes the ABI issue since it calls 
`ASTContext::getIntTypeForBitwidth` which gets the correct type for a given 
target.
- Element type for `svfloat16_t` is changed from `Float16Ty` to `HalfTy` for 
VLS types since this is what’s used elsewhere.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87358/new/

https://reviews.llvm.org/D87358

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c


Index: clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-512
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=1024 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-1024
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=2048 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-2048
+// RUN: %clang_cc1 -triple aarch64_32-unknown-darwin -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ILP32
 
 #include 
 
@@ -579,3 +580,11 @@
 // CHECK-2048-NEXT:  %local_arr_f64 = alloca [3 x <32 x double>], align 16
 // CHECK-2048-NEXT:  %local_arr_bf16 = alloca [3 x <128 x bfloat>], align 16
 // CHECK-2048-NEXT:  %local_arr_bool = alloca [3 x <32 x i8>], align 2
+
+//===--===//
+// ILP32 ABI
+//===--===//
+// CHECK-ILP32: @global_i32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_i64 = global <8 x i64> zeroinitializer, align 16
+// CHECK-ILP32: @global_u32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_u64 = global <8 x i64> zeroinitializer, align 16
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5627,7 +5627,7 @@
   ResType = llvm::ScalableVectorType::get(
   llvm::Type::getInt64Ty(getVMContext()), 2);
   break;
-case BuiltinType::Float16:
+case BuiltinType::Half:
   ResType = llvm::ScalableVectorType::get(
   llvm::Type::getHalfTy(getVMContext()), 8);
   break;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2317,38 +2317,13 @@
   assert(isVLSTBuiltinType() && "unsupported type!");
 
   const BuiltinType *BTy = getAs();
-  switch (BTy->getKind()) {
-  default:
-llvm_unreachable("Unknown builtin SVE type!");
-  case BuiltinType::SveInt8:
-return Ctx.SignedCharTy;
-  case BuiltinType::SveUint8:
-  case BuiltinType::SveBool:
+  if (BTy->getKind() == BuiltinType::SveBool)
 // Represent predicates as i8 rather than i1 to avoid any layout issues.
 // The type is bitcasted to a scalable predicate type when casting between
 // scalable and fixed-length vectors.
-return Ctx.UnsignedCharTy;
-  case BuiltinType::SveInt16:
-return Ctx.ShortTy;
-  case BuiltinType::SveUint16:
-return Ctx.UnsignedShortTy;
-  case BuiltinType::SveInt32:
-return Ctx.IntTy;
-  case BuiltinType::SveUint32:
-return Ctx.UnsignedIntTy;
-  case BuiltinType::SveInt64:
-return Ctx.LongTy;
-  case BuiltinType::SveUint64:
-return Ctx.UnsignedLongTy;
-  case BuiltinType::SveFloat16:
-return Ctx.Float16Ty;
-  case BuiltinType::SveBFloat16:
-return Ctx.BFloat16Ty;
-  case BuiltinType::SveFloat32:
-return Ctx.FloatTy;
-  case BuiltinType::SveFloat64:
-return Ctx.DoubleTy;
-  }
+return Ctx.getIntTypeForBitwidth(8, /*Signed=*/0);
+  else
+return Ctx.getBuiltinVectorTypeInfo(BTy).ElementType;
 }
 
 bool QualType::isPODType(const ASTContext ) const {
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3388,7 +3388,7 @@
   case BuiltinType::ULong:
 TypeName = "__SVUint64_t";
 break;
-  case BuiltinType::Float16:
+  case BuiltinType::Half:
 TypeName = 

[PATCH] D87358: [clang][aarch64] Fix ILP32 ABI for arm_sve_vector_bits

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: efriedma, sdesmalen, rsandifo-arm.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
c-rhodes requested review of this revision.

The element types of scalable vectors are defined in terms of stdint
types in the ACLE. This patch fixes the mapping to builtin types for the
ILP32 ABI when creating VLS types with the arm_sve_vector_bits, where
the mapping is as follows:

  int32_t -> LongTy
  int64_t -> LongLongTy
  uint32_t -> UnsignedLongTy
  uint64_t -> UnsignedLongLongTy

The test uses the 'aarch64_32-unknown-darwin' triple since that seems to be the
only AArch64 target with ILP32 ABI support. Interested in suggestions if
there's another way to test this.

For more information, see:

https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#types-varying-by-data-model
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-support-for-scalable-vectors


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87358

Files:
  clang/lib/AST/Type.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c


Index: clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-512
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=1024 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-1024
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=2048 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-2048
+// RUN: %clang_cc1 -triple aarch64_32-unknown-darwin -target-feature +sve 
-target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns 
-S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-ILP32
 
 #include 
 
@@ -579,3 +580,11 @@
 // CHECK-2048-NEXT:  %local_arr_f64 = alloca [3 x <32 x double>], align 16
 // CHECK-2048-NEXT:  %local_arr_bf16 = alloca [3 x <128 x bfloat>], align 16
 // CHECK-2048-NEXT:  %local_arr_bool = alloca [3 x <32 x i8>], align 2
+
+//===--===//
+// ILP32 ABI
+//===--===//
+// CHECK-ILP32: @global_i32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_i64 = global <8 x i64> zeroinitializer, align 16
+// CHECK-ILP32: @global_u32 = global <16 x i32> zeroinitializer, align 16
+// CHECK-ILP32: @global_u64 = global <8 x i64> zeroinitializer, align 16
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2316,6 +2316,9 @@
 QualType Type::getSveEltType(const ASTContext ) const {
   assert(isVLSTBuiltinType() && "unsupported type!");
 
+  llvm::Triple Triple = Ctx.getTargetInfo().getTriple();
+  bool IsILP32 = Triple.getArch() == llvm::Triple::aarch64_32;
+
   const BuiltinType *BTy = getAs();
   switch (BTy->getKind()) {
   default:
@@ -2333,13 +2336,13 @@
   case BuiltinType::SveUint16:
 return Ctx.UnsignedShortTy;
   case BuiltinType::SveInt32:
-return Ctx.IntTy;
+return IsILP32 ? Ctx.LongTy : Ctx.IntTy;
   case BuiltinType::SveUint32:
-return Ctx.UnsignedIntTy;
+return IsILP32 ? Ctx.UnsignedLongTy : Ctx.UnsignedIntTy;
   case BuiltinType::SveInt64:
-return Ctx.LongTy;
+return IsILP32 ? Ctx.LongLongTy : Ctx.LongTy;
   case BuiltinType::SveUint64:
-return Ctx.UnsignedLongTy;
+return IsILP32 ? Ctx.UnsignedLongLongTy : Ctx.UnsignedLongTy;
   case BuiltinType::SveFloat16:
 return Ctx.Float16Ty;
   case BuiltinType::SveBFloat16:


Index: clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
===
--- clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
+++ clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=512 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-512
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=1024 -fallow-half-arguments-and-returns -S -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-1024
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -msve-vector-bits=2048 

[PATCH] D86720: [clang][aarch64] Drop experimental from __ARM_FEATURE_SVE_BITS macro

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9091e56d34f: [clang][aarch64] Drop experimental from  
__ARM_FEATURE_SVE_BITS macro (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D86720?vs=288352=289668#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86720/new/

https://reviews.llvm.org/D86720

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
  clang/test/CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
  clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
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
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -44,12 +44,12 @@
 // CHECK-NOT: __ARM_BF16_FORMAT_ALTERNATIVE 1
 // CHECK-NOT: __ARM_FEATURE_BF16 1
 // CHECK-NOT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 0
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 512
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 1024
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 2048
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 0
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 128
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 256
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 512
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 1024
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 2048
 
 // RUN: %clang -target aarch64_be-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-BIGENDIAN
 // CHECK-BIGENDIAN: __ARM_BIG_ENDIAN 1
@@ -444,10 +444,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// NOTE: The __ARM_FEATURE_SVE_BITS feature macro is experimental until the
-// feature is complete.
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 512
-// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 1024
-// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 2048
+// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS 128
+// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS 256
+// CHECK-SVE-VECTOR-BITS-512: __ARM_FEATURE_SVE_BITS 512
+// CHECK-SVE-VECTOR-BITS-1024: __ARM_FEATURE_SVE_BITS 1024
+// CHECK-SVE-VECTOR-BITS-2048: __ARM_FEATURE_SVE_BITS 2048
Index: clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes abandoned this revision.
c-rhodes added a comment.

Closing this now the prototype has been split into separate patches that have 
landed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85128/new/

https://reviews.llvm.org/D85128

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2244839 , @leonardchan wrote:

>> The IR differences were caused by the new pass manager which is on by 
>> default for the Fuchsia builder. I've re-landed the patch with a fix for 
>> `CodeGen/attr-arm-sve-vector-bits-call.c` to use the legacy pm with 
>> `-fno-experimental-new-pass-manager`.
>
> Thanks for the update! We do have the new PM on by default, but I'm surprised 
> that this wouldn't appear on `clang-x86_64-debian-new-pass-manager-fast` 
> which also tests the new PM.

No problem, I checked and it did fail for that builder [1] but for some reason 
I didn't receive an email.

[1] 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/14050


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85743/new/

https://reviews.llvm.org/D85743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2243188 , @c-rhodes wrote:

> In D85743#2242931 , @leonardchan 
> wrote:
>
>> Hi! The `attr-arm-sve-vector-bits-call.c` test seems to be failing on our 
>> clang builders:
>>
>> Could you take a look? Thanks.
>>
>> Builder: 
>> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8870800848452818112?
>
> Sorry about that, I've reverted it (commit 2e7041f 
> ) whilst 
> I investigate. Thanks for raising.

The IR differences were caused by the new pass manager which is on by default 
for the Fuchsia builder. I've re-landed the patch with a fix for 
`CodeGen/attr-arm-sve-vector-bits-call.c` to use the legacy pm with 
`-fno-experimental-new-pass-manager`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85743/new/

https://reviews.llvm.org/D85743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2242931 , @leonardchan wrote:

> Hi! The `attr-arm-sve-vector-bits-call.c` test seems to be failing on our 
> clang builders:
>
> Could you take a look? Thanks.
>
> Builder: 
> https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8870800848452818112?

Sorry about that, I've reverted it (commit 2e7041f 
) whilst I 
investigate. Thanks for raising.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85743/new/

https://reviews.llvm.org/D85743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86720: [clang][aarch64] Drop experimental from __ARM_FEATURE_SVE_BITS macro

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, david-arm, efriedma.
Herald added subscribers: danielkiss, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a reviewer: aaron.ballman.
Herald added a project: clang.
c-rhodes requested review of this revision.

The __ARM_FEATURE_SVE_BITS feature macro is specified in the Arm C
Language Extensions (ACLE) for SVE [1] (version 00bet5). From the spec,
where __ARM_FEATURE_SVE_BITS==N:

  When N is nonzero, indicates that the implementation is generating
  code for an N-bit SVE target and that the arm_sve_vector_bits(N)
  attribute is available.

This was defined in D83550  as 
__ARM_FEATURE_SVE_BITS_EXPERIMENTAL and
enabled under the -msve-vector-bits flag to simplify initial tests.
This patch drops _EXPERIMENTAL now there is support for the feature.

[1] https://developer.arm.com/documentation/100987/latest


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86720

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-codegen.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-types.c
  clang/test/CodeGenCXX/aarch64-mangle-sve-fixed-vectors.cpp
  clang/test/CodeGenCXX/aarch64-sve-fixedtypeinfo.cpp
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
 // expected-no-diagnostics
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
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
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=1024 -fallow-half-arguments-and-returns %s
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -msve-vector-bits=2048 -fallow-half-arguments-and-returns %s
 
-#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+#define N __ARM_FEATURE_SVE_BITS
 
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -44,12 +44,12 @@
 // CHECK-NOT: __ARM_BF16_FORMAT_ALTERNATIVE 1
 // CHECK-NOT: __ARM_FEATURE_BF16 1
 // CHECK-NOT: __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 0
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 512
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 1024
-// CHECK-NOT: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 2048
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 0
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 128
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 256
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 512
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 1024
+// CHECK-NOT: __ARM_FEATURE_SVE_BITS 2048
 
 // RUN: %clang -target aarch64_be-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-BIGENDIAN
 // CHECK-BIGENDIAN: __ARM_BIG_ENDIAN 1
@@ -444,10 +444,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=1024 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-1024 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
 // RUN: %clang -target aarch64-arm-none-eabi -march=armv8-a+sve -msve-vector-bits=2048 -x c -E -dM %s -o - 2>&1 | FileCheck -check-prefix=CHECK-SVE-VECTOR-BITS-2048 %s
-// NOTE: The __ARM_FEATURE_SVE_BITS feature macro is experimental until the
-// feature is complete.
-// CHECK-SVE-VECTOR-BITS-128: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 128
-// CHECK-SVE-VECTOR-BITS-256: __ARM_FEATURE_SVE_BITS_EXPERIMENTAL 256
-// CHECK-SVE-VECTOR-BITS-512: 

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85743#2219215 , @efriedma wrote:

> LGTM
>
> Like I mentioned on the review for the prototype, I still think we should try 
> to implement a scheme that makes CK_BItCast between fixed and scalable types 
> trivial.  Doing coercion this way is going to have a significant performance 
> cost.  But there isn't any user-visible effect, so I'm fine with leaving that 
> for a followup.

I agree the bitcast scheme certainly isn't optimal but it's a start at least 
and something we intend to address going forward. Thanks for reviewing!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85743/new/

https://reviews.llvm.org/D85743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfeed5a7239d8: [Sema][AArch64] Support arm_sve_vector_bits 
attribute (authored by c-rhodes).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// expected-no-diagnostics
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+
+template struct S { T var; };
+
+S s;
+
+svint8_t to_svint8_t(fixed_int8_t x) { return x; }
+fixed_int8_t from_svint8_t(svint8_t x) { return x; }
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
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(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 {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+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)}}
+
+// 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 implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error {{returning 'vSInt32' (vector of 4 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
+svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error-re {{returning 'vSInt32' (vector of {{[0-9]+}} 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 
-vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'vSInt32' (vector of 4 'int' values)}}
+vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // 

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 7 inline comments as done.
c-rhodes added a comment.

@rsandifo-arm @aaron.ballman thanks for reviewing!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 288250.
c-rhodes added a comment.

Address comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// expected-no-diagnostics
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+
+template struct S { T var; };
+
+S s;
+
+svint8_t to_svint8_t(fixed_int8_t x) { return x; }
+fixed_int8_t from_svint8_t(svint8_t x) { return x; }
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
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(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 {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+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)}}
+
+// 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 implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error {{returning 'vSInt32' (vector of 4 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
+svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error-re {{returning 'vSInt32' (vector of {{[0-9]+}} 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 
-vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'vSInt32' (vector of 4 'int' values)}}
+vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // expected-error-re {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'vSInt32' (vector of {{[0-9]+}} 

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86100: [Clang][SVE] NFC: Move info about ACLE types into separate function.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM, just one minor nit but seems like a nice improvement




Comment at: clang/include/clang/AST/ASTContext.h:1318
+  BuiltinVectorTypeInfo
+  getElementTypeForBuiltinVector(const BuiltinType *VecTy) const;
+

nit: not sure on the name here since it's returning more than the element type, 
how about `getBuiltinVectorTypeInfo`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86100/new/

https://reviews.llvm.org/D86100

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 2 inline comments as done.
c-rhodes added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3330
+// appendices to the Procedure Call Standard for the Arm Architecture, see:
+// 
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-c-mangling
+void CXXNameMangler::mangleAArch64FixedSveVectorType(const VectorType *T) {

efriedma wrote:
> Mangling them the same way is going to cause practical issues; they're 
> different types from a C++ perspective, so they need distinct manglings.  For 
> example, you'll crash the compiler if you refer to both foo and 
> foo.
> Mangling them the same way is going to cause practical issues; they're 
> different types from a C++ perspective, so they need distinct manglings. For 
> example, you'll crash the compiler if you refer to both foo and 
> foo.

The ACLE is yet to define the mangling scheme for fixed-length SVE types so I 
kept the mangling the same, which is also what GCC currently does. After 
speaking with @rsandifo-arm yesterday we agreed to come up with a mangling 
scheme where the types are mangled in the same way as:

```__SVE_VLS```

where the first argument is the underlying variable-length type and the second 
argument is the SVE vector length in bits.  For example:

```#if __ARM_FEATURE_SVE_BITS==512
// Mangled as 9__SVE_VLSIu11__SVInt32_tLj512EE
typedef svint32_t vec __attribute__((arm_sve_vector_bits(512)));  
// Mangled as 9__SVE_VLSIu10__SVBool_tLj512EE  
typedef svbool_t pred __attribute__((arm_sve_vector_bits(512)));  
#endif```

let us know if you have any feedback/concerns about this approach.



Comment at: clang/lib/CodeGen/CGCall.cpp:1361
+Tmp.getAlignment().getAsAlign(),
+llvm::ConstantInt::get(CGF.IntPtrTy, DstSize.getKnownMinSize()));
   }

@efriedma If we're happy with the element bitcast above this can also be fixed 
but I wasn't if that was ok, although it's pretty much what was implemented in 
the original codegen patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85743/new/

https://reviews.llvm.org/D85743

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 4 inline comments as done.
c-rhodes added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:1941
+// Adjust the alignment for fixed-length SVE predicates.
+if (VT->getVectorKind() == VectorType::SveFixedLengthPredicateVector)
+  Align = 16;

rsandifo-arm wrote:
> The alignment of the SVE data vectors should be 128 too (see the alignof 
> stuff in the ACLE doc).  FWIW, there were two reasons for defining it like 
> that:
>   - the fixed-length types map to the same ABI machine type as the 
> variable-length types
>   - the length isn't required to be a power of 2: an implementation that 
> supports fixed-length 384-bit vectors could define `__ARM_FEATURE_SVE_BITS` 
> to 384.
> 
> 
> 
> The alignment of the SVE data vectors should be 128 too (see the alignof 
> stuff in the ACLE doc).

Ah, good point. This would've bitten us when when we come to supporting 
non-power-of-2 vector lengths.



Comment at: clang/lib/AST/Type.cpp:2338
+  case BuiltinType::SveUint32:
+return Ctx.UnsignedIntTy;
+  case BuiltinType::SveInt64:

rsandifo-arm wrote:
> This is just a note, not sure what can be done about it, but:
> 
> The element types are defined in terms of the stdint.h types.  One snag is 
> that ILP32 newlib defines `int32_t` to be UnsignedLongTy instead of 
> UnsignedIntTy, and defines `uint64_t` to UnsignedLongLongTy.
> 
> In GCC we got away with this because GCC already has hard-coded knowledge 
> about the target C library's choices.  I don't think that information is 
> available here though.
> 
> Like I say, there's nothing that necessarily needs to be “fixed”/dealt-with 
> now, just thought it was worth mentioning.
Thanks for the heads up! I'm not familiar with ILP32 so that's good to know. So 
if `-mabi=ilp32` is passed then I guess we should handle that here. I'll make a 
note of this and address it in a later patch.



Comment at: clang/lib/Sema/SemaExpr.cpp:8934
+  ASTContext ) {
+  auto IsValidCast = [](QualType LHSType, QualType RHSType,
+ASTContext ) {

rsandifo-arm wrote:
> I guess this is personal preference, but it seems more natural to use `[&]` 
> and not pass the context.  Maybe different names from LHSType and RHSType 
> would be better for the nested function, since it's called with both orders.
> I guess this is personal preference, but it seems more natural to use [&] and 
> not pass the context.

Agreed, that's nicer.

> Maybe different names from LHSType and RHSType would be better for the nested 
> function, since it's called with both orders.

I've renamed them to first/second.




Comment at: clang/lib/Sema/SemaExpr.cpp:8942
+VT->getVectorKind() == VectorType::SveFixedLengthPredicateVector &&
+isVector(RHSType, LHSType->getFixedLengthSveEltType(Context)))
+  return true;

rsandifo-arm wrote:
> Is the isVector just being defensive?  I'd have expected it to be redundant, 
> since we shouldn't create SveFixedLengthPredicateVectors with invalid element 
> types.
> Is the isVector just being defensive? I'd have expected it to be redundant, 
> since we shouldn't create SveFixedLengthPredicateVectors with invalid element 
> types.

Yeah although as you say it's not really necessary, fixed.



Comment at: clang/lib/Sema/SemaExpr.cpp:8945
+
+if (VT->getVectorKind() == VectorType::SveFixedLengthDataVector &&
+isVector(RHSType, LHSType->getFixedLengthSveEltType(Context)))

rsandifo-arm wrote:
> It looks like this could still trigger for SveBools.  Maybe it would be 
> better to have:
> 
>   if (BT->getKind() == BuiltinType::SveBool) {
> …
>   } else {
> …
>   }
> 
> instead.  Perhaps it'd also be worth having an assert to show that we've 
> already checked that any builtin type is an SVE type.
I've simplified this somewhat although I'm not sure if it's what you meant. If 
it's a predicate vector kind then it's compatible if the builtin is an SVE 
bool, otherwise it compares the types. Worth noting I've moved this to 
ASTContext also since it's used for the C++ conversions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 285285.
c-rhodes marked an inline comment as not done.
c-rhodes added a comment.

Address @rsandifo-arm comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// expected-no-diagnostics
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+
+template struct S { T var; };
+
+S s;
+
+svint8_t to_svint8_t(fixed_int8_t x) { return x; }
+fixed_int8_t from_svint8_t(svint8_t x) { return x; }
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
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(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 {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+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)}}
+
+// 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 implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error {{returning 'vSInt32' (vector of 4 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
+svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error-re {{returning 'vSInt32' (vector of {{[0-9]+}} 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 
-vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'vSInt32' (vector of 4 'int' values)}}
+vSInt32 to_gnut_from_svint32_t(svint32_t x) { return x; } // expected-error-re {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible 

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes updated this revision to Diff 285124.
c-rhodes added a comment.

Added missing implicit conversions for C++. I considered handling this with the
existing implicit vector conversion although one side of the conversion will be
an SVE builtin, so instead I've added a new conversion specifically for SVE. I
suspect the existing one could support this but I wasn't sure if that was a good
idea (?). In C++ implicit conversions between VLA/VLS have a rank just below
derived-to-base conversion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85736/new/

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c
  clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp

Index: clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fsyntax-only -verify -std=c++11 -msve-vector-bits=512 -fallow-half-arguments-and-returns %s
+// expected-no-diagnostics
+
+#define N __ARM_FEATURE_SVE_BITS_EXPERIMENTAL
+
+typedef __SVInt8_t svint8_t;
+typedef svint8_t fixed_int8_t __attribute__((arm_sve_vector_bits(N)));
+
+template struct S { T var; };
+
+S s;
+
+svint8_t to_svint8_t(fixed_int8_t x) { return x; }
+fixed_int8_t from_svint8_t(svint8_t x) { return x; }
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
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(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 {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+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)}}
+
+// 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 implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error {{returning 'vSInt32' (vector of 4 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
+svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // expected-error-re {{returning 'vSInt32' (vector of {{[0-9]+}} 'int' values) from a function with incompatible result type 'svint32_t' (aka '__SVInt32_t')}}
 
-vSInt32 

[PATCH] D85736: [Sema][AArch64] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: efriedma, sdesmalen, rsandifo-arm, aaron.ballman, 
paulwalker-arm.
Herald added subscribers: danielkiss, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
c-rhodes requested review of this revision.

This patch implements the semantics for the 'arm_sve_vector_bits' type
attribute, defined by the Arm C Language Extensions (ACLE) for SVE [1].
The purpose of this attribute is to define vector-length-specific (VLS)
versions of existing vector-length-agnostic (VLA) types.

The semantics were already implemented by D83551 
, although the
implementation approach has since changed to represent VLSTs as
VectorType in the AST and fixed-length vectors in the IR everywhere
except in function args/returns. This is described in the prototype
patch D85128  demonstrating the new approach.

The semantic changes added in D83551  are 
changed since the
AttributedType is replaced by VectorType in the AST. Minimal changes
were necessary in the previous patch as the canonical type for both VLA
and VLS was the same (i.e. sizeless), except in constructs such as
globals and structs where sizeless types are unsupported. This patch
reverts the changes that permitted VLS types that were represented as
sizeless types in such circumstances, and adds support for implicit
casting between VLA <-> VLS types as described in section 3.7.3.2 of the
ACLE.

Since the SVE builtin types for bool and uint8 are both represented as
BuiltinType::UChar in VLSTs, two new vector kinds are implemented to
distinguish predicate and data vectors.

[1] https://developer.arm.com/documentation/100987/latest


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85736

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

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
@@ -102,8 +102,11 @@
   svint8_t ss8;
 
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{incompatible operand types ('svint8_t' (aka '__SVInt8_t') and 'fixed_int8_t' (aka '__SVInt8_t'))}}
-  sel = c ? fs8 : ss8; // expected-error {{incompatible operand types ('fixed_int8_t' (aka '__SVInt8_t') and 'svint8_t' (aka '__SVInt8_t'))}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+
+  sel = fs8 + ss8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
+  sel = ss8 + fs8; // expected-error {{cannot convert between fixed-length and sizeless vector}}
 }
 
 // --//
@@ -192,14 +195,18 @@
 TEST_CAST(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 {{returning 'svuint8_t' (aka '__SVUint8_t') from a function with incompatible result type 'fixed_int8_t' (aka '__SVInt8_t')}}
-fixed_bool_t to_fixed_bool_t__from_svint32_t(svint32_t x) { return x; } // expected-error {{returning 'svint32_t' (aka '__SVInt32_t') from a function with incompatible result type 'fixed_bool_t' (aka '__SVBool_t')}}
+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)}}
+
+// 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 implicit conversion only applies to fixed-length types
 typedef signed int vSInt32 __attribute__((__vector_size__(16)));
-svint32_t to_svint32_t_from_gnut(vSInt32 x) { return x; } // 

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85128#2193309 , @tschuett wrote:

> Sorry. I meant ABI. Can link GCC .o files with Clang .o files using the 
> attributes?

Yes they should be compatible. The machine-level ABI distinguishes 4 types of 
SVE vector [1]:

- VG×64-bit vector of 8-bit elements
- VG×64-bit vector of 16-bit elements
- VG×64-bit vector of 32-bit elements
- VG×64-bit vector of 64-bit elements

The VLS and VLA types are defined by the ACLE to map to the same machine-level 
SVE vectors (in both compilers). Hence the mangling changes in this patch.

[1] 
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#fundamental-data-types


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85128/new/

https://reviews.llvm.org/D85128

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85128#2192867 , @c-rhodes wrote:

> In D85128#2191108 , @tschuett wrote:
>
>> - Is it compatible with GCC?
>
> Support for this attribute landed in GCC 10 and it's more complete than what 
> this patch implements. We've yet to implement the behaviour guarded by the 
> `__ARM_FEATURE_SVE_VECTOR_OPERATORS` and 
> `__ARM_FEATURE_SVE_PREDICATE_OPERATORS` feature macros, so the GNU 
> `__attribute__((vector_size))` extension is not available and operators such 
> as binary '+' are not supported for VLSTs. Support for this is intended to be 
> addressed by later patches.

Just to clarify, GCC doesn't have support for vectors of booleans or operations 
on them (`__ARM_FEATURE_SVE_PREDICATE_OPERATORS`) yet either but does support 
the behaviour indicated by the other macro.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85128/new/

https://reviews.llvm.org/D85128

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85128#2191401 , @efriedma wrote:

> Not going to write detailed review comments, but this looks like the right 
> approach in general.

Thanks for taking a look! I'll split this up into separate patches soon.

> One high-level thing to consider: we could still decide that in IR 
> generation, we want to represent VLSTs registers using scalable vector types, 
> like the original patch did.  This would allow avoiding the awkward "bitcast" 
> implementation.  That interacts with a relatively narrow slice of clang 
> CodeGen, though; we could easily change it later without impacting the rest 
> of the changes.

Yeah now that the VLST is part of the canonical type with the new vector kinds 
we have more information if we were to go the CodeGenTypes route if that's what 
you're referring to as the narrow slice of CodeGen. That would still require 
converting between VLAT/VLST, I quite like this approach as it gives me more 
confidence we're not missing bitcasts when doing it as part of a cast 
operation. I guess with what you're suggesting the bitcast could still be 
emitted there but the cast operations could be limited in Sema to cases where 
ultimately `ConvertType` would return a type that requires bitcasting, or are 
you saying that could be avoided completely?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85128/new/

https://reviews.llvm.org/D85128

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D85128#2191108 , @tschuett wrote:

> Stupid questions.
>
> - Is it for convenience? You get arrays, global variables, structs, ... . 
> Vectorization becomes easier ...

Yes, this allows the definition of types that can be used in constructs 
sizeless types cannot. In earlier revisions of the ACLE there was the concept 
of sizeless structs defined by a `__sizeless_struct` keyword that could have 
members of sizeless type in addition to members of sized type, although there 
was push back on the idea of sizeless aggregates in general and that idea was 
dropped. If you're interested in the background there's more information here 
[1][2].

[1] https://gcc.gnu.org/legacy-ml/gcc/2019-11/msg00088.html
[2] https://gcc.gnu.org/legacy-ml/gcc-patches/2018-10/msg00868.html

> - Are there any potential performance benefits over scalable vectors?

If VLSTs are represented as fixed-length vectors in LLVM as they are in this 
prototype then hopefully we can take advantage of existing optimisations, 
although I think there is work to be done there especially around the 
interaction with scalable vectors and supporting those in existing passes. This 
patch required a few changes to existing passes to bail out for scalable 
vectors so we're already hitting parts of the codebase we've yet to hit that 
would be good candidates for optimisation work. This also ties into the 
fixed-length code generation work @paulwalker-arm has been doing which is still 
early days. I'm not sure if that answers your question, but ultimately the 
compiler should have more information about these types given the vector size 
is explicit so it should be able to do a better job at optimisation.

> - Is it compatible with GCC?

Support for this attribute landed in GCC 10 and it's more complete than what 
this patch implements. We've yet to implement the behaviour guarded by the 
`__ARM_FEATURE_SVE_VECTOR_OPERATORS` and 
`__ARM_FEATURE_SVE_PREDICATE_OPERATORS` feature macros, so the GNU 
`__attribute__((vector_size))` extension is not available and operators such as 
binary '+' are not supported for VLSTs. Support for this is intended to be 
addressed by later patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85128/new/

https://reviews.llvm.org/D85128

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes abandoned this revision.
c-rhodes added a comment.

I've posted a prototype D85128  with an 
alternative implementation, given it's quite different to this patch I've 
posted it as a separate patch and am abandoning this one. See new patch for 
more details, cheers


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83553/new/

https://reviews.llvm.org/D83553

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

there's a few places the `getNumElements` calls can be fixed by getting the 
initial cast right




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5986
   case NEON::BI__builtin_neon_vqrdmulh_lane_v: {
 auto *RTy = cast(Ty);
 if (BuiltinID == NEON::BI__builtin_neon_vqdmulhq_lane_v ||

cast here



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9872
   case NEON::BI__builtin_neon_vfma_laneq_v: {
 llvm::VectorType *VTy = cast(Ty);
 // v1f64 fma should be mapped to Neon scalar f64 fma

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:1765
   }
   auto *VectorTy = dyn_cast(
   cast(Addr.getPointer()->getType())->getElementType());

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:1799
   llvm::Type *SrcTy = Value->getType();
   auto *VecTy = dyn_cast(SrcTy);
   // Handle vec3 special.

cast here



Comment at: clang/lib/CodeGen/CGExpr.cpp:2214
   if (const VectorType *VTy = Dst.getType()->getAs()) {
 unsigned NumSrcElts = VTy->getNumElements();
 unsigned NumDstElts =

missed one here



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1638
 
 llvm::VectorType *MTy = cast(Mask->getType());
 

cast here



Comment at: clang/lib/CodeGen/SwiftCallingConv.cpp:321
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();

cast here


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82582/new/

https://reviews.llvm.org/D82582



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:152
+  Align, Name,
+  /*ArraySize=*/nullptr, Alloca);
 

efriedma wrote:
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type?  I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type? I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.

You were right, I've spent some time investigating this. The current 
implementation crashes on:
```fixed_int32_t global;
fixed_int32_t address_of_global() {
  fixed_int32_t *global_ptr;
  global_ptr = 
  return *global_ptr;
}```

the reason being `global` is represented as an `ArrayType` whereas the pointer 
`global_ptr` is scalable:

```@global = global [4 x i32] zeroinitializer, align 16
%global_ptr = alloca *, align 8```

so when storing the address of `global` to `global_ptr` the store it tries to 
create causes a crash:

`store [4 x i32]* @global, ** %global_ptr, align 8`

I tried your suggestion to bitcast to alloca to the array type in 
`CreateMemTemp` but found for that example it isn't called, it's created by a 
call to `CreateTempAlloca` in CGDecl.cpp (`EmitAutoVarAlloca`). 
`CreateTempAlloca` takes an `llvm::Type *Ty` so it's not as straightforward as 
doing a bitcast there, although I found it could be done in `EmitAutoVarAlloca` 
but it means having to handle this is two places I'm aware of and potentially 
others I haven't hit. In this case as well it also required looking through the 
pointer to see if the pointee was a VLST then doing a bitcast.

I've also experimented with representing allocas as fixed-length arrays to see 
if that will make it any easier and it does simplify the patch a little. It 
does require handling `PointerType` in `ConvertTypeForMem` however as we do for 
`ConstantArray`, same issue I mentioned in response to your other comment about 
removing that.

I planning to update the patch with that implementation but I've just found 
another issue:

```fixed_int32_t arr[3];
fixed_int32_t *z() {
  fixed_int32_t *array_ptr;
  array_ptr = [0];
  return array_ptr;
}```

trying to create a store:
`store [4 x i32]* %0, ** %retval, align 8`

although this is done in CGStmt.cpp as it's for a retval so it looks like a 
bitcast could also be required there.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3985
+else
+  Init = EmitNullConstant(D->getType());
   } else {

efriedma wrote:
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.

Good spot, these changes can be removed



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:151
+  return llvm::ArrayType::get(*MemTy, A->getSize().getZExtValue());
+  }
+

efriedma wrote:
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.

`ConvertType` looks at the canonical type so the type attribute is lost.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83553/new/

https://reviews.llvm.org/D83553



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits