[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll abandoned this revision.
simoll added a comment.

Taking this off the review queues - we will use the `ext_vector_type` attribute 
instead (D88905 ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

D88905  uses ext_vector_type instead of 
vector_size following up on @rsmith 's suggestion 
. The review should continue there.

I'll keep D81083  around for reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 296137.
simoll added a comment.

(wrong Diff attached in earlier update)

- rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 296135.
simoll added a comment.
Herald added subscribers: llvm-commits, nikic, pengfei, hiraditya, mgorny.
Herald added a project: LLVM.

fixed for privatized ElementCount members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
  llvm/include/llvm/CodeGen/ExpandVectorPredication.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/Analysis/TargetTransformInfo.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/ExpandVectorPredication.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/test/CodeGen/AArch64/O0-pipeline.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/ARM/O3-pipeline.ll
  llvm/test/CodeGen/Generic/expand-vp.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -578,6 +578,7 @@
   initializePostInlineEntryExitInstrumenterPass(Registry);
   initializeUnreachableBlockElimLegacyPassPass(Registry);
   initializeExpandReductionsPass(Registry);
+  initializeExpandVectorPredicationPass(Registry);
   initializeWasmEHPreparePass(Registry);
   initializeWriteBitcodePassPass(Registry);
   initializeHardwareLoopsPass(Registry);
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -318,6 +318,7 @@
   initializeVectorization(*Registry);
   initializeScalarizeMaskedMemIntrinPass(*Registry);
   initializeExpandReductionsPass(*Registry);
+  initializeExpandVectorPredicationPass(*Registry);
   initializeHardwareLoopsPass(*Registry);
   initializeTransformUtils(*Registry);
 
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -24,6 +24,7 @@
 ; CHECK-NEXT:   Lower constant intrinsics
 ; CHECK-NEXT:   Remove unreachable blocks from the CFG
 ; CHECK-NEXT:   Instrument function entry/exit with calls to e.g. mcount() (post inlining)
+; CHECK-NEXT:   Expand vector predication intrinsics
 ; CHECK-NEXT:   Scalarize Masked Memory Intrinsics
 ; CHECK-NEXT:   Expand reduction intrinsics
 ; CHECK-NEXT:   Expand indirectbr instructions
Index: llvm/test/CodeGen/Generic/expand-vp.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/expand-vp.ll
@@ -0,0 +1,84 @@
+; RUN: opt --expand-vec-pred -S < %s | FileCheck %s
+
+; All VP intrinsics have to be lowered into non-VP ops
+; CHECK-NOT: {{call.* @llvm.vp.add}}
+; CHECK-NOT: {{call.* @llvm.vp.sub}}
+; CHECK-NOT: {{call.* @llvm.vp.mul}}
+; CHECK-NOT: {{call.* @llvm.vp.sdiv}}
+; CHECK-NOT: {{call.* @llvm.vp.srem}}
+; CHECK-NOT: {{call.* @llvm.vp.udiv}}
+; CHECK-NOT: {{call.* @llvm.vp.urem}}
+; CHECK-NOT: {{call.* @llvm.vp.and}}
+; CHECK-NOT: {{call.* @llvm.vp.or}}
+; CHECK-NOT: {{call.* @llvm.vp.xor}}
+; CHECK-NOT: {{call.* @llvm.vp.ashr}}
+; CHECK-NOT: {{call.* @llvm.vp.lshr}}
+; CHECK-NOT: {{call.* @llvm.vp.shl}}
+
+define void @test_vp_int_v8(<8 x i32> %i0, <8 x i32> %i1, <8 x i32> %i2, <8 x i32> %f3, <8 x i1> %m, i32 %n) {
+  %r0 = call <8 x i32> @llvm.vp.add.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r1 = call <8 x i32> @llvm.vp.sub.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r2 = call <8 x i32> @llvm.vp.mul.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r3 = call <8 x i32> @llvm.vp.sdiv.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r4 = call <8 x i32> @llvm.vp.srem.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r5 = call <8 x i32> @llvm.vp.udiv.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r6 = call <8 x i32> @llvm.vp.urem.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r7 = call <8 x i32> @llvm.vp.and.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r8 = call <8 x i32> @llvm.vp.or.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %r9 = call <8 x i32> @llvm.vp.xor.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rA = call <8 x i32> @llvm.vp.ashr.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rB = call <8 x i32> @llvm.vp.lshr.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  %rC = call <8 x i32> @llvm.vp.shl.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
+  ret void
+}
+
+; fixed-width vectors
+; integer arith
+declare <8 x i32> @llvm.vp.add.v8i32(<8 x i32>, <8 x i32>, <8 x i1>, i32)
+declare <8 x i32> 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

I'd like to hear your feedback on how to proceed with this patch.

Here is what i would do:

For this patch, we switch to ext_vector_type for bool vectors to not clash head 
on with GCC should GCC decide to allow vector_size bool with a different 
semantics in the future.
For the future, we propose a `_BitVector(N)` extension possibly aiming for a 
C/C++ TS. We can build on our experience with the ext_vector_type bool for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 289893.
simoll added a comment.

Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
  

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

In D81083#2237284 , @rsmith wrote:

> 

Thanks for dropping by!

> Have you considered using the `ext_vector_type` attribute instead of 
> `vector_size` for this? That would have a couple of advantages:
>
> - It's not a GCC attribute, so there's no risk that GCC would give different 
> meaning to the same syntax (such as using one byte per vector element, which 
> would be the natural meaning for a vector of `bool` as an extension of 
> existing functionality).
> - `ext_vector_type` takes as an argument a number of elements, not a size in 
> bytes, so you could support vectors of non-multiple-of-8 `bool`s.

Whichever solution is going to land in Clang, we will end up in the situation 
that Clang supports bool vectors and GCC does not. Any code that uses this 
feature and is supposed to compile with GCC will need some `#ifdef` guard. This 
is why i am not too worried about (ab-)using the vector_size attribute for this.

> However, using either an `ext_vector_type` of `bool` or a `vector_size` of 
> `bool` would still seem problematic in a couple of other ways:
>
> - Those types always produce a vector whose element type is the specified 
> type; you can form an lvalue denoting the element, the element is always 
> represented the same as the underlying type, and you can pun between a vector 
> of N `T` and an array of N `T`. GCC even permits forming a pointer or 
> reference to a vector element, and you should assume that Clang will 
> eventually support that too. That would not be possible for `bool` if packed 
> as a bit-vector.
> - You can already perform logical operations between our existing vector 
> types -- for example, comparing two vectors of 4 `int`s. If vectors of `bool` 
> are valid, it would be logical and natural to expect a vector of 4 `bool`s to 
> have some connection to the result of such a comparison, but it won't: the 
> type of a comparison of two vectors of 4 `int`s is a vector of 4 `int`s each 
> of which is either 0 or -1, and we can't convert that to a vector of 4 
> `bool`s represented as our existing vector types, because such conversions 
> would be interpreted as bitcasts, and we'd reject because the bitwidth of the 
> vectors is different.
> - Vectors of integer types (such as `bool`) generally support arithmetic 
> operators, which you presumably do not want to support here.
> - Our existing vector types carry a lot of baggage (for example, lax vector 
> conversions, and the egregious attribute-on-typedef-changes-the-type hack) 
> that we would not want a new type to include.

I agree that the vector_size syntax for bool vectors raises some expectations 
on the behavior of the bool vector. However, i see this more as an issue of 
proper documentation. The moment somebody uses vector_size attributes they know 
they are deep in compiler-extension land and far from standard C/C++ semantics.
Now about the vector representation issues in Clang and how we could make this 
work for bool vectors: earlier, i suggested that we could introduce a 
Clang-internal `Bit` datatype that can only be used as a vector element type. 
By doing so, we wouldn't violate the invariant that `bits(vector) == 
size_of_vector * bits(element_type)`.

> So I think you should seriously consider using a new syntax to form a 
> bit-vector type. It seems to me that an N-bit bit-vector is quite similar to 
> an `_ExtInt(N)`, but without the arithmetic support and with a different 
> expected IR type. Maybe following that syntactic path and adding 
> `_BitVector(N)` syntax would work out better?

I am open to a different syntax, if piggy backing on the vector_size syntax is 
the main point of contention. Up to now, the plan was to have vector_size bool 
now and work on a proper bool vector type to replace it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Have you considered using the `ext_vector_type` attribute instead of 
`vector_size` for this? That would have a couple of advantages:

- It's not a GCC attribute, so there's no risk that GCC would give different 
meaning to the same syntax (such as using one byte per vector element, which 
would be the natural meaning for a vector of `bool` as an extension of existing 
functionality).
- `ext_vector_type` takes as an argument a number of elements, not a size in 
bytes, so you could support vectors of non-multiple-of-8 `bool`s.

However, using either an `ext_vector_type` of `bool` or a `vector_size` of 
`bool` would still seem problematic in a couple of other ways:

- Those types always produce a vector whose element type is the specified type; 
you can form an lvalue denoting the element, the element is always represented 
the same as the underlying type, and you can pun between a vector of N `T` and 
an array of N `T`. GCC even permits forming a pointer or reference to a vector 
element, and you should assume that Clang will eventually support that too. 
That would not be possible for `bool` if packed as a bit-vector.
- You can already perform logical operations between our existing vector types 
-- for example, comparing two vectors of 4 `int`s. If vectors of `bool` are 
valid, it would be logical and natural to expect a vector of 4 `bool`s to have 
some connection to the result of such a comparison, but it won't: the type of a 
comparison of two vectors of 4 `int`s is a vector of 4 `int`s each of which is 
either 0 or -1, and we can't convert that to a vector of 4 `bool`s represented 
as our existing vector types, because such conversions would be interpreted as 
bitcasts, and we'd reject because the bitwidth of the vectors is different.
- Vectors of integer types (such as `bool`) generally support arithmetic 
operators, which you presumably do not want to support here.
- Our existing vector types carry a lot of baggage (for example, lax vector 
conversions, and the egregious attribute-on-typedef-changes-the-type hack) that 
we would not want a new type to include.

So I think you should seriously consider using a new syntax to form a 
bit-vector type. It seems to me that an N-bit bit-vector is quite similar to an 
`_ExtInt(N)`, but without the arithmetic support and with a different expected 
IR type. Maybe following that syntactic path and adding `_BitVector(N)` syntax 
would work out better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 287692.
simoll added a comment.

NFC. Make clang-tidy happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 287658.
simoll added a comment.

- made bool-vector layout target dependent (one byte per bool element on 
Hexagon, one bit everywhere else).
- Added hvx-specific debug info test.
- Updated docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

The Hexagon builtins explicitly require bool vectors to have 8-bit wide bool 
elements. That clashes with our new vector type because bool vectors in builtin 
functions and vector_size bool vectors have the same internal representation in 
Clang. However, AFAIK Hexagon is the only target that actually need 8-bit 
bools. I see two possible ways out of this:

1. Introduce a `Bit` type in Clang that is only constructible as a vector 
element type. This way, any existing users of generic bool vectors are 
undisturbed. VE can use `Bit` in its builtin declarations to match the 
vector_size bool type. We may re-purpose the `Bit` type for a proper boolean 
vector type in the future.
2. Add a new target property: `BoolInVectorSize` that is the size of a bool 
element in boolean vectors. Hexagon, as most targets, would define 
`BoolInVectorSize = BoolSize`. VE would define `BoolInVectorSize = 1` for the 
intended behavior.

My preference is for 2. since it means small changes to the current patch and 
less surprises than the other option. Opinions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

In D81083#2227760 , @rsandifo-arm 
wrote:

> I'm not qualified to review the CodeGen stuff (or accept the patch, obvs. 
> :-)) but FWIW, here are some comments on the doc and Sema side.

Thanks for your comments! Maybe you know people who could review the CodeGen 
side of this..?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 286971.
simoll edited the summary of this revision.
simoll added a comment.

- Rebased.
- Allow comparisons on boolean vectors.
- Restored result type for vector comparisons on other types.
- Added operator, alignment and constexpr tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll marked 4 inline comments as done.
simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:492
+
+The memory representation of a boolean vector is the smallest fitting
+power-of-two integer. The alignment is the alignment of that integer type.  
This

rsandifo-arm wrote:
> simoll wrote:
> > rsandifo-arm wrote:
> > > It might be worth clarifying this.  With the alignment referring 
> > > specifically to “integer type”, I wasn't sure what something like:
> > > 
> > >   bool __attribute__((vector_size(256)))
> > > 
> > > would mean on targets that don't provide 256-byte integer types.  Is the 
> > > type still 256-byte aligned?
> > Not exactly: It is the alignment of the corresponding integer type.
> > For example the following C code:
> > 
> > typedef bool bool256 __attribute__((vector_size(256)));
> > bool256 P;
> > 
> > gives you (x86_64 host, no machine flags specified):
> > 
> > %P = alloca i2048, align 32
> I guess my point is pedantic, but what I meant was: does the concept of “the 
> corresponding integer type” necessarily exist at the C/C++ level?  E.g. how 
> would you end up with the same LLVM IR statement using C integer types 
> instead of vectors?
> 
> My worry was that “the alignment of the corresponding integer type” would 
> only be meaningful to the user if they could identify what the corresponding 
> (C) integer type actually was, and be able to determine its alignment that 
> way.
> 
To be honest, i hadn't given alignment much thought before. Actually, i am not 
too happy about bringing in integer types here.

I guess it's sensible to do no different than for the other vector types: take 
the size rounded up to the next power-ot-two capped by the target's maximum 
vector alignment.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:492
+
+The memory representation of a boolean vector is the smallest fitting
+power-of-two integer. The alignment is the alignment of that integer type.  
This

simoll wrote:
> rsandifo-arm wrote:
> > It might be worth clarifying this.  With the alignment referring 
> > specifically to “integer type”, I wasn't sure what something like:
> > 
> >   bool __attribute__((vector_size(256)))
> > 
> > would mean on targets that don't provide 256-byte integer types.  Is the 
> > type still 256-byte aligned?
> Not exactly: It is the alignment of the corresponding integer type.
> For example the following C code:
> 
> typedef bool bool256 __attribute__((vector_size(256)));
> bool256 P;
> 
> gives you (x86_64 host, no machine flags specified):
> 
> %P = alloca i2048, align 32
I guess my point is pedantic, but what I meant was: does the concept of “the 
corresponding integer type” necessarily exist at the C/C++ level?  E.g. how 
would you end up with the same LLVM IR statement using C integer types instead 
of vectors?

My worry was that “the alignment of the corresponding integer type” would only 
be meaningful to the user if they could identify what the corresponding (C) 
integer type actually was, and be able to determine its alignment that way.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll planned changes to this revision.
simoll marked 6 inline comments as done.
simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:492
+
+The memory representation of a boolean vector is the smallest fitting
+power-of-two integer. The alignment is the alignment of that integer type.  
This

rsandifo-arm wrote:
> It might be worth clarifying this.  With the alignment referring specifically 
> to “integer type”, I wasn't sure what something like:
> 
>   bool __attribute__((vector_size(256)))
> 
> would mean on targets that don't provide 256-byte integer types.  Is the type 
> still 256-byte aligned?
Not exactly: It is the alignment of the corresponding integer type.
For example the following C code:

typedef bool bool256 __attribute__((vector_size(256)));
bool256 P;

gives you (x86_64 host, no machine flags specified):

%P = alloca i2048, align 32



Comment at: clang/include/clang/AST/Type.h:3248
 
+  bool isVectorSizeBoolean() const {
+return (getVectorKind() == VectorKind::GenericVector) &&

rsandifo-arm wrote:
> Maybe isGenericBooleanVector(), so that the terminology is consistent?  Just 
> a suggestion though, not sure if it's an improvement.
I gave it this specific name so where ever it is used, it documents that 
whatever depends on it is specific to vector_size bool. This should be cleaned 
up when a better boolean vector type is introduced.



Comment at: clang/lib/Sema/SemaExpr.cpp:9779
+  return Ty->isBooleanType() ||
+ (Ty->isVectorType() &&
+  Ty->getAs()->getElementType()->isBooleanType());

rsandifo-arm wrote:
> Is this deliberately wider than isVectorSizeBoolean(), or does it amount to 
> the same thing?
That's an artifact. Will narrow this down to `isVectorSizeBoolean()`.



Comment at: clang/lib/Sema/SemaExpr.cpp:11929
  VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.BoolTy))
+return Context.getVectorType(Context.BoolTy, VTy->getNumElements(),

rsandifo-arm wrote:
> In practice, won't this either be a no-op (e.g. for 4-byte-per-bool targets) 
> or always trump the fallback CharTy case?  I wasn't sure why the case was 
> needed.
I guess so. Will remove this case.



Comment at: clang/lib/Sema/SemaExpr.cpp:11956
+  /*AllowBoolConversions*/ getLangOpts().ZVector,
+  /*AllowBooleanOperation*/ false);
   if (vType.isNull())

rsandifo-arm wrote:
> Seems like it might be useful to support comparisons too (with false < true, 
> as for scalars).  E.g. I guess x == y would otherwise need to be written ~(x 
> ^ y), which seems less readable.  Supporting more operators would also help 
> in templated contexts.
Agreed.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:6271
+   /*AllowBoolConversions*/ false,
+   /*AllowBoolOperator*/ false);
 

rsandifo-arm wrote:
> Any reason not to support this for booleans?  ?: seems like a natural 
> operation for all vector element types.
Sure, will allow it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

I'm not qualified to review the CodeGen stuff (or accept the patch, obvs. :-)) 
but FWIW, here are some comments on the doc and Sema side.

It might be good to have more Sema tests for valid and invalid usage, e.g. for 
which operators are valid and which aren't.

Thanks again for doing this btw.




Comment at: clang/docs/LanguageExtensions.rst:459
+
+Different than GCC, Clang also allows the attribute to be used with boolean
+element types. For example:

Maybe “Unlike GCC,”?  Not much in it though.



Comment at: clang/docs/LanguageExtensions.rst:489
+  packing).
+* Bitwise `~`, `|`, `&`, `^` and `~` are the only allowed operators on boolean
+  vectors.

`~` is listed twice.  I guess “operators” (without qualification) might make 
people think of the C++ keyword, in which case assignment and `[]` are allowed 
too.



Comment at: clang/docs/LanguageExtensions.rst:492
+
+The memory representation of a boolean vector is the smallest fitting
+power-of-two integer. The alignment is the alignment of that integer type.  
This

It might be worth clarifying this.  With the alignment referring specifically 
to “integer type”, I wasn't sure what something like:

  bool __attribute__((vector_size(256)))

would mean on targets that don't provide 256-byte integer types.  Is the type 
still 256-byte aligned?



Comment at: clang/include/clang/AST/Type.h:3248
 
+  bool isVectorSizeBoolean() const {
+return (getVectorKind() == VectorKind::GenericVector) &&

Maybe isGenericBooleanVector(), so that the terminology is consistent?  Just a 
suggestion though, not sure if it's an improvement.



Comment at: clang/lib/Sema/SemaExpr.cpp:9779
+  return Ty->isBooleanType() ||
+ (Ty->isVectorType() &&
+  Ty->getAs()->getElementType()->isBooleanType());

Is this deliberately wider than isVectorSizeBoolean(), or does it amount to the 
same thing?



Comment at: clang/lib/Sema/SemaExpr.cpp:11929
  VectorType::GenericVector);
+  else if (TypeSize == Context.getTypeSize(Context.BoolTy))
+return Context.getVectorType(Context.BoolTy, VTy->getNumElements(),

In practice, won't this either be a no-op (e.g. for 4-byte-per-bool targets) or 
always trump the fallback CharTy case?  I wasn't sure why the case was needed.



Comment at: clang/lib/Sema/SemaExpr.cpp:11956
+  /*AllowBoolConversions*/ getLangOpts().ZVector,
+  /*AllowBooleanOperation*/ false);
   if (vType.isNull())

Seems like it might be useful to support comparisons too (with false < true, as 
for scalars).  E.g. I guess x == y would otherwise need to be written ~(x ^ y), 
which seems less readable.  Supporting more operators would also help in 
templated contexts.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:6271
+   /*AllowBoolConversions*/ false,
+   /*AllowBoolOperator*/ false);
 

Any reason not to support this for booleans?  ?: seems like a natural operation 
for all vector element types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.

Thank you preparing this i1 patch and doing it on clang side only.  We were 
testing this patch and sending problem reports.  Now, we can use this patch 
without modifying llvm code.  We can define vector mask types like below.  
Then, we can define intrinsic functions using this type.  Those are converted 
to mask type, e.g v256i1, in the back end without adding bitcasts in 
ISelLowering.cpp.  It helps developers very much.  I hope this extension is 
accepted.

  typedef double __vr __attribute__((__vector_size__(2048)));
  #if __STDC_VERSION__ >= 199901L
  // For C99
  typedef _Bool __vm__attribute__((__vector_size__(32)));
  typedef _Bool __vm256 __attribute__((__vector_size__(32)));
  typedef _Bool __vm512 __attribute__((__vector_size__(64)));
  #else
  #ifdef __cplusplus
  // For C++
  typedef bool __vm__attribute__((__vector_size__(32)));
  typedef bool __vm256 __attribute__((__vector_size__(32)));
  typedef bool __vm512 __attribute__((__vector_size__(64)));
  #else
  #error need C++ or C99 to use vector intrinsics for VE
  #endif
  #endif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 285282.
simoll added a comment.

- Fixed type printing & added type printing test.
- Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 283636.
simoll added a comment.

NFC. Cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto T = CmpMul(a, b);
   // CHECK: store <4 x i8> 
Index: 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 283632.
simoll added a comment.

Fixed debug info representation for bool vectors.
Interpret 8*N with the N in vector_size(N) as the bool numbers of bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll planned changes to this revision.
simoll added a comment.

Thanks for the feedback!
Planned change: interpret the `N` in `vector_size(N)` as the number of bytes in 
the type, such that, for example, a `vector_size(2)` bool vector holds 16 bits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > rsandifo-arm wrote:
> > > > simoll wrote:
> > > > > rsandifo-arm wrote:
> > > > > > simoll wrote:
> > > > > > > lenary wrote:
> > > > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > > > Yep. that caught me by surprise too. I'll move that sentence to 
> > > > > > > the paragraph about GCC vectors above.
> > > > > > Sorry for the extremely late comment.  Like @lenary I hadn't 
> > > > > > thought about this.  I'd assumed that the vector woiuld still be a 
> > > > > > multiple of 8 bits in size, but I agree that's probably too 
> > > > > > restrictive to be the only option available.
> > > > > > 
> > > > > > In that case, would it make sense to add a separate attribute 
> > > > > > instead?  I think it's too surprising to change the units of the 
> > > > > > existing attribute based on the element type.  Perhaps we should 
> > > > > > even make it take two parameters: the total number of elements, and 
> > > > > > the number of bits per element.  That might be more natural for 
> > > > > > some AVX and SVE combinations.  We wouldn't need to supporrt all 
> > > > > > combinations from the outset, it's just a question whether we 
> > > > > > should make the syntax general enough to support it in future.
> > > > > > 
> > > > > > Perhaps we could do both: support `vector_size` for `bool` using 
> > > > > > byte sizes (and not allowing subbyte vector lengths), and add a 
> > > > > > new, more general attribute that allows subbyte lengths and 
> > > > > > explicit subbyte element sizes.
> > > > > > In that case, would it make sense to add a separate attribute 
> > > > > > instead? I think it's too surprising to change the units of the 
> > > > > > existing attribute based on the element type. Perhaps we should 
> > > > > > even make it take two parameters: the total number of elements, and 
> > > > > > the number of bits per element. That might be more natural for some 
> > > > > > AVX and SVE combinations. We wouldn't need to supporrt all 
> > > > > > combinations from the outset, it's just a question whether we 
> > > > > > should make the syntax general enough to support it in future.
> > > > > 
> > > > > I guess adding a new attribute makes sense mid to long term. For now, 
> > > > > i'd want something that just does the job... ie, what is proposed 
> > > > > here. We should absolutely document the semantics of vector_size 
> > > > > properly.. it already is counterintuitive (broken, if you ask me).
> > > > > 
> > > > > 
> > > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > > sizes (and not allowing subbyte vector lengths), and add a new, 
> > > > > > more general attribute that allows subbyte lengths and explicit 
> > > > > > subbyte element sizes.
> > > > > 
> > > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > > because these types are produced implicitly by compares of (legal) 
> > > > > vector types. Consider this:
> > > > > 
> > > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > > >   int3 A = ...;
> > > > >   int3 B = ...;
> > > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > > vector_size(3)`-typed value.
> > > > > 
> > > > > 
> > > > > Regarding your proposal for a separate subbyte element size and 
> > > > > subbyte length, that may or may not make sense but it is surely 
> > > > > something that should be discussed more broadly with its own proposal.
> > > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > > sizes (and not allowing subbyte vector lengths), and add a new, 
> > > > > > more general attribute that allows subbyte lengths and explicit 
> > > > > > subbyte element sizes.
> > > > > 
> > > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > > because these types are produced implicitly by compares of (legal) 
> > > > > vector types. Consider this:
> > > > > 
> > > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > > >   int3 A = ...;
> > > > >   int3 B = ...;
> > > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > > vector_size(3)`-typed value.
> > > > 
> > > > Yeah, I understand the need for some way of creating subbyte vectors.  
> > > > I'm just not sure using the existing `vector_size` attribute would be 
> > > > the best choice for that case.  I.e. the objection wasn't based on 
> > > > “keeping things simple” but more “keeping things consistent“.
> > > > 
> > > > That doesn't mean that the above 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

rsandifo-arm wrote:
> simoll wrote:
> > rsandifo-arm wrote:
> > > simoll wrote:
> > > > rsandifo-arm wrote:
> > > > > simoll wrote:
> > > > > > lenary wrote:
> > > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > > > > paragraph about GCC vectors above.
> > > > > Sorry for the extremely late comment.  Like @lenary I hadn't thought 
> > > > > about this.  I'd assumed that the vector woiuld still be a multiple 
> > > > > of 8 bits in size, but I agree that's probably too restrictive to be 
> > > > > the only option available.
> > > > > 
> > > > > In that case, would it make sense to add a separate attribute 
> > > > > instead?  I think it's too surprising to change the units of the 
> > > > > existing attribute based on the element type.  Perhaps we should even 
> > > > > make it take two parameters: the total number of elements, and the 
> > > > > number of bits per element.  That might be more natural for some AVX 
> > > > > and SVE combinations.  We wouldn't need to supporrt all combinations 
> > > > > from the outset, it's just a question whether we should make the 
> > > > > syntax general enough to support it in future.
> > > > > 
> > > > > Perhaps we could do both: support `vector_size` for `bool` using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > > In that case, would it make sense to add a separate attribute 
> > > > > instead? I think it's too surprising to change the units of the 
> > > > > existing attribute based on the element type. Perhaps we should even 
> > > > > make it take two parameters: the total number of elements, and the 
> > > > > number of bits per element. That might be more natural for some AVX 
> > > > > and SVE combinations. We wouldn't need to supporrt all combinations 
> > > > > from the outset, it's just a question whether we should make the 
> > > > > syntax general enough to support it in future.
> > > > 
> > > > I guess adding a new attribute makes sense mid to long term. For now, 
> > > > i'd want something that just does the job... ie, what is proposed here. 
> > > > We should absolutely document the semantics of vector_size properly.. 
> > > > it already is counterintuitive (broken, if you ask me).
> > > > 
> > > > 
> > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > 
> > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > because these types are produced implicitly by compares of (legal) 
> > > > vector types. Consider this:
> > > > 
> > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > >   int3 A = ...;
> > > >   int3 B = ...;
> > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > vector_size(3)`-typed value.
> > > > 
> > > > 
> > > > Regarding your proposal for a separate subbyte element size and subbyte 
> > > > length, that may or may not make sense but it is surely something that 
> > > > should be discussed more broadly with its own proposal.
> > > > > Perhaps we could do both: support vector_size for bool using byte 
> > > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > > element sizes.
> > > > 
> > > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > > because these types are produced implicitly by compares of (legal) 
> > > > vector types. Consider this:
> > > > 
> > > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > > >   int3 A = ...;
> > > >   int3 B = ...;
> > > >   auto Z = A < B; // vector compare yielding a `bool 
> > > > vector_size(3)`-typed value.
> > > 
> > > Yeah, I understand the need for some way of creating subbyte vectors.  
> > > I'm just not sure using the existing `vector_size` attribute would be the 
> > > best choice for that case.  I.e. the objection wasn't based on “keeping 
> > > things simple” but more “keeping things consistent“.
> > > 
> > > That doesn't mean that the above code should be invalid.  It just means 
> > > that it wouldn't be possible to write the type of Z using the existing 
> > > `vector_size` attribute.
> > > 
> > > (FWIW, `vector_size` was 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > rsandifo-arm wrote:
> > > > simoll wrote:
> > > > > lenary wrote:
> > > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > > prominently displayed - it's something I hadn't realised before.
> > > > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > > > paragraph about GCC vectors above.
> > > > Sorry for the extremely late comment.  Like @lenary I hadn't thought 
> > > > about this.  I'd assumed that the vector woiuld still be a multiple of 
> > > > 8 bits in size, but I agree that's probably too restrictive to be the 
> > > > only option available.
> > > > 
> > > > In that case, would it make sense to add a separate attribute instead?  
> > > > I think it's too surprising to change the units of the existing 
> > > > attribute based on the element type.  Perhaps we should even make it 
> > > > take two parameters: the total number of elements, and the number of 
> > > > bits per element.  That might be more natural for some AVX and SVE 
> > > > combinations.  We wouldn't need to supporrt all combinations from the 
> > > > outset, it's just a question whether we should make the syntax general 
> > > > enough to support it in future.
> > > > 
> > > > Perhaps we could do both: support `vector_size` for `bool` using byte 
> > > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > > general attribute that allows subbyte lengths and explicit subbyte 
> > > > element sizes.
> > > > In that case, would it make sense to add a separate attribute instead? 
> > > > I think it's too surprising to change the units of the existing 
> > > > attribute based on the element type. Perhaps we should even make it 
> > > > take two parameters: the total number of elements, and the number of 
> > > > bits per element. That might be more natural for some AVX and SVE 
> > > > combinations. We wouldn't need to supporrt all combinations from the 
> > > > outset, it's just a question whether we should make the syntax general 
> > > > enough to support it in future.
> > > 
> > > I guess adding a new attribute makes sense mid to long term. For now, i'd 
> > > want something that just does the job... ie, what is proposed here. We 
> > > should absolutely document the semantics of vector_size properly.. it 
> > > already is counterintuitive (broken, if you ask me).
> > > 
> > > 
> > > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > > attribute that allows subbyte lengths and explicit subbyte element 
> > > > sizes.
> > > 
> > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > because these types are produced implicitly by compares of (legal) vector 
> > > types. Consider this:
> > > 
> > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > >   int3 A = ...;
> > >   int3 B = ...;
> > >   auto Z = A < B; // vector compare yielding a `bool 
> > > vector_size(3)`-typed value.
> > > 
> > > 
> > > Regarding your proposal for a separate subbyte element size and subbyte 
> > > length, that may or may not make sense but it is surely something that 
> > > should be discussed more broadly with its own proposal.
> > > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > > attribute that allows subbyte lengths and explicit subbyte element 
> > > > sizes.
> > > 
> > > Disallowing subbyte bool vectors actually makes this more complicated 
> > > because these types are produced implicitly by compares of (legal) vector 
> > > types. Consider this:
> > > 
> > >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> > >   int3 A = ...;
> > >   int3 B = ...;
> > >   auto Z = A < B; // vector compare yielding a `bool 
> > > vector_size(3)`-typed value.
> > 
> > Yeah, I understand the need for some way of creating subbyte vectors.  I'm 
> > just not sure using the existing `vector_size` attribute would be the best 
> > choice for that case.  I.e. the objection wasn't based on “keeping things 
> > simple” but more “keeping things consistent“.
> > 
> > That doesn't mean that the above code should be invalid.  It just means 
> > that it wouldn't be possible to write the type of Z using the existing 
> > `vector_size` attribute.
> > 
> > (FWIW, `vector_size` was originally a GNUism and GCC stil requires vector 
> > sizes to be a power of 2, but I realise that isn't relevant here.  And the 
> > same principle applies with s/3/4 in the above example 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

rsandifo-arm wrote:
> simoll wrote:
> > rsandifo-arm wrote:
> > > simoll wrote:
> > > > lenary wrote:
> > > > > It would be nice if this aside about non-bool vectors was more 
> > > > > prominently displayed - it's something I hadn't realised before.
> > > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > > paragraph about GCC vectors above.
> > > Sorry for the extremely late comment.  Like @lenary I hadn't thought 
> > > about this.  I'd assumed that the vector woiuld still be a multiple of 8 
> > > bits in size, but I agree that's probably too restrictive to be the only 
> > > option available.
> > > 
> > > In that case, would it make sense to add a separate attribute instead?  I 
> > > think it's too surprising to change the units of the existing attribute 
> > > based on the element type.  Perhaps we should even make it take two 
> > > parameters: the total number of elements, and the number of bits per 
> > > element.  That might be more natural for some AVX and SVE combinations.  
> > > We wouldn't need to supporrt all combinations from the outset, it's just 
> > > a question whether we should make the syntax general enough to support it 
> > > in future.
> > > 
> > > Perhaps we could do both: support `vector_size` for `bool` using byte 
> > > sizes (and not allowing subbyte vector lengths), and add a new, more 
> > > general attribute that allows subbyte lengths and explicit subbyte 
> > > element sizes.
> > > In that case, would it make sense to add a separate attribute instead? I 
> > > think it's too surprising to change the units of the existing attribute 
> > > based on the element type. Perhaps we should even make it take two 
> > > parameters: the total number of elements, and the number of bits per 
> > > element. That might be more natural for some AVX and SVE combinations. We 
> > > wouldn't need to supporrt all combinations from the outset, it's just a 
> > > question whether we should make the syntax general enough to support it 
> > > in future.
> > 
> > I guess adding a new attribute makes sense mid to long term. For now, i'd 
> > want something that just does the job... ie, what is proposed here. We 
> > should absolutely document the semantics of vector_size properly.. it 
> > already is counterintuitive (broken, if you ask me).
> > 
> > 
> > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > attribute that allows subbyte lengths and explicit subbyte element sizes.
> > 
> > Disallowing subbyte bool vectors actually makes this more complicated 
> > because these types are produced implicitly by compares of (legal) vector 
> > types. Consider this:
> > 
> >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> >   int3 A = ...;
> >   int3 B = ...;
> >   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed 
> > value.
> > 
> > 
> > Regarding your proposal for a separate subbyte element size and subbyte 
> > length, that may or may not make sense but it is surely something that 
> > should be discussed more broadly with its own proposal.
> > > Perhaps we could do both: support vector_size for bool using byte sizes 
> > > (and not allowing subbyte vector lengths), and add a new, more general 
> > > attribute that allows subbyte lengths and explicit subbyte element sizes.
> > 
> > Disallowing subbyte bool vectors actually makes this more complicated 
> > because these types are produced implicitly by compares of (legal) vector 
> > types. Consider this:
> > 
> >   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
> >   int3 A = ...;
> >   int3 B = ...;
> >   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed 
> > value.
> 
> Yeah, I understand the need for some way of creating subbyte vectors.  I'm 
> just not sure using the existing `vector_size` attribute would be the best 
> choice for that case.  I.e. the objection wasn't based on “keeping things 
> simple” but more “keeping things consistent“.
> 
> That doesn't mean that the above code should be invalid.  It just means that 
> it wouldn't be possible to write the type of Z using the existing 
> `vector_size` attribute.
> 
> (FWIW, `vector_size` was originally a GNUism and GCC stil requires vector 
> sizes to be a power of 2, but I realise that isn't relevant here.  And the 
> same principle applies with s/3/4 in the above example anyway.)
> 
> > Regarding your proposal for a separate subbyte element size and subbyte 
> > length, that may or may not make sense but it is surely something that 
> > should be discussed more broadly with its 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > lenary wrote:
> > > > It would be nice if this aside about non-bool vectors was more 
> > > > prominently displayed - it's something I hadn't realised before.
> > > Yep. that caught me by surprise too. I'll move that sentence to the 
> > > paragraph about GCC vectors above.
> > Sorry for the extremely late comment.  Like @lenary I hadn't thought about 
> > this.  I'd assumed that the vector woiuld still be a multiple of 8 bits in 
> > size, but I agree that's probably too restrictive to be the only option 
> > available.
> > 
> > In that case, would it make sense to add a separate attribute instead?  I 
> > think it's too surprising to change the units of the existing attribute 
> > based on the element type.  Perhaps we should even make it take two 
> > parameters: the total number of elements, and the number of bits per 
> > element.  That might be more natural for some AVX and SVE combinations.  We 
> > wouldn't need to supporrt all combinations from the outset, it's just a 
> > question whether we should make the syntax general enough to support it in 
> > future.
> > 
> > Perhaps we could do both: support `vector_size` for `bool` using byte sizes 
> > (and not allowing subbyte vector lengths), and add a new, more general 
> > attribute that allows subbyte lengths and explicit subbyte element sizes.
> > In that case, would it make sense to add a separate attribute instead? I 
> > think it's too surprising to change the units of the existing attribute 
> > based on the element type. Perhaps we should even make it take two 
> > parameters: the total number of elements, and the number of bits per 
> > element. That might be more natural for some AVX and SVE combinations. We 
> > wouldn't need to supporrt all combinations from the outset, it's just a 
> > question whether we should make the syntax general enough to support it in 
> > future.
> 
> I guess adding a new attribute makes sense mid to long term. For now, i'd 
> want something that just does the job... ie, what is proposed here. We should 
> absolutely document the semantics of vector_size properly.. it already is 
> counterintuitive (broken, if you ask me).
> 
> 
> > Perhaps we could do both: support vector_size for bool using byte sizes 
> > (and not allowing subbyte vector lengths), and add a new, more general 
> > attribute that allows subbyte lengths and explicit subbyte element sizes.
> 
> Disallowing subbyte bool vectors actually makes this more complicated because 
> these types are produced implicitly by compares of (legal) vector types. 
> Consider this:
> 
>   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
>   int3 A = ...;
>   int3 B = ...;
>   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed 
> value.
> 
> 
> Regarding your proposal for a separate subbyte element size and subbyte 
> length, that may or may not make sense but it is surely something that should 
> be discussed more broadly with its own proposal.
> > Perhaps we could do both: support vector_size for bool using byte sizes 
> > (and not allowing subbyte vector lengths), and add a new, more general 
> > attribute that allows subbyte lengths and explicit subbyte element sizes.
> 
> Disallowing subbyte bool vectors actually makes this more complicated because 
> these types are produced implicitly by compares of (legal) vector types. 
> Consider this:
> 
>   typedef int int3 __attribute__((vector_size(3 * sizeof(int;
>   int3 A = ...;
>   int3 B = ...;
>   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed 
> value.

Yeah, I understand the need for some way of creating subbyte vectors.  I'm just 
not sure using the existing `vector_size` attribute would be the best choice 
for that case.  I.e. the objection wasn't based on “keeping things simple” but 
more “keeping things consistent“.

That doesn't mean that the above code should be invalid.  It just means that it 
wouldn't be possible to write the type of Z using the existing `vector_size` 
attribute.

(FWIW, `vector_size` was originally a GNUism and GCC stil requires vector sizes 
to be a power of 2, but I realise that isn't relevant here.  And the same 
principle applies with s/3/4 in the above example anyway.)

> Regarding your proposal for a separate subbyte element size and subbyte 
> length, that may or may not make sense but it is surely something that should 
> be discussed more broadly with its own proposal.

Yeah, I agree any new attribute would need to be discussed more widely.


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

rsandifo-arm wrote:
> simoll wrote:
> > lenary wrote:
> > > It would be nice if this aside about non-bool vectors was more 
> > > prominently displayed - it's something I hadn't realised before.
> > Yep. that caught me by surprise too. I'll move that sentence to the 
> > paragraph about GCC vectors above.
> Sorry for the extremely late comment.  Like @lenary I hadn't thought about 
> this.  I'd assumed that the vector woiuld still be a multiple of 8 bits in 
> size, but I agree that's probably too restrictive to be the only option 
> available.
> 
> In that case, would it make sense to add a separate attribute instead?  I 
> think it's too surprising to change the units of the existing attribute based 
> on the element type.  Perhaps we should even make it take two parameters: the 
> total number of elements, and the number of bits per element.  That might be 
> more natural for some AVX and SVE combinations.  We wouldn't need to supporrt 
> all combinations from the outset, it's just a question whether we should make 
> the syntax general enough to support it in future.
> 
> Perhaps we could do both: support `vector_size` for `bool` using byte sizes 
> (and not allowing subbyte vector lengths), and add a new, more general 
> attribute that allows subbyte lengths and explicit subbyte element sizes.
> In that case, would it make sense to add a separate attribute instead? I 
> think it's too surprising to change the units of the existing attribute based 
> on the element type. Perhaps we should even make it take two parameters: the 
> total number of elements, and the number of bits per element. That might be 
> more natural for some AVX and SVE combinations. We wouldn't need to supporrt 
> all combinations from the outset, it's just a question whether we should make 
> the syntax general enough to support it in future.

I guess adding a new attribute makes sense mid to long term. For now, i'd want 
something that just does the job... ie, what is proposed here. We should 
absolutely document the semantics of vector_size properly.. it already is 
counterintuitive (broken, if you ask me).


> Perhaps we could do both: support vector_size for bool using byte sizes (and 
> not allowing subbyte vector lengths), and add a new, more general attribute 
> that allows subbyte lengths and explicit subbyte element sizes.

Disallowing subbyte bool vectors actually makes this more complicated because 
these types are produced implicitly by compares of (legal) vector types. 
Consider this:

  typedef int int3 __attribute__((vector_size(3 * sizeof(int;
  int3 A = ...;
  int3 B = ...;
  auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed 
value.


Regarding your proposal for a separate subbyte element size and subbyte length, 
that may or may not make sense but it is surely something that should be 
discussed more broadly with its own proposal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a subscriber: lenary.
rsandifo-arm added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

simoll wrote:
> lenary wrote:
> > It would be nice if this aside about non-bool vectors was more prominently 
> > displayed - it's something I hadn't realised before.
> Yep. that caught me by surprise too. I'll move that sentence to the paragraph 
> about GCC vectors above.
Sorry for the extremely late comment.  Like @lenary I hadn't thought about 
this.  I'd assumed that the vector woiuld still be a multiple of 8 bits in 
size, but I agree that's probably too restrictive to be the only option 
available.

In that case, would it make sense to add a separate attribute instead?  I think 
it's too surprising to change the units of the existing attribute based on the 
element type.  Perhaps we should even make it take two parameters: the total 
number of elements, and the number of bits per element.  That might be more 
natural for some AVX and SVE combinations.  We wouldn't need to supporrt all 
combinations from the outset, it's just a question whether we should make the 
syntax general enough to support it in future.

Perhaps we could do both: support `vector_size` for `bool` using byte sizes 
(and not allowing subbyte vector lengths), and add a new, more general 
attribute that allows subbyte lengths and explicit subbyte element sizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 added a comment.

Thank you for implementing `EmitFromMemory`.  We are locally trying to use this 
patch to implement vector mask intrinsic instructions on Aurora VE.  It is 
working well with regression tests.  We will try test-suite next.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 281878.
simoll added a comment.

NFC

- Style updates
- Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto T = CmpMul(a, b);
   // CHECK: store <4 x i8> 
Index: clang/lib/Sema/SemaType.cpp

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 281844.
simoll added a comment.

Updated clang test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/constexpr-vectors.cpp
===
--- clang/test/SemaCXX/constexpr-vectors.cpp
+++ clang/test/SemaCXX/constexpr-vectors.cpp
@@ -204,35 +204,35 @@
 
   constexpr auto w = FourCharsVecSize{1, 2, 3, 4} <
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto x = FourCharsVecSize{1, 2, 3, 4} >
  FourCharsVecSize{4, 3, 2, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto y = FourCharsVecSize{1, 2, 3, 4} <=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto z = FourCharsVecSize{1, 2, 3, 4} >=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto A = FourCharsVecSize{1, 2, 3, 4} ==
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto B = FourCharsVecSize{1, 2, 3, 4} !=
  FourCharsVecSize{4, 3, 3, 1};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto C = FourCharsVecSize{1, 2, 3, 4} < 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto D = FourCharsVecSize{1, 2, 3, 4} > 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto E = FourCharsVecSize{1, 2, 3, 4} <= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto F = FourCharsVecSize{1, 2, 3, 4} >= 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto G = FourCharsVecSize{1, 2, 3, 4} == 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto H = FourCharsVecSize{1, 2, 3, 4} != 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto I = FourCharsVecSize{1, 2, 3, 4} &
  FourCharsVecSize{4, 3, 2, 1};
@@ -252,15 +252,15 @@
 
   constexpr auto O = FourCharsVecSize{5, 0, 6, 0} &&
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto P = FourCharsVecSize{5, 0, 6, 0} ||
  FourCharsVecSize{5, 5, 0, 0};
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto Q = FourCharsVecSize{5, 0, 6, 0} && 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
   constexpr auto R = FourCharsVecSize{5, 0, 6, 0} || 3;
-  // CHECK: store <4 x i8> 
+  // CHECK: store i8 bitcast (<8 x i1>  to i8)
 
   constexpr auto T = CmpMul(a, b);
   // CHECK: store <4 x i8> 
Index: clang/lib/Sema/SemaType.cpp

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 281322.
simoll added a comment.

- Rebased
- Implement `EmitFromMemory`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2464,10 +2464,10 @@
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
SourceLocation AttrLoc) {
-  // The base type must be integer (not Boolean or enumeration) or float, and
+  // The base type must be boolean or integer (not enumeration) or float, and
   // can't already be a vector.
   if (!CurType->isDependentType() &&
-  (!CurType->isBuiltinType() || CurType->isBooleanType() ||
+  (!CurType->isBuiltinType() ||
(!CurType->isIntegerType() && !CurType->isRealFloatingType( {
 Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
 return QualType();
@@ -2496,8 +2496,15 @@
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize->getZExtValue() * 8;
-  unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
+
+  uint64_t VecNumElems = VecSize->getZExtValue();
+  uint64_t VectorSizeBits =
+  CurType->isBooleanType()
+  ? VecNumElems
+  : VecNumElems * 8; // FIXME "bitsof(CharUnit)"
+  unsigned TypeSize = CurType->isBooleanType()
+  ? 1
+  : static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
 Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -7557,13 +7564,13 @@
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// HandleVectorSizeAttribute - this attribute is only applicable to integral
-/// and float scalars, although arrays, pointers, and function return values are
-/// allowed in conjunction with this construct. Aggregates with this attribute
-/// are invalid, even if they are of the same size as a corresponding scalar.
-/// The raw attribute should contain precisely 1 argument, the vector size for
-/// the variable, measured in bytes. If curType and rawAttr are well formed,
-/// this routine will return a new vector type.
+/// HandleVectorSizeAttribute - this attribute is only applicable to boolean,
+/// integral and float scalars, although arrays, pointers, and function return
+/// values are allowed in conjunction with this construct. Aggregates with this
+/// attribute are invalid, even if they are of the same size as a corresponding
+/// scalar. The raw attribute should contain precisely 1 argument, the vector
+/// size for the variable, measured in bytes. If curType and rawAttr are well
+/// formed, this routine will return a new vector type.
 static void HandleVectorSizeAttr(QualType , const ParsedAttr ,
  Sema ) {
   // Check the attribute arguments.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5946,7 +5946,8 @@
   } else if (LHSVT || RHSVT) {
 ResultType = CheckVectorOperands(
 LHS, RHS, QuestionLoc, /*isCompAssign*/ 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 268488.
simoll added a comment.

- Improved documentation for the size argument of vector_size.
- Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2464,10 +2464,10 @@
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
SourceLocation AttrLoc) {
-  // The base type must be integer (not Boolean or enumeration) or float, and
+  // The base type must be boolean or integer (not enumeration) or float, and
   // can't already be a vector.
   if (!CurType->isDependentType() &&
-  (!CurType->isBuiltinType() || CurType->isBooleanType() ||
+  (!CurType->isBuiltinType() ||
(!CurType->isIntegerType() && !CurType->isRealFloatingType( {
 Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
 return QualType();
@@ -2496,8 +2496,14 @@
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize.getZExtValue() * 8;
-  unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
+
+  uint64_t VectorSizeBits =
+  CurType->isBooleanType()
+  ? VecSize.getZExtValue()
+  : VecSize.getZExtValue() * 8; // FIXME "bitsof(CharUnit)"
+  unsigned TypeSize = CurType->isBooleanType()
+  ? 1
+  : static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
 Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -7549,13 +7555,13 @@
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// HandleVectorSizeAttribute - this attribute is only applicable to integral
-/// and float scalars, although arrays, pointers, and function return values are
-/// allowed in conjunction with this construct. Aggregates with this attribute
-/// are invalid, even if they are of the same size as a corresponding scalar.
-/// The raw attribute should contain precisely 1 argument, the vector size for
-/// the variable, measured in bytes. If curType and rawAttr are well formed,
-/// this routine will return a new vector type.
+/// HandleVectorSizeAttribute - this attribute is only applicable to boolean,
+/// integral and float scalars, although arrays, pointers, and function return
+/// values are allowed in conjunction with this construct. Aggregates with this
+/// attribute are invalid, even if they are of the same size as a corresponding
+/// scalar. The raw attribute should contain precisely 1 argument, the vector
+/// size for the variable, measured in bytes. If curType and rawAttr are well
+/// formed, this routine will return a new vector type.
 static void HandleVectorSizeAttr(QualType , const ParsedAttr ,
  Sema ) {
   // Check the attribute arguments.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5947,7 +5947,8 @@
   } else if (LHSVT || RHSVT) {
 ResultType = CheckVectorOperands(
 LHS, RHS, QuestionLoc, 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll marked an inline comment as done.
simoll added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

lenary wrote:
> It would be nice if this aside about non-bool vectors was more prominently 
> displayed - it's something I hadn't realised before.
Yep. that caught me by surprise too. I'll move that sentence to the paragraph 
about GCC vectors above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083



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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).

It would be nice if this aside about non-bool vectors was more prominently 
displayed - it's something I hadn't realised before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083



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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll marked 2 inline comments as done.
simoll added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4710
 if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  auto *Vec4Ty = llvm::FixedVectorType::get(
+  auto Vec4Ty = llvm::VectorType::get(
   cast(DstTy)->getElementType(), 4);

Accidental change. Will undo.



Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:590
+  CharUnits Tail =
+  getSize(Prior->Data); // FIXME assumes `i8` multiples for boolean vector!
   for (std::vector::iterator Member = Prior + 1,

This is an old comment that is no longer helpful. Will remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083



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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 268140.
simoll added a comment.

NFC. Undid stray change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2464,10 +2464,10 @@
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
SourceLocation AttrLoc) {
-  // The base type must be integer (not Boolean or enumeration) or float, and
+  // The base type must be boolean or integer (not enumeration) or float, and
   // can't already be a vector.
   if (!CurType->isDependentType() &&
-  (!CurType->isBuiltinType() || CurType->isBooleanType() ||
+  (!CurType->isBuiltinType() ||
(!CurType->isIntegerType() && !CurType->isRealFloatingType( {
 Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
 return QualType();
@@ -2496,8 +2496,14 @@
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize.getZExtValue() * 8;
-  unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
+
+  uint64_t VectorSizeBits =
+  CurType->isBooleanType()
+  ? VecSize.getZExtValue()
+  : VecSize.getZExtValue() * 8; // FIXME "bitsof(CharUnit)"
+  unsigned TypeSize = CurType->isBooleanType()
+  ? 1
+  : static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
 Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -7549,13 +7555,13 @@
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// HandleVectorSizeAttribute - this attribute is only applicable to integral
-/// and float scalars, although arrays, pointers, and function return values are
-/// allowed in conjunction with this construct. Aggregates with this attribute
-/// are invalid, even if they are of the same size as a corresponding scalar.
-/// The raw attribute should contain precisely 1 argument, the vector size for
-/// the variable, measured in bytes. If curType and rawAttr are well formed,
-/// this routine will return a new vector type.
+/// HandleVectorSizeAttribute - this attribute is only applicable to boolean,
+/// integral and float scalars, although arrays, pointers, and function return
+/// values are allowed in conjunction with this construct. Aggregates with this
+/// attribute are invalid, even if they are of the same size as a corresponding
+/// scalar. The raw attribute should contain precisely 1 argument, the vector
+/// size for the variable, measured in bytes. If curType and rawAttr are well
+/// formed, this routine will return a new vector type.
 static void HandleVectorSizeAttr(QualType , const ParsedAttr ,
  Sema ) {
   // Check the attribute arguments.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5947,7 +5947,8 @@
   } else if (LHSVT || RHSVT) {
 ResultType = CheckVectorOperands(
 LHS, RHS, QuestionLoc, 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll created this revision.
simoll added reviewers: hfinkel, erichkeane, craig.topper, rsandifo-arm, kaz7, 
k-ishizaka.
simoll added a project: clang.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a reviewer: rengolin.
simoll updated this revision to Diff 268140.
simoll added a comment.

NFC. Undid stray change.


This patch extends Clang to allow 'bool' as a valid vector element type 
(attribute `vector_size`).

This is the natural type for SIMD masks and facilitates clean vector intrinsic 
declarations.
Vectors of i1 are supported on IR level and below down to many SIMD ISAs, such 
as AVX512, ARM SVE (fixed vector length) and the VE target (NEC SX-Aurora 
TSUBASA).

The RFC on cfe-dev: 
https://lists.llvm.org/pipermail/cfe-dev/2020-May/065434.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2464,10 +2464,10 @@
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
SourceLocation AttrLoc) {
-  // The base type must be integer (not Boolean or enumeration) or float, and
+  // The base type must be boolean or integer (not enumeration) or float, and
   // can't already be a vector.
   if (!CurType->isDependentType() &&
-  (!CurType->isBuiltinType() || CurType->isBooleanType() ||
+  (!CurType->isBuiltinType() ||
(!CurType->isIntegerType() && !CurType->isRealFloatingType( {
 Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
 return QualType();
@@ -2496,8 +2496,14 @@
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize.getZExtValue() * 8;
-  unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
+
+  uint64_t VectorSizeBits =
+  CurType->isBooleanType()
+  ? VecSize.getZExtValue()
+  : VecSize.getZExtValue() * 8; // FIXME "bitsof(CharUnit)"
+  unsigned TypeSize = CurType->isBooleanType()
+  ? 1
+  : static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
 Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -7549,13 +7555,13 @@
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// HandleVectorSizeAttribute - this attribute is only applicable to integral
-/// and float scalars, although arrays, pointers, and function return values are
-/// allowed in conjunction with this construct. Aggregates with this attribute
-/// are invalid, even if they are of the same size as a corresponding scalar.
-/// The raw attribute should contain precisely 1 argument, the vector size for
-/// the variable, measured in bytes. If curType and rawAttr are well formed,
-/// this routine will return a new vector type.
+/// HandleVectorSizeAttribute - this attribute is only applicable to boolean,
+/// integral and float scalars, although arrays, pointers, and function return
+/// values are allowed in conjunction with this construct. Aggregates with this
+/// attribute are invalid, even if they are of the same size as a corresponding
+/// scalar. The raw attribute should contain