[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG349636d2bfc3: Implement VectorType conditional operator GNU 
extension. (authored by erichkeane).

Changed prior to commit:
  https://reviews.llvm.org/D71463?vs=234977=237768#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71463

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/SemaCXX/vector-conditional.cpp

Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using TwoUInts = unsigned __attribute__((__vector_size__(8)));
+using FourInts = int __attribute__((__vector_size__(16)));
+using FourUInts = unsigned __attribute__((__vector_size__(16)));
+using TwoLongLong = long long __attribute__((__vector_size__(16)));
+using FourLongLong = long long __attribute__((__vector_size__(32)));
+using TwoFloats = float __attribute__((__vector_size__(8)));
+using FourFloats = float __attribute__((__vector_size__(16)));
+using TwoDoubles = double __attribute__((__vector_size__(16)));
+using FourDoubles = double __attribute__((__vector_size__(32)));
+
+FourShorts four_shorts;
+TwoInts two_ints;
+TwoUInts two_uints;
+FourInts four_ints;
+FourUInts four_uints;
+TwoLongLong two_ll;
+FourLongLong four_ll;
+TwoFloats two_floats;
+FourFloats four_floats;
+TwoDoubles two_doubles;
+FourDoubles four_doubles;
+
+enum E {};
+enum class SE {};
+E e;
+SE se;
+
+// Check the rules of the condition of the conditional operator.
+void Condition() {
+  // Only int types are allowed here, the rest should fail to convert to bool.
+  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+}
+
+// Check the rules of the LHS/RHS of the conditional operator.
+void Operands() {
+  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
+  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
+
+  // Vector types must be the same element size as the condition.
+  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do not have the same number of elements}}
+  (void)(four_ints ? four_ll : four_ll);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourLongLong' (vector of 4 'long long' values) do not have elements of the same size}}
+  (void)(four_ints ? two_doubles : two_doubles);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoDoubles' (vector of 2 'double' values) do not have the same number of elements}}
+  (void)(four_ints ? four_doubles : four_doubles); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourDoubles' (vector of 4 'double' values) do not have elements of the same size}}
+  (void)(four_ints ?: two_ints);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'TwoInts' (vector of 2 'int' values)}}
+  (void)(four_ints ?: four_doubles);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'FourDoubles' (vector of 4 'double' values)}}
+
+  // Scalars are promoted, but must be the same element size.
+  (void)(four_ints ? 3.0f : 3.0); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type '__attribute__((__vector_size__(4 * sizeof(double double' 

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 2 inline comments as done.
erichkeane added a comment.

In D71463#1797436 , @Anastasia wrote:

> It looks good, I was just thinking whether it would be possible to share more 
> common infrastructure. There is `Sema::CheckVectorOperands` that 
> corresponding OpenCL methods are using internally. Do you think it is 
> possible to share the code more?


I tried in my initial implementation.  Unfortunately CheckVectorOperands and 
the OpenCL vectors have significantly different semantic rules from this, these 
aren't even consistent with the normal GCC vector conversion rules. The 
similarities/differences between all of these are frustrating, any attempt I 
made at unifying the logic made both really difficult to follow.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6880
+def err_conditional_vector_has_void : Error<
+  "GNU vector conditional operand cannot be %select{void|a throw 
expression}0">;
+def err_conditional_vector_operand_type

Anastasia wrote:
> Would this only apply to GNU extension? I am just wondering if this can be 
> generalized.
To be honest, I limited the scope of this a bit since the ext_vector type isn't 
something I have a great knowledge of.  The OpenCL vector types end up having 
frustratingly similar/different behavior that it required significant 
differences in logic.

Does OpenCL prohibit void/throw expressions in their vector conditionals?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5797
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

Anastasia wrote:
> Don't we already have a diagnostic for this used in OpenCL?
The one I found was less specific than just 'mismatched'.  It ended up 
containing info about not convertible.  I tried a set of 'selects' on it but it 
ended up getting hacked up more than I was comfortable with.  Unless you're 
referring to a different one that I missed?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

It looks good, I was just thinking whether it would be possible to share more 
common infrastructure. There is `Sema::CheckVectorOperands` that corresponding 
OpenCL methods are using internally. Do you think it is possible to share the 
code more?




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6880
+def err_conditional_vector_has_void : Error<
+  "GNU vector conditional operand cannot be %select{void|a throw 
expression}0">;
+def err_conditional_vector_operand_type

