ahatanak added inline comments.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
if (!IsCompAssign) {
- LHS = S.UsualUnaryConversions(LHS.get());
+ if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+ LHS = S.UsualUnaryConversions(LHS.get());
----------------
It wasn't clear to me why we have to call DefaultFunctionArrayLvalueConversion
instead of UsualUnaryConversions. Is it possible to come up with a test case
that would fail without this change, and if it is, can you add it to vecshift.c?
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
// Note that RHS might not be a vector.
QualType RHSType = RHS.get()->getType();
----------------
We no longer error out when LHS is not a vector, so it should mention that
either the LHS or the RHS might not be a vector.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8752
@@ -8747,3 +8751,3 @@
// OpenCL v1.1 s6.3.j says that the operands need to be integers.
if (!LHSEleType->isIntegerType()) {
----------------
This comment should be updated since other vector types (e.g., gcc's vector
type) are handled by this function too.
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8774
@@ +8773,3 @@
+ QualType VecTy =
+ S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+ LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
----------------
Is it correct to always create an ExtVectorType here? What if the RHS is a
GenericVector?
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
}
+ if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+ const BuiltinType *LHSBT = LHSEleType->getAs<clang::BuiltinType>();
----------------
Is it possible to split this out to another patch? The first patch would fix
handling of (scalar << vector) and the second patch would make clang reject
vector shifts if element sizes of the LHS and RHS are different. It's not clear
whether it's correct to reject the latter case, so perhaps you can discuss it
in a separate patch?
================
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
}
----------------
When the LHS is a scalar, we check the type of the LHS and the element type of
the RHS, and if necessary, cast the LHS to the element type of the RHS. What's
the reason we don't do the same here?
Repository:
rL LLVM
https://reviews.llvm.org/D24467
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
