myhsu marked 2 inline comments as done.
myhsu 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();
----------------
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


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7975
+    // condition and result type.
+    QualType CondTy = Cond.get()->getType();
+    if (CondTy->isExtVectorType()) {
----------------
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`


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

https://reviews.llvm.org/D80574



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

Reply via email to