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

[Lei Huang via Phabricator via cfe-commits]

lei added a comment.

LGTM, just a few nit that can be addressed on commit.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:7723
+
+  if (SrcTy->isVectorType()) {
+    VectorType::VectorKind SrcVecKind =
----------------
do we really need this check since we have an assert above?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7726
+        SrcTy->castAs<VectorType>()->getVectorKind();
+    isSrcTyAltivec = (SrcVecKind == VectorType::AltiVecVector);
+  }
----------------
I don't think the initialization above is needed.  Can just inline it here.
```
  bool  isSrcTyAltivec = (SrcVecKind == VectorType::AltiVecVector);
```


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7728
+  }
+  if (DestTy->isVectorType()) {
+    VectorType::VectorKind DestVecKind =
----------------
same.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7731
+        DestTy->castAs<VectorType>()->getVectorKind();
+    isDestTyAltivec = (DestVecKind == VectorType::AltiVecVector);
+  }
----------------
same.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:7715
 
+bool Sema::areAnyVectorTypesAltivec(QualType SrcTy, QualType DestTy) {
+  assert(DestTy->isVectorType() || SrcTy->isVectorType());
----------------
maryammo wrote:
> lei wrote:
> > maryammo wrote:
> > > amyk wrote:
> > > > Can we add some brief documentation for this function, like what is done for other functions in this file?
> > > sure
> > feels like this should be written to just take either 1 param or multiple params via vararg.. since the 2 arg are not really related in any way.
> I am not sure what you mean, can you please elaborate on that?
actually ignore that comment. I see why you need this now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126540