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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Write some codegen tests please, I'd like to have a better idea of what you want ! to do here.  Additionally, I'm not sure as to the limits you've placed on this patch.  Please justifiy them.C



================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2746
+  if (E->getType()->isVectorType() &&
+      E->getType()->castAs<VectorType>()->getVectorKind() ==
+          VectorType::GenericVector) {
----------------
Why limit this to just the base vector type?  Doesn't this remove the ext-vector implementation?




================
Comment at: clang/lib/Sema/SemaExpr.cpp:14442
+      break;
+    } else if (Context.getLangOpts().CPlusPlus && resultType->isVectorType()) {
+      const VectorType *VTy = resultType->castAs<VectorType>();
----------------
Why C++ only?  It seems if we're doing this, it should be for all language modes.


================
Comment at: clang/test/CodeGen/vector.c:90
+// CHECK: define i32 @lax_vector_logic_not1(i32 {{.*}}, i32 {{.*}})
+// CHECK: icmp ne i32
+
----------------
Can you clarify what this is doing here?  It doesn't seem clear to me what the output of this is.

Additionally, what about FP types?  What do we expect this to emit?


================
Comment at: clang/test/CodeGen/vector.c:98
+// CHECK: define void @lax_vector_logic_not2(<2 x i32>* {{.*sret.*}}, i64 {{.*}}, i64 {{.*}})
+// CHECK: icmp ne <2 x i32>
----------------
Why is the IR so different between int and long long (here and the above test)?  It would seem to me the IR should be basically identical, perhaps with a cast somewhere, but not completely different IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80979