Would this only apply to GNU extension? I am just wondering if this can be 
generalized.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5797
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

Don't we already have a diagnostic for this used in OpenCL?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

@eli.friedman @rjmccall Do you have further comments on these?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 234977.
erichkeane marked 4 inline comments as done.
erichkeane added a comment.

@aaron.ballman s comments done.


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

https://reviews.llvm.org/D71463

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/SemaCXX/vector-conditional.cpp

Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using TwoUInts = unsigned __attribute__((__vector_size__(8)));
+using FourInts = int __attribute__((__vector_size__(16)));
+using FourUInts = unsigned __attribute__((__vector_size__(16)));
+using TwoLongLong = long long __attribute__((__vector_size__(16)));
+using FourLongLong = long long __attribute__((__vector_size__(32)));
+using TwoFloats = float __attribute__((__vector_size__(8)));
+using FourFloats = float __attribute__((__vector_size__(16)));
+using TwoDoubles = double __attribute__((__vector_size__(16)));
+using FourDoubles = double __attribute__((__vector_size__(32)));
+
+FourShorts four_shorts;
+TwoInts two_ints;
+TwoUInts two_uints;
+FourInts four_ints;
+FourUInts four_uints;
+TwoLongLong two_ll;
+FourLongLong four_ll;
+TwoFloats two_floats;
+FourFloats four_floats;
+TwoDoubles two_doubles;
+FourDoubles four_doubles;
+
+enum E {};
+enum class SE {};
+E e;
+SE se;
+
+// Check the rules of the condition of the conditional operator.
+void Condition() {
+  // Only int types are allowed here, the rest should fail to convert to bool.
+  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+}
+
+// Check the rules of the LHS/RHS of the conditional operator.
+void Operands() {
+  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
+  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
+
+  // Vector types must be the same element size as the condition.
+  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do not have the same number of elements}}
+  (void)(four_ints ? four_ll : four_ll);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourLongLong' (vector of 4 'long long' values) do not have elements of the same size}}
+  (void)(four_ints ? two_doubles : two_doubles);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoDoubles' (vector of 2 'double' values) do not have the same number of elements}}
+  (void)(four_ints ? four_doubles : four_doubles); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourDoubles' (vector of 4 'double' values) do not have elements of the same size}}
+  (void)(four_ints ?: two_ints);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'TwoInts' (vector of 2 'int' values)}}
+  (void)(four_ints ?: four_doubles);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'FourDoubles' (vector of 4 'double' values)}}
+
+  // Scalars are promoted, but must be the same element size.
+  (void)(four_ints ? 3.0f : 3.0); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values) do not have elements of the same size}}
+  (void)(four_ints ? 5ll : 5);// expected-error {{vector condition type 'FourInts' 

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 13 inline comments as done.
erichkeane added a comment.

New patch coming as soon as my build finishes.




Comment at: clang/docs/LanguageExtensions.rst:480
 =yes yes yes yes
-:?   yes --  --  --
+:?[*]_  yes --  yes --
 sizeof   yes yes yes yes

aaron.ballman wrote:
> Do these columns line up in the actual source file? They don't appear to line 
> up in the Phab viewer, but I don't know if that's an artifact of Phab or not.
They don't, I had misread the sphinx doc it seems and was doing footnotes 
incorrectly.  A coworker shared the live-sphinx viewer so I corrected this.  
See next patch.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4301
+llvm::Type *condType = ConvertType(condExpr->getType());
+llvm::VectorType *vecTy = cast(condType);
+llvm::Value *zeroVec = llvm::Constant::getNullValue(vecTy);

aaron.ballman wrote:
> `auto *`
> 
> Also, should these three variables have identifiers starting with an 
> uppercase like `CondV` et al?
Probably, I just C'ed this code from the above group :/



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5757
+  const QualType EltTy =
+  cast(CondTy.getCanonicalType().getTypePtr())
+  ->getElementType();

aaron.ballman wrote:
> Do you actually need the `getTypePtr()` bit, or will the `cast<>` suffice to 
> trigger the cast through `Type*`?
TIL!



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5793
+Diag(QuestionLoc, diag::err_conditional_vector_operand_type)
+<< /*isExtVector*/ true << RHSType;
+return {};

