[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 5 09:49:18 PST 2021


erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5951
   const QualType EltTy =
-      cast<VectorType>(CondTy.getCanonicalType())->getElementType();
+      CondTy->isVectorType()
+          ? cast<VectorType>(CondTy.getCanonicalType())->getElementType()
----------------
Why is this change necessary?  Doesn't ExtVectorType inherit from VectorType?  These should both be calling the same function here.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6012
     }
-    ResultType = Context.getVectorType(
-        ResultElementTy, CondType->castAs<VectorType>()->getNumElements(),
-        VectorType::GenericVector);
+    if (std::is_same<VectorTy, ExtVectorType>::value)
+      ResultType = Context.getExtVectorType(
----------------
Oh boy, this is awkward as all heck...

Makes me think we should instead have a templated function Context.getVectorType<VectorType>() or <ExtVectorType>


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6144
+    auto CondTy = Cond.get()->getType();
+    if (CondTy->isExtVectorType())
+      return CheckVectorConditionalTypes<ExtVectorType>(Cond, LHS, RHS,
----------------
Again I wonder why the 'if' here is necessary?  Shouldn't just removing the 'error' diag in the call take care of this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98055



More information about the cfe-commits mailing list