[PATCH] D80574: [ExtVector] Support ExtVectorType conditional operator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7484
+  // Only ext vector is allowed
+  if (const auto *VecCondTy = Cond->getType()->getAs<ExtVectorType>()) {
+    QualType EleTy = VecCondTy->getElementType();
----------------
myhsu wrote:
> Anastasia wrote:
> > Why do you need this change? I believe OpenCL makes the same restriction.
> I'm not quite sure what restriction you're referring here. If you're saying OpenCL only allow ext_vector_type on condition operand, I don't think that's the case since on line 7956 ~ 7957 of Sema/SemaExpr.cpp :
> 
> ```
> if (getLangOpts().OpenCL && Cond.get()->getType()->isVectorType())
>     return OpenCLCheckVectorConditional(*this, Cond, LHS, RHS, QuestionLoc);
> ```
> `isVectorType()` will be true for both GNU vector and ext_vector_type.
> 
> Regarding why i add the change here rather than merging with OpenCL's flow (which is related to another comments below), OpenCL's flow diverge into line 7957, as shown above, pretty early in the entire checking flow. And `OpenCLCheckVectorConditional` do other additional works. Most notably, promoting all scalar operands into vector if condition is a vector. I'm not sure if we should bring this feature to ext_vector
> I'm not quite sure what restriction you're referring here. 

You are restricting the use non-integer vectors in condition, is it not?



> I'm not sure if we should bring this feature to ext_vector.

So what is it that you are trying to implement? My initial understanding was that you are just enabling behavior from OpenCL vectors to work in non-OpenCL mode. But you need to deviate from it?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7975
+    // condition and result type.
+    QualType CondTy = Cond.get()->getType();
+    if (CondTy->isExtVectorType()) {
----------------
myhsu wrote:
> Anastasia wrote:
> > Do you know where it is done for OpenCL? I think we should try to share the same code...
> It's inside `OpenCLCheckVectorConditional` function. As I mentioned in earlier comment, as long as we agree to bring every OpenCL features/rules regarding conditional vectors to ext_vector_type, I'm happy to reuse `OpenCLCheckVectorConditional`
I don't know... I have no preference tbh. I think following OpenCL rules might be simpler from implementation and documentation perspective. But is it what you need?


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

https://reviews.llvm.org/D80574