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

Min-Yih Hsu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 29 09:46:01 PDT 2020


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:
> 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?
> My initial understanding was that you are just enabling behavior from OpenCL vectors to work in non-OpenCL mode

My initial thoughts was to bring, and only bring, OpenCL-style condition operands to ext_vector. Since that's the key to enable some hardware accelerations like the Intel AVX example I put in the original proposal.

But maybe I'm too cautious about introducing other OpenCL vector properties to ext_vector. After crafting some test cases, I found that there is little to no impact on simply using full set of OpenCL vector's semantic rules. So I'll update this patch to unify the checks between OpenCL vectors and ext_vector.


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

https://reviews.llvm.org/D80574





More information about the cfe-commits mailing list