[PATCH] D126540: PowerPC] Emit warning for incompatible vector types that are currently diagnosed with -fno-lax-vector-conversions

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

Some additional minor comments.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7572
+  "Current bitcast for incompatible vector types (%0 and %1) are deprecated. "
+  "The default behaviour will change to what implied by the "
+  "-fno-lax-vector-conversions option">,
----------------
amyk wrote:
> 
nit: Update to 
>The default behaviour will change to what is implied by the


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7715
 
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
----------------
Can we add some brief documentation for this function, like what is done for 
other functions in this file?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7716
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
+
----------------
Can we add a message to this `assert()`?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7722
+  if (SrcTy->isVectorType())
+  {
+    VectorType::VectorKind SrcVecKind = 
SrcTy->castAs<VectorType>()->getVectorKind();
----------------
nit: Can we put braces on the same line? (And same goes for 7727)


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7735
+
+bool Sema::areVectorTypesSameElmType(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
----------------
Can we add some brief documentation for this function, like what is done for 
other functions in this file?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7736
+bool Sema::areVectorTypesSameElmType(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
+
----------------
Can we add a message to this assert?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7762
   }
-
   return areLaxCompatibleVectorTypes(srcTy, destTy);
----------------
Unnecessary change?


================
Comment at: clang/lib/Sema/SemaOverload.cpp:13027
   // end up here.
+  //
   return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc,
----------------
amyk wrote:
> Unnecessary change?
Unnecessary change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126540

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