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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