aaron.ballman wrote:
> Shouldn't this pass in the `LHSType` instead because that's what's being 
> tested?
Yep!



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5805
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

aaron.ballman wrote:
> Do you need to canonicalize these types before comparing them?
No, both versions of this function canonicalize for me: 
https://clang.llvm.org/doxygen/ASTContext_8h_source.html#l02305


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:480
 =yes yes yes yes
-:?   yes --  --  --
+:?[*]_  yes --  yes --
 sizeof   yes yes yes yes

Do these columns line up in the actual source file? They don't appear to line 
up in the Phab viewer, but I don't know if that's an artifact of Phab or not.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6882
+def err_conditional_vector_operand_type
+: Error<"%select{enumeral|extended vector}0 type %1 is not allowed in a "
+"vector conditional">;

We don't actually use enumeral in diagnostics anywhere -- I think that should 
be enumeration instead.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4301
+llvm::Type *condType = ConvertType(condExpr->getType());
+llvm::VectorType *vecTy = cast(condType);
+llvm::Value *zeroVec = llvm::Constant::getNullValue(vecTy);

`auto *`

Also, should these three variables have identifiers starting with an uppercase 
like `CondV` et al?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5757
+  const QualType EltTy =
+  cast(CondTy.getCanonicalType().getTypePtr())
+  ->getElementType();

Do you actually need the `getTypePtr()` bit, or will the `cast<>` suffice to 
trigger the cast through `Type*`?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5761-5762
+  // isIntegralType doesn't exactly match the the needs here, but would 
function
+  // well enough.  The boolean check is redundant since a vector of type bool
+  // isn't allowed, and the enumeral type check is redundant because
+  // isIntegralType isn't true in 'C++' mode.  Additionally, enumeral types

If these checks are redundant, would you prefer to assert them?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5779
+  QualType CondType = Cond.get()->getType();
+  const VectorType *CondVT = CondType->getAs();
+  QualType CondElementTy = CondVT->getElementType();

`const auto *` (same elsewhere in the function where the type is spelled out in 
the initialization)



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5793
+Diag(QuestionLoc, diag::err_conditional_vector_operand_type)
+<< /*isExtVector*/ true << RHSType;
+return {};

Shouldn't this pass in the `LHSType` instead because that's what's being tested?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5799
+Diag(QuestionLoc, diag::err_conditional_vector_operand_type)
+<< /*isExtVector*/ true << LHSType;
+return {};

And this one do the `RHSType`?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5805
+// If both are vector types, they must be the same type.
+if (!Context.hasSameType(LHSType, RHSType)) {
+  Diag(QuestionLoc, diag::err_conditional_vector_mismatched_vectors)

Do you need to canonicalize these types before comparing them?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping :)


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 233861.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

Rebase + @eli.friedman s comments.


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

https://reviews.llvm.org/D71463

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/SemaCXX/vector-conditional.cpp

Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using TwoUInts = unsigned __attribute__((__vector_size__(8)));
+using FourInts = int __attribute__((__vector_size__(16)));
+using FourUInts = unsigned __attribute__((__vector_size__(16)));
+using TwoLongLong = long long __attribute__((__vector_size__(16)));
+using FourLongLong = long long __attribute__((__vector_size__(32)));
+using TwoFloats = float __attribute__((__vector_size__(8)));
+using FourFloats = float __attribute__((__vector_size__(16)));
+using TwoDoubles = double __attribute__((__vector_size__(16)));
+using FourDoubles = double __attribute__((__vector_size__(32)));
+
+FourShorts four_shorts;
+TwoInts two_ints;
+TwoUInts two_uints;
+FourInts four_ints;
+FourUInts four_uints;
+TwoLongLong two_ll;
+FourLongLong four_ll;
+TwoFloats two_floats;
+FourFloats four_floats;
+TwoDoubles two_doubles;
+FourDoubles four_doubles;
+
+enum E {};
+enum class SE {};
+E e;
+SE se;
+
+// Check the rules of the condition of the conditional operator.
+void Condition() {
+  // Only int types are allowed here, the rest should fail to convert to bool.
+  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+}
+
+// Check the rules of the LHS/RHS of the conditional operator.
+void Operands() {
+  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
+  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
+
+  // Vector types must be the same element size as the condition.
+  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do not have the same number of elements}}
+  (void)(four_ints ? four_ll : four_ll);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourLongLong' (vector of 4 'long long' values) do not have elements of the same size}}
+  (void)(four_ints ? two_doubles : two_doubles);   // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoDoubles' (vector of 2 'double' values) do not have the same number of elements}}
+  (void)(four_ints ? four_doubles : four_doubles); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'FourDoubles' (vector of 4 'double' values) do not have elements of the same size}}
+  (void)(four_ints ?: two_ints);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'TwoInts' (vector of 2 'int' values)}}
+  (void)(four_ints ?: four_doubles);   // expected-error {{vector operands to the vector conditional must be the same type ('FourInts' (vector of 4 'int' values) and 'FourDoubles' (vector of 4 'double' values)}}
+
+  // Scalars are promoted, but must be the same element size.
+  (void)(four_ints ? 3.0f : 3.0); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values) do not have elements of the same size}}
+  (void)(four_ints ? 5ll : 5);// expected-error {{vector condition type 

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 5 inline comments as done.
erichkeane added a comment.

