[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 <https://reviews.llvm.org/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