[PATCH] D80979: [clang] Implement VectorType logic not operator.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2746
+  if (E->getType()->isVectorType() &&
+      E->getType()->castAs<VectorType>()->getVectorKind() ==
+          VectorType::GenericVector) {
----------------
erichkeane wrote:
> junparser wrote:
> > erichkeane wrote:
> > > Why limit this to just the base vector type?  Doesn't this remove the ext-vector implementation?
> > > 
> > > 
> > the kind of ext-vector is  GenericVector as well. so it also includes ext-vector.
> "isVectorType" also includes ExtVectorType.  My question is which vector types are you attempting to exclude here?
> 
> Can the ExtVectorKind ever be a AltiVec* or Neon Vector type?  If so, this change would break code for those.
Just dug in and answered my own question, ExtVector is always created as Generic, see ASTContext::getExtVectorType.  So I think the check for GenericVector (here and in Sema) properly constrains this patch to only adding C++-mode standard vector-types.


================
Comment at: clang/test/CodeGen/vector-1.cpp:2
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+typedef __attribute__((__vector_size__(16))) float float4;
----------------
I don't think copying the whole test from the other file is the right idea. We already validate the rest of the operations on normal vectors in a number of places.  If any of those are C++ tests, just add your tests there.  Otherwise this test should only validate the logical-not operator.


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

https://reviews.llvm.org/D80979