In D71463#1783953 , @efriedma wrote:

> Can you update 
> https://clang.llvm.org/docs/LanguageExtensions.html#langext-vectors with a 
> description of the interaction between the different language features here?


Done, see the next patch.

> It's unfortunate that the gcc extension is explicitly incompatible with 
> OpenCL, but I guess we're stuck with it.

I think it is the other way around actually.  The GCC extension predates the 
OpenCL one from what I can tell.




Comment at: clang/include/clang/AST/Expr.h:3738
+// implementation of this that the standard implies that this
+// could be true in the C++ standard as well.
+(cond->isTypeDependent() || lhs->isTypeDependent() ||

efriedma wrote:
> You can just refer to [temp.dep.expr] here; the type does in fact "depend" on 
> the type of the condition.  I'm a little concerned that the C++ standard 
> might not preserve this guarantee in the future.
I wasn't able to find a bug report in the core issues list, and this has been 
known about by Doug for ~15 years now.  I'm somewhat less concerned, 
particularly since GCC has this issue as well.



Comment at: clang/lib/Sema/SemaExpr.cpp:8981
+else
+  return true;
   } else if (VectorEltTy->isRealFloatingType()) {

efriedma wrote:
> Can you separate this out into an independent patch?
348f22eac83d9a3ee946e41be43fe507f04a89b6



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5826
+else
+  ResultElementTy = UsualArithmeticConversions(LHS, RHS);
+

efriedma wrote:
> I'm not completely sure this works the way you want it to... in particular, 
> if the types don't match, and both operands can be promoted to int, you end 
> up with an int vector.
I think I'm OK with that... GCC does allow some promotions here (a short and a 
ushort act as an int).  I tried a bunch of cases and couldn't come up with a 
case where this didn't work in the 2 scalar versions.  You wouldn't happen to 
have something in mind, would you?


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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Can you update 
https://clang.llvm.org/docs/LanguageExtensions.html#langext-vectors with a 
description of the interaction between the different language features here?

It's unfortunate that the gcc extension is explicitly incompatible with OpenCL, 
but I guess we're stuck with it.




