[PATCH] D24467: Fix an error after D21678
vbyakovlcl created this revision. vbyakovlcl added reviewers: ahatanak, aaron.ballman. vbyakovlcl added subscribers: andreybokhanko, cfe-commits. vbyakovlcl set the repository for this revision to rL LLVM. vbyakovlcl changed the visibility of this Differential Revision from "Public (No Login Required)" to "All Users". Herald added a subscriber: aemerson. This fixes an error and gcc incompatibilities in vector shifts reported by Akira Hatanaka -- From: Akira Hatanaka Date: Fri, Sep 2, 2016 at 3:00 AM Subject: Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values To: vladimi...@gmail.com, ulrich.weig...@de.ibm.com, amjad.ab...@intel.com, guy.ben...@intel.com, aaron.ball...@gmail.com Cc: ahata...@gmail.com, andreybokha...@gmail.com, anastasia.stul...@arm.com, dmitry.poluk...@gmail.com, cfe-commits@lists.llvm.org ahatanak added a subscriber: ahatanak. ahatanak added a comment. This patch causes clang to error out on the following code, which used to compile fine: $ cat f2.c typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8; vector_ushort8 foo1(void) { return 1 << (vector_ushort8){7,6,5,4,3,2,1,0}; } $ clang f2.c -c clang used to transform the scaler operand to a vector operand (similar to the way gcc's vector is handled) when compiling for normal c/c++ (but printed an error message when compiling for opencl), but this patch dropped the check for LangOpts added in r230464 and changed that behavior. I don't think this was intentional? Repository: rL LLVM https://reviews.llvm.org/D21678 Repository: rL LLVM https://reviews.llvm.org/D24467 Files: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td llvm/tools/clang/lib/Sema/SemaExpr.cpp llvm/tools/clang/test/Sema/vecshift.c Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp === --- llvm/tools/clang/lib/Sema/SemaExpr.cpp +++ llvm/tools/clang/lib/Sema/SemaExpr.cpp @@ -8721,24 +8721,28 @@ static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) { // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector. - if (!LHS.get()->getType()->isVectorType()) { + if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) && + !LHS.get()->getType()->isVectorType()) { S.Diag(Loc, diag::err_shift_rhs_only_vector) << RHS.get()->getType() << LHS.get()->getType() << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); return QualType(); } if (!IsCompAssign) { -LHS = S.UsualUnaryConversions(LHS.get()); +if (S.LangOpts.OpenCL || S.LangOpts.ZVector) + LHS = S.UsualUnaryConversions(LHS.get()); +else + LHS = S.DefaultFunctionArrayLvalueConversion(LHS.get()); if (LHS.isInvalid()) return QualType(); } RHS = S.UsualUnaryConversions(RHS.get()); if (RHS.isInvalid()) return QualType(); QualType LHSType = LHS.get()->getType(); - const VectorType *LHSVecTy = LHSType->castAs(); - QualType LHSEleType = LHSVecTy->getElementType(); + const VectorType *LHSVecTy = LHSType->getAs(); + QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType; // Note that RHS might not be a vector. QualType RHSType = RHS.get()->getType(); @@ -8758,7 +8762,19 @@ return QualType(); } - if (RHSVecTy) { + if (!LHSVecTy) { +assert(RHSVecTy); +if (IsCompAssign) + return RHSType; +if (LHSEleType != RHSEleType) { + LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast); + LHSEleType = RHSEleType; +} +QualType VecTy = +S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements()); +LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat); +LHSType = VecTy; + } else if (RHSVecTy) { // OpenCL v1.1 s6.3.j says that for vector types, the operators // are applied component-wise. So if RHS is a vector, then ensure // that the number of elements is the same as LHS... @@ -8768,6 +8784,62 @@ << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); return QualType(); } +if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) { + const BuiltinType *LHSBT = LHSEleType->getAs(); + const BuiltinType *RHSBT = RHSEleType->getAs(); + if (LHSBT != RHSBT) { +BuiltinType::Kind LHSKind = LHSBT->getKind(); +BuiltinType::Kind RHSKind = RHSBT->getKind(); +bool DiffSizes = true; +switch (LHSKind) { +case BuiltinType::Char_S: + DiffSizes = + RHSKind != BuiltinType::Char_U && RHSKind != BuiltinType::UChar; + break; +case BuiltinType::Char_U: + DiffSizes = + RHSKind != BuiltinType::Char_S && RHSKind != BuiltinType::UChar; + break; +case BuiltinType::UChar: + Diff
Re: [PATCH] D24467: Fix an error after D21678
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(); 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
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl removed rL LLVM as the repository for this revision. vbyakovlcl changed the visibility of this Differential Revision from "All Users" to "Public (No Login Required)". vbyakovlcl updated this revision to Diff 71135. https://reviews.llvm.org/D24467 Files: llvm/tools/clang/lib/Sema/SemaExpr.cpp llvm/tools/clang/test/Sema/vecshift.c Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp === --- llvm/tools/clang/lib/Sema/SemaExpr.cpp +++ llvm/tools/clang/lib/Sema/SemaExpr.cpp @@ -8721,7 +8721,8 @@ static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) { // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector. - if (!LHS.get()->getType()->isVectorType()) { + if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) && + !LHS.get()->getType()->isVectorType()) { S.Diag(Loc, diag::err_shift_rhs_only_vector) << RHS.get()->getType() << LHS.get()->getType() << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); @@ -8737,8 +8738,10 @@ if (RHS.isInvalid()) return QualType(); QualType LHSType = LHS.get()->getType(); - const VectorType *LHSVecTy = LHSType->castAs(); - QualType LHSEleType = LHSVecTy->getElementType(); + // Note that LHS might be a scalar because the routine calls not only in + // OpenCL case. + const VectorType *LHSVecTy = LHSType->getAs(); + QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType; // Note that RHS might not be a vector. QualType RHSType = RHS.get()->getType(); @@ -8758,7 +8761,19 @@ return QualType(); } - if (RHSVecTy) { + if (!LHSVecTy) { +assert(RHSVecTy); +if (IsCompAssign) + return RHSType; +if (LHSEleType != RHSEleType) { + LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast); + LHSEleType = RHSEleType; +} +QualType VecTy = +S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements()); +LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat); +LHSType = VecTy; + } else if (RHSVecTy) { // OpenCL v1.1 s6.3.j says that for vector types, the operators // are applied component-wise. So if RHS is a vector, then ensure // that the number of elements is the same as LHS... Index: llvm/tools/clang/test/Sema/vecshift.c === --- llvm/tools/clang/test/Sema/vecshift.c +++ llvm/tools/clang/test/Sema/vecshift.c @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +typedef __attribute__((__ext_vector_type__(8))) char vector_char8; +typedef __attribute__((__ext_vector_type__(8))) short vector_short8; +typedef __attribute__((__ext_vector_type__(8))) int vector_int8; +typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8; +typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8; +typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8; +typedef __attribute__((__ext_vector_type__(4))) char vector_char4; +typedef __attribute__((__ext_vector_type__(4))) short vector_short4; +typedef __attribute__((__ext_vector_type__(4))) int vector_int4; +typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4; +typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4; +typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4; + +char c; +short s; +int i; +unsigned char uc; +unsigned short us; +unsigned int ui; +vector_char8 vc8; +vector_short8 vs8; +vector_int8 vi8; +vector_uchar8 vuc8; +vector_ushort8 vus8; +vector_uint8 vui8; +vector_char4 vc4; +vector_short4 vs4; +vector_int4 vi4; +vector_uchar4 vuc4; +vector_ushort4 vus4; +vector_uint4 vui4; + +void foo() { + vc8 = 1 << vc8; + vuc8 = 1 << vuc8; + vi8 = 1 << vi8; + vui8 = 1 << vui8; + vs8 = 1 << vs8; + vus8 = 1 << vus8; + + vc8 = c << vc8; + vuc8 = i << vuc8; + vi8 = uc << vi8; + vui8 = us << vui8; + vs8 = ui << vs8; + vus8 = 1 << vus8; + + vc8 = vc8 << vc8; + vi8 = vi8 << vuc8; + vuc8 = vuc8 << vi8; + vus8 = vus8 << vui8; + vui8 = vui8 << vs8; + + vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same number of elements}} + vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same number of elements}} + vuc4 = vuc4 << vi8; // expected-error {{vector operands do not have the same number of elements}} + vus4 = vus4 << vui8; // expected-error {{vector operands do not have the same number of elements}} + vui4 = vui4 << vs8; // expected-error {{vector operands do not have the same number of elements}} + + + vc8 = vc8 << vc4; // expected-error {{vector operands do not have the same number of elements}} + vi8 = vi8 << vuc4; // expected-error {{vector operands do not have the same number of elements}} + vuc8 = vuc8 << vi4; // expected-error {{vector operands do not have the same numb
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl 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()); ahatanak wrote: > 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? I cannot remember why I did this change. Removing it doesn't cause any new fails. 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(); ahatanak wrote: > 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. I added a comment. 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(); ahatanak wrote: > 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? Ok. I removed this and will fail a new review after committing of this review. 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); } ahatanak wrote: > 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? No need, because CodeGen inserts a conversion. define <8 x i16> @foo1(<8 x i16> %n, i32 %m) #0 { entry: %n.addr = alloca <8 x i16>, align 16 %m.addr = alloca i32, align 4 store <8 x i16> %n, <8 x i16>* %n.addr, align 16 store i32 %m, i32* %m.addr, align 4 %0 = load <8 x i16>, <8 x i16>* %n.addr, align 16 %1 = load i32, i32* %m.addr, align 4 %splat.splatinsert = insertelement <8 x i32> undef, i32 %1, i32 0 %splat.splat = shufflevector <8 x i32> %splat.splatinsert, <8 x i32> undef, <8 x i32> zeroinitializer %sh_prom = trunc <8 x i32> %splat.splat to <8 x i16> %shl = shl <8 x i16> %0, %sh_prom ret <8 x i16> %shl We need insert a conversion of scalar LHS to the RHS element type because we must return a vector of the RHS type. https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
ahatanak added inline comments. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8751 @@ -8747,3 +8750,3 @@ // OpenCL v1.1 s6.3.j says that the operands need to be integers. if (!LHSEleType->isIntegerType()) { Should we mention that any vectors used as shift operands have to have integer element types? This comment gives the impression that only OpenCL vectors need to be integers. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790 @@ -8774,3 +8789,3 @@ S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements()); RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat); } OK. So the rule is that the type of the scalar operand is normally converted to the vector element type of the other operand before being transformed to a vector, but we don't have to do this when it's used as the RHS (shift amount) because IRGen generates the correct IR without the conversion? Comment at: llvm/tools/clang/test/Sema/vecshift.c:56 @@ +55,3 @@ + + vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same number of elements}} + vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same number of elements}} I don't think this patch was necessary to have clang print diagnostic "vector operands do not have the same number of elements"? If that's the case, I think it's better to add these lines in a separate commit. https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl added inline comments. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790 @@ -8774,3 +8789,3 @@ S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements()); RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat); } ahatanak wrote: > OK. So the rule is that the type of the scalar operand is normally converted > to the vector element type of the other operand before being transformed to a > vector, but we don't have to do this when it's used as the RHS (shift amount) > because IRGen generates the correct IR without the conversion? Yes. I think a CodeGen test should be added for checking of IR correctness. Comment at: llvm/tools/clang/test/Sema/vecshift.c:56 @@ +55,3 @@ + + vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same number of elements}} + vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same number of elements}} ahatanak wrote: > I don't think this patch was necessary to have clang print diagnostic "vector > operands do not have the same number of elements"? If that's the case, I > think it's better to add these lines in a separate commit. Do you propose to delete lines with this diagnostic? https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
ahatanak added inline comments. Comment at: llvm/tools/clang/test/Sema/vecshift.c:56 @@ +55,3 @@ + + vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same number of elements}} + vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same number of elements}} vbyakovlcl wrote: > ahatanak wrote: > > I don't think this patch was necessary to have clang print diagnostic > > "vector operands do not have the same number of elements"? If that's the > > case, I think it's better to add these lines in a separate commit. > Do you propose to delete lines with this diagnostic? Yes, I think you can move them to a separate patch. Doing so will help people who read the patch understand what problems you are trying to fix with this patch. https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl updated this revision to Diff 71498. https://reviews.llvm.org/D24467 Files: llvm/tools/clang/lib/Sema/SemaExpr.cpp llvm/tools/clang/test/CodeGen/vecshift.c llvm/tools/clang/test/Sema/vecshift.c Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp === --- llvm/tools/clang/lib/Sema/SemaExpr.cpp +++ llvm/tools/clang/lib/Sema/SemaExpr.cpp @@ -8721,7 +8721,8 @@ static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) { // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector. - if (!LHS.get()->getType()->isVectorType()) { + if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) && + !LHS.get()->getType()->isVectorType()) { S.Diag(Loc, diag::err_shift_rhs_only_vector) << RHS.get()->getType() << LHS.get()->getType() << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); @@ -8737,15 +8738,17 @@ if (RHS.isInvalid()) return QualType(); QualType LHSType = LHS.get()->getType(); - const VectorType *LHSVecTy = LHSType->castAs(); - QualType LHSEleType = LHSVecTy->getElementType(); + // Note that LHS might be a scalar because the routine calls not only in + // OpenCL case. + const VectorType *LHSVecTy = LHSType->getAs(); + QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType; // Note that RHS might not be a vector. QualType RHSType = RHS.get()->getType(); const VectorType *RHSVecTy = RHSType->getAs(); QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType; - // OpenCL v1.1 s6.3.j says that the operands need to be integers. + // The operands need to be integers. if (!LHSEleType->isIntegerType()) { S.Diag(Loc, diag::err_typecheck_expect_int) << LHS.get()->getType() << LHS.get()->getSourceRange(); @@ -8758,7 +8761,19 @@ return QualType(); } - if (RHSVecTy) { + if (!LHSVecTy) { +assert(RHSVecTy); +if (IsCompAssign) + return RHSType; +if (LHSEleType != RHSEleType) { + LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast); + LHSEleType = RHSEleType; +} +QualType VecTy = +S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements()); +LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat); +LHSType = VecTy; + } else if (RHSVecTy) { // OpenCL v1.1 s6.3.j says that for vector types, the operators // are applied component-wise. So if RHS is a vector, then ensure // that the number of elements is the same as LHS... Index: llvm/tools/clang/test/CodeGen/vecshift.c === --- llvm/tools/clang/test/CodeGen/vecshift.c +++ llvm/tools/clang/test/CodeGen/vecshift.c @@ -0,0 +1,146 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s + +typedef __attribute__((__ext_vector_type__(8))) char vector_char8; +typedef __attribute__((__ext_vector_type__(8))) short vector_short8; +typedef __attribute__((__ext_vector_type__(8))) int vector_int8; +typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8; +typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8; +typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8; +typedef __attribute__((__ext_vector_type__(4))) char vector_char4; +typedef __attribute__((__ext_vector_type__(4))) short vector_short4; +typedef __attribute__((__ext_vector_type__(4))) int vector_int4; +typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4; +typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4; +typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4; + +char c; +short s; +int i; +unsigned char uc; +unsigned short us; +unsigned int ui; +vector_char8 vc8; +vector_short8 vs8; +vector_int8 vi8; +vector_uchar8 vuc8; +vector_ushort8 vus8; +vector_uint8 vui8; +vector_char4 vc4; +vector_short4 vs4; +vector_int4 vi4; +vector_uchar4 vuc4; +vector_ushort4 vus4; +vector_uint4 vui4; + +void foo() { + vc8 = 1 << vc8; +// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, align 8 +// CHECK: shl <8 x i8> , [[t0]] + vuc8 = 1 << vuc8; +// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* @vuc8, align 8 +// CHECK: shl <8 x i8> , [[t1]] + vi8 = 1 << vi8; +// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32 +// CHECK: shl <8 x i32> , [[t2]] + vui8 = 1 << vui8; +// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* @vui8, align 32 +// CHECK: shl <8 x i32> , [[t3]] + vs8 = 1 << vs8; +// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* @vs8, align 16 +// CHECK: shl <8 x i16> , [[t4]] + vus8 = 1 << vus8; +// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* @vus8, align 16 +// CHECK: shl <8 x i16> , [[t5]] + + vc8 = c << vc8; +// CHECK: [[t6:%.+]] = load i8, i8* @c, align 1 +// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0 +// CHEC
Re: [PATCH] D24467: Fix an error after D21678
ahatanak added a comment. LGTM with a nit in test case. Comment at: llvm/tools/clang/test/CodeGen/vecshift.c:43 @@ +42,3 @@ + vi8 = 1 << vi8; +// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32 +// CHECK: shl <8 x i32> , [[t2]] This test fails on my machine because MaxVectorAlign is 128 (16B) for darwin. Maybe you can remove the alignment checks or add a triple to the RUN line? https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl added inline comments. Comment at: llvm/tools/clang/test/CodeGen/vecshift.c:43 @@ +42,3 @@ + vi8 = 1 << vi8; +// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32 +// CHECK: shl <8 x i32> , [[t2]] ahatanak wrote: > This test fails on my machine because MaxVectorAlign is 128 (16B) for darwin. > Maybe you can remove the alignment checks or add a triple to the RUN line? Done https://reviews.llvm.org/D24467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24467: Fix an error after D21678
vbyakovlcl updated this revision to Diff 71545. https://reviews.llvm.org/D24467 Files: llvm/tools/clang/lib/Sema/SemaExpr.cpp llvm/tools/clang/test/CodeGen/vecshift.c llvm/tools/clang/test/Sema/vecshift.c Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp === --- llvm/tools/clang/lib/Sema/SemaExpr.cpp +++ llvm/tools/clang/lib/Sema/SemaExpr.cpp @@ -8721,7 +8721,8 @@ static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign) { // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector. - if (!LHS.get()->getType()->isVectorType()) { + if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) && + !LHS.get()->getType()->isVectorType()) { S.Diag(Loc, diag::err_shift_rhs_only_vector) << RHS.get()->getType() << LHS.get()->getType() << LHS.get()->getSourceRange() << RHS.get()->getSourceRange(); @@ -8737,15 +8738,17 @@ if (RHS.isInvalid()) return QualType(); QualType LHSType = LHS.get()->getType(); - const VectorType *LHSVecTy = LHSType->castAs(); - QualType LHSEleType = LHSVecTy->getElementType(); + // Note that LHS might be a scalar because the routine calls not only in + // OpenCL case. + const VectorType *LHSVecTy = LHSType->getAs(); + QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType; // Note that RHS might not be a vector. QualType RHSType = RHS.get()->getType(); const VectorType *RHSVecTy = RHSType->getAs(); QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType; - // OpenCL v1.1 s6.3.j says that the operands need to be integers. + // The operands need to be integers. if (!LHSEleType->isIntegerType()) { S.Diag(Loc, diag::err_typecheck_expect_int) << LHS.get()->getType() << LHS.get()->getSourceRange(); @@ -8758,7 +8761,19 @@ return QualType(); } - if (RHSVecTy) { + if (!LHSVecTy) { +assert(RHSVecTy); +if (IsCompAssign) + return RHSType; +if (LHSEleType != RHSEleType) { + LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast); + LHSEleType = RHSEleType; +} +QualType VecTy = +S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements()); +LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat); +LHSType = VecTy; + } else if (RHSVecTy) { // OpenCL v1.1 s6.3.j says that for vector types, the operators // are applied component-wise. So if RHS is a vector, then ensure // that the number of elements is the same as LHS... Index: llvm/tools/clang/test/CodeGen/vecshift.c === --- llvm/tools/clang/test/CodeGen/vecshift.c +++ llvm/tools/clang/test/CodeGen/vecshift.c @@ -0,0 +1,146 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s + +typedef __attribute__((__ext_vector_type__(8))) char vector_char8; +typedef __attribute__((__ext_vector_type__(8))) short vector_short8; +typedef __attribute__((__ext_vector_type__(8))) int vector_int8; +typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8; +typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8; +typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8; +typedef __attribute__((__ext_vector_type__(4))) char vector_char4; +typedef __attribute__((__ext_vector_type__(4))) short vector_short4; +typedef __attribute__((__ext_vector_type__(4))) int vector_int4; +typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4; +typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4; +typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4; + +char c; +short s; +int i; +unsigned char uc; +unsigned short us; +unsigned int ui; +vector_char8 vc8; +vector_short8 vs8; +vector_int8 vi8; +vector_uchar8 vuc8; +vector_ushort8 vus8; +vector_uint8 vui8; +vector_char4 vc4; +vector_short4 vs4; +vector_int4 vi4; +vector_uchar4 vuc4; +vector_ushort4 vus4; +vector_uint4 vui4; + +void foo() { + vc8 = 1 << vc8; +// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> , [[t0]] + vuc8 = 1 << vuc8; +// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> , [[t1]] + vi8 = 1 << vi8; +// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> , [[t2]] + vui8 = 1 << vui8; +// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> , [[t3]] + vs8 = 1 << vs8; +// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: shl <8 x i16> , [[t4]] + vus8 = 1 << vus8; +// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: shl <8 x i16> , [[t5]] + + vc8 = c << vc8; +// CHECK: [[t6:%.+]] = load i8, i8* @c, +// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0 +// CHECK: [[splat_splat:%.+]] = shufflevector <8 x i8>
Re: [PATCH] D24467: Fix an error after D21678
This revision was automatically updated to reflect the committed changes. Closed by commit rL281669: [Sema] Allow shifting a scalar operand by a vector operand. (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D24467?vs=71545&id=71569#toc Repository: rL LLVM https://reviews.llvm.org/D24467 Files: cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/CodeGen/vecshift.c cfe/trunk/test/Sema/vecshift.c Index: cfe/trunk/test/CodeGen/vecshift.c === --- cfe/trunk/test/CodeGen/vecshift.c +++ cfe/trunk/test/CodeGen/vecshift.c @@ -0,0 +1,146 @@ +// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s + +typedef __attribute__((__ext_vector_type__(8))) char vector_char8; +typedef __attribute__((__ext_vector_type__(8))) short vector_short8; +typedef __attribute__((__ext_vector_type__(8))) int vector_int8; +typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8; +typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8; +typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8; +typedef __attribute__((__ext_vector_type__(4))) char vector_char4; +typedef __attribute__((__ext_vector_type__(4))) short vector_short4; +typedef __attribute__((__ext_vector_type__(4))) int vector_int4; +typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4; +typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4; +typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4; + +char c; +short s; +int i; +unsigned char uc; +unsigned short us; +unsigned int ui; +vector_char8 vc8; +vector_short8 vs8; +vector_int8 vi8; +vector_uchar8 vuc8; +vector_ushort8 vus8; +vector_uint8 vui8; +vector_char4 vc4; +vector_short4 vs4; +vector_int4 vi4; +vector_uchar4 vuc4; +vector_ushort4 vus4; +vector_uint4 vui4; + +void foo() { + vc8 = 1 << vc8; +// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> , [[t0]] + vuc8 = 1 << vuc8; +// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> , [[t1]] + vi8 = 1 << vi8; +// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> , [[t2]] + vui8 = 1 << vui8; +// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> , [[t3]] + vs8 = 1 << vs8; +// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: shl <8 x i16> , [[t4]] + vus8 = 1 << vus8; +// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: shl <8 x i16> , [[t5]] + + vc8 = c << vc8; +// CHECK: [[t6:%.+]] = load i8, i8* @c, +// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0 +// CHECK: [[splat_splat:%.+]] = shufflevector <8 x i8> [[splat_splatinsert]], <8 x i8> undef, <8 x i32> zeroinitializer +// CHECK: [[t7:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> [[splat_splat]], [[t7]] + vuc8 = i << vuc8; +// CHECK: [[t8:%.+]] = load i32, i32* @i, +// CHECK: [[tconv:%.+]] = trunc i32 [[t8]] to i8 +// CHECK: [[splat_splatinsert7:%.+]] = insertelement <8 x i8> undef, i8 [[tconv]], i32 0 +// CHECK: [[splat_splat8:%.+]] = shufflevector <8 x i8> [[splat_splatinsert7]], <8 x i8> undef, <8 x i32> zeroinitializer +// CHECK: [[t9:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: shl <8 x i8> [[splat_splat8]], [[t9]] + vi8 = uc << vi8; +// CHECK: [[t10:%.+]] = load i8, i8* @uc, +// CHECK: [[conv10:%.+]] = zext i8 [[t10]] to i32 +// CHECK: [[splat_splatinsert11:%.+]] = insertelement <8 x i32> undef, i32 [[conv10]], i32 0 +// CHECK: [[splat_splat12:%.+]] = shufflevector <8 x i32> [[splat_splatinsert11]], <8 x i32> undef, <8 x i32> zeroinitializer +// CHECK: [[t11:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> [[splat_splat12]], [[t11]] + vui8 = us << vui8; +// CHECK: [[t12:%.+]] = load i16, i16* @us, +// CHECK: [[conv14:%.+]] = zext i16 [[t12]] to i32 +// CHECK: [[splat_splatinsert15:%.+]] = insertelement <8 x i32> undef, i32 [[conv14]], i32 0 +// CHECK: [[splat_splat16:%.+]] = shufflevector <8 x i32> [[splat_splatinsert15]], <8 x i32> undef, <8 x i32> zeroinitializer +// CHECK: [[t13:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}}, +// CHECK: shl <8 x i32> [[splat_splat16]], [[t13]] + vs8 = ui << vs8; +// CHECK: [[t14:%.+]] = load i32, i32* @ui, +// CHECK: [[conv18:%.+]] = trunc i32 [[t14]] to i16 +// CHECK: [[splat_splatinsert19:%.+]] = insertelement <8 x i16> undef, i16 [[conv18]], i32 0 +// CHECK: [[splat_splat20:%.+]] = shufflevector <8 x i16> [[splat_splatinsert19]], <8 x i16> undef, <8 x i32> zeroinitializer +// CHECK: [[t15:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: shl <8 x i16> [[splat_splat20]], [[t15]] + vus8 = 1 << vus8; +// CHECK: [[t16:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}}, +// CHECK: [[shl22:%.+]] = shl <8 x i16> , [[t16]] + + vc8 = vc8 << vc8; +// CHECK: [[t17:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, +// CHECK: [[t18:%.+]] = load <8