Comment at: clang/include/clang/AST/Expr.h:3738
+// implementation of this that the standard implies that this
+// could be true in the C++ standard as well.
+(cond->isTypeDependent() || lhs->isTypeDependent() ||

You can just refer to [temp.dep.expr] here; the type does in fact "depend" on 
the type of the condition.  I'm a little concerned that the C++ standard might 
not preserve this guarantee in the future.



Comment at: clang/lib/Sema/SemaExpr.cpp:8981
+else
+  return true;
   } else if (VectorEltTy->isRealFloatingType()) {

Can you separate this out into an independent patch?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5826
+else
+  ResultElementTy = UsualArithmeticConversions(LHS, RHS);
+

I'm not completely sure this works the way you want it to... in particular, if 
the types don't match, and both operands can be promoted to int, you end up 
with an int vector.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71463



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

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: eli.friedman, aaron.ballman, echristo, rjmccall.
Herald added a subscriber: Anastasia.
Herald added a project: clang.
erichkeane added a comment.

Sorry in advance for the reviewer choices... Blame got me you all for all 
touching these things in the early 2010s.  Since then, Richard and I seem to be 
the only one to have touched these types.


GCC supports the conditional operator on VectorTypes that acts as a
'select' in C++ mode. This patch implements the support. Types are
converted as closely to GCC's behavior as possible, though in a few
places consistency with our existing vector type support was preferred.

Note that this implementation is different from the OpenCL version in a
number of ways, so it unfortunately required a different implementation.

First, the SEMA rules and promotion rules are significantly different.

Secondly, GCC implements COND[i] != 0 ? LHS[i] : RHS[i] (where i is in
the range 0- VectorSize, for each element).  In OpenCL, the condition is
COND[i] < 0 ? LHS[i]: RHS[i].

In the process of implementing this, it was also required to make the
expression COND ? LHS : RHS type dependent if COND is type dependent,
since the type is now dependent on the condition.  For example:

  T ? 1 : 2;

Is not typically type dependent, since the result can be deduced from
the operands.  HOWEVER, if T is a VectorType now, it could change this
to a 'select' (basically a swizzle with a non-constant mask) with the 1
and 2 being promoted to vectors themselves.

While this is a change, it is NOT a standards incompatible change. Based
on my (and D. Gregor's, at the time of writing the code) reading of the
standard, the expression is supposed to be type dependent if ANY
sub-expression is type dependent.


Repository:
  rC Clang

https://reviews.llvm.org/D71463

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/Sema/vector-gcc-compat.cpp
  clang/test/SemaCXX/vector-conditional.cpp

Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -fsyntax-only -verify -fexceptions -fcxx-exceptions %s -std=c++17
+// Note that this test depends on the size of long-long to be different from
+// int, so it specifies a triple.
+
+using FourShorts = short __attribute__((__vector_size__(8)));
+using TwoInts = int __attribute__((__vector_size__(8)));
+using TwoUInts = unsigned __attribute__((__vector_size__(8)));
+using FourInts = int __attribute__((__vector_size__(16)));
+using FourUInts = unsigned __attribute__((__vector_size__(16)));
+using TwoLongLong = long long __attribute__((__vector_size__(16)));
+using FourLongLong = long long __attribute__((__vector_size__(32)));
+using TwoFloats = float __attribute__((__vector_size__(8)));
+using FourFloats = float __attribute__((__vector_size__(16)));
+using TwoDoubles = double __attribute__((__vector_size__(16)));
+using FourDoubles = double __attribute__((__vector_size__(32)));
+
+FourShorts four_shorts;
+TwoInts two_ints;
+TwoUInts two_uints;
+FourInts four_ints;
+FourUInts four_uints;
+TwoLongLong two_ll;
+FourLongLong four_ll;
+TwoFloats two_floats;
+FourFloats four_floats;
+TwoDoubles two_doubles;
+FourDoubles four_doubles;
+
+enum E {};
+enum class SE {};
+E e;
+SE se;
+
+// Check the rules of the condition of the conditional operator.
+void Condition() {
+  // Only int types are allowed here, the rest should fail to convert to bool.
+  (void)(four_floats ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+  (void)(two_doubles ? 1 : 1); // expected-error {{is not contextually convertible to 'bool'}}}
+}
+
+// Check the rules of the LHS/RHS of the conditional operator.
+void Operands() {
+  (void)(four_ints ? four_ints : throw 1); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? throw 1 : four_ints); // expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ?: throw 1);// expected-error {{GNU vector conditional operand cannot be a throw expression}}
+  (void)(four_ints ? (void)1 : four_ints); // expected-error {{GNU vector conditional operand cannot be void}}
+  (void)(four_ints ?: (void)1);// expected-error {{GNU vector conditional operand cannot be void}}
+
+  // Vector types must be the same element size as the condition.
+  (void)(four_ints ? two_ll : two_ll); // expected-error {{vector condition type 'FourInts' (vector of 4 'int' values) and result type 'TwoLongLong' (vector of 2 'long long' values) do 

[PATCH] D71463: Implement VectorType conditional operator GNU extension.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Sorry in advance for the reviewer choices... Blame got me you all for all 
touching these things in the early 2010s.  Since then, Richard and I seem to be 
the only one to have touched these types.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71463



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