Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
I looked at r278501 and realized it’s doing more than just fixing the test case in PR28288. For example, when the following code was compiled, vi8 = vi8 << vuc8; // vuc8: <8 x unsigned char>, vi8: <8 x int> vc8 = vc8 << vs4; // vc8: <8 x char>, vs4: <4 x short> prior to r278501, clang would reject the first statement but accept the second, while after r278501, it would accept the first and reject the second. gcc 6.2 rejects both. Was this change discussed thoroughly and is this really the behavior we want? > On Sep 7, 2016, at 12:53 AM, Vladimir Yakovlev wrote: > > I did another fix (attached). It fixes the error and chang-check has not any > new errors. Please take a look. > > Where can I see your fix? > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
Thanks, I’ll take a look at your patch today. vec1.patch Description: Binary data > On Sep 7, 2016, at 12:53 AM, Vladimir Yakovlev wrote: > > I did another fix (attached). It fixes the error and chang-check has not any > new errors. Please take a look. > > Where can I see your fix? > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
It seems to me that the test case in test/Sema/vector-cast.c looks incorrect.
According to gcc’s documentation, you can cast vectors to and from scalars of
the same size, so clang shouldn’t reject “double += <1 x double>".
https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html
<https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html>
It is possible to cast from one vector type to another, provided they are of
the same size (in fact, you can also cast vectors to and from other datatypes
of the same size).
> On Sep 6, 2016, at 3:09 PM, Akira Hatanaka wrote:
>
> This is urgent, so I’ve tried to come up with a patch to fix this. I tried
> reverting r278501 and then made changes to have Sema::CheckVectorOperands
> call tryVectorConvertAndSplat when the vector type is a normal gcc vector.
> This fixes the test case I sent and also doesn’t affect the regression tests
> except that clang prints different error messages in a few cases and doesn’t
> print an expected diagnostic in one case, which is line 63 of
> test/Sema/vector-cast.c. We currently expect clang to print a diagnostic when
> we have a compound assignment like “double += <1 X double>”, but my patch
> makes clang accept it.
>
>> On Sep 5, 2016, at 6:36 AM, Vladimir Yakovlev via cfe-commits
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>
>> I'll fix this.
>>
>>
>>
>> Vladimir
>>
>>
>>
>> ---------------------------
>>
>> From: Akira Hatanaka mailto:ahata...@gmail.com>>
>> 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 <mailto:vladimi...@gmail.com>,
>> ulrich.weig...@de.ibm.com <mailto:ulrich.weig...@de.ibm.com>,
>> amjad.ab...@intel.com <mailto:amjad.ab...@intel.com>, guy.ben...@intel.com
>> <mailto:guy.ben...@intel.com>, aaron.ball...@gmail.com
>> <mailto:aaron.ball...@gmail.com>
>> Cc: ahata...@gmail.com <mailto:ahata...@gmail.com>, andreybokha...@gmail.com
>> <mailto:andreybokha...@gmail.com>, anastasia.stul...@arm.com
>> <mailto:anastasia.stul...@arm.com>, dmitry.poluk...@gmail.com
>> <mailto:dmitry.poluk...@gmail.com>, cfe-commits@lists.llvm.org
>> <mailto: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 <https://reviews.llvm.org/D21678>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
This is urgent, so I’ve tried to come up with a patch to fix this. I tried
reverting r278501 and then made changes to have Sema::CheckVectorOperands call
tryVectorConvertAndSplat when the vector type is a normal gcc vector. This
fixes the test case I sent and also doesn’t affect the regression tests except
that clang prints different error messages in a few cases and doesn’t print an
expected diagnostic in one case, which is line 63 of test/Sema/vector-cast.c.
We currently expect clang to print a diagnostic when we have a compound
assignment like “double += <1 X double>”, but my patch makes clang accept it.
> On Sep 5, 2016, at 6:36 AM, Vladimir Yakovlev via cfe-commits
> wrote:
>
> I'll fix this.
>
>
>
> Vladimir
>
>
>
> ---
>
> From: Akira Hatanaka mailto:ahata...@gmail.com>>
> 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 <mailto:vladimi...@gmail.com>,
> ulrich.weig...@de.ibm.com <mailto:ulrich.weig...@de.ibm.com>,
> amjad.ab...@intel.com <mailto:amjad.ab...@intel.com>, guy.ben...@intel.com
> <mailto:guy.ben...@intel.com>, aaron.ball...@gmail.com
> <mailto:aaron.ball...@gmail.com>
> Cc: ahata...@gmail.com <mailto:ahata...@gmail.com>, andreybokha...@gmail.com
> <mailto:andreybokha...@gmail.com>, anastasia.stul...@arm.com
> <mailto:anastasia.stul...@arm.com>, dmitry.poluk...@gmail.com
> <mailto:dmitry.poluk...@gmail.com>, cfe-commits@lists.llvm.org
> <mailto: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 <https://reviews.llvm.org/D21678>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
I'll fix this.
Vladimir
---
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278501: Fix For pr28288 - Error message in shift of vector
values (authored by asbokhan).
Changed prior to commit:
https://reviews.llvm.org/D21678?vs=67140&id=67820#toc
Repository:
rL LLVM
https://reviews.llvm.org/D21678
Files:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/shift.c
Index: cfe/trunk/test/Sema/shift.c
===
--- cfe/trunk/test/Sema/shift.c
+++ cfe/trunk/test/Sema/shift.c
@@ -67,3 +67,14 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__((vector_size(16)));
+typedef unsigned vec8 __attribute__((vector_size(8)));
+
+void vect_shift_1(vec16 *x) { *x = *x << 4; }
+
+void vect_shift_2(vec16 *x, vec16 y) { *x = *x << y; }
+
+void vect_shift_3(vec16 *x, vec8 y) {
+ *x = *x << y; // expected-error {{vector operands do not have the same
number of elements}}
+}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -8684,11 +8684,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8756,23 +8755,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHSVecType = RHS.get()->getType()->getAs())
if (RHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
+return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
// Shifts don't perform usual arithmetic conversions, they just do integer
Index: cfe/trunk/test/Sema/shift.c
===
--- cfe/trunk/test/Sema/shift.c
+++ cfe/trunk/test/Sema/shift.c
@@ -67,3 +67,14 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__((vector_size(16)));
+typedef unsigned vec8 __attribute__((vector_size(8)));
+
+void vect_shift_1(vec16 *x) { *x = *x << 4; }
+
+void vect_shift_2(vec16 *x, vec16 y) { *x = *x << y; }
+
+void vect_shift_3(vec16 *x, vec8 y) {
+ *x = *x << y; // expected-error {{vector operands do not have the same number of elements}}
+}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -8684,11 +8684,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8756,23 +8755,18 @@
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
andreybokhanko added a subscriber: andreybokhanko. andreybokhanko added a comment. @uweigand Ulrich, any objections here? If not, I will commit this patch tomorrow (as the author, Vladimir, doesn't have commit access yet). Andrey Repository: rL LLVM https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but I am also not well-versed in vector operations, so you may want to wait a day or two for @uweigand or someone else to sign off as well. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683 @@ -8680,5 +8676,3 @@ } -return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign, - /*AllowBothBool*/true, - /*AllowBoolConversions*/false); } vbyakovl wrote: > aaron.ballman wrote: > > Are you saying that calling `CheckVectorOperands` was always incorrect? Or > > that it's no longer required because it should be fully covered by > > `checkVectorShift`? Because the two methods do considerably different > > checking, and I would have expected there to be more behavioral differences > > in the tests by removing the call to `CheckVectorOperands` that suggests > > there are tests missing. > Yes, calling CheckVectorOperands is not correct here because it compares > operand types to be the same. For shift it is not true. Thank you for clarifying! Repository: rL LLVM https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl added inline comments. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683 @@ -8680,5 +8676,3 @@ } -return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign, - /*AllowBothBool*/true, - /*AllowBoolConversions*/false); } aaron.ballman wrote: > Are you saying that calling `CheckVectorOperands` was always incorrect? Or > that it's no longer required because it should be fully covered by > `checkVectorShift`? Because the two methods do considerably different > checking, and I would have expected there to be more behavioral differences > in the tests by removing the call to `CheckVectorOperands` that suggests > there are tests missing. Yes, calling CheckVectorOperands is not correct here because it compares operand types to be the same. For shift it is not true. Repository: rL LLVM https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
aaron.ballman added inline comments. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683 @@ -8680,5 +8676,3 @@ } -return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign, - /*AllowBothBool*/true, - /*AllowBoolConversions*/false); } Are you saying that calling `CheckVectorOperands` was always incorrect? Or that it's no longer required because it should be fully covered by `checkVectorShift`? Because the two methods do considerably different checking, and I would have expected there to be more behavioral differences in the tests by removing the call to `CheckVectorOperands` that suggests there are tests missing. Repository: rL LLVM https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc, bool IsCompAssign) {
aaron.ballman wrote:
> Why does this drop the mention of OpenCL in the function name?
This routine has common applying rather than for OpenCL only.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
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...
+// 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...
aaron.ballman wrote:
> It's good to keep language references in the comments, so I would leave the
> reference in even though this is being expanded for non-OpenCL vector types
> (unless the reference is incorrect).
I'll restore the language reference.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
}
aaron.ballman wrote:
> Why is it correct to remove semantic checking for vector operands?
This routine targets for checking operands of operations like plus, mul ...,
but not shifts. The tests vect_shift_1 and vect_shift_2 (I added) are examples
which were compiled with error.
Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{
aaron.ballman wrote:
> Please clang-format the test case.
Ok.
Repository:
rL LLVM
https://reviews.llvm.org/D21678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl set the repository for this revision to rL LLVM.
vbyakovl updated this revision to Diff 67140.
Repository:
rL LLVM
https://reviews.llvm.org/D21678
Files:
llvm/tools/clang/lib/Sema/SemaExpr.cpp
llvm/tools/clang/test/Sema/shift.c
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8664,23 +8663,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHSVecType = RHS.get()->getType()->getAs())
if (RHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
+return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
// Shifts don't perform usual arithmetic conversions, they just do integer
Index: llvm/tools/clang/test/Sema/shift.c
===
--- llvm/tools/clang/test/Sema/shift.c
+++ llvm/tools/clang/test/Sema/shift.c
@@ -67,3 +67,14 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__((vector_size(16)));
+typedef unsigned vec8 __attribute__((vector_size(8)));
+
+void vect_shift_1(vec16 *x) { *x = *x << 4; }
+
+void vect_shift_2(vec16 *x, vec16 y) { *x = *x << y; }
+
+void vect_shift_3(vec16 *x, vec8 y) {
+ *x = *x << y; // expected-error {{vector operands do not have the same
number of elements}}
+}
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8664,23 +8663,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHS
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
aaron.ballman added inline comments.
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc, bool IsCompAssign) {
Why does this drop the mention of OpenCL in the function name?
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
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...
+// 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...
It's good to keep language references in the comments, so I would leave the
reference in even though this is being expanded for non-OpenCL vector types
(unless the reference is incorrect).
Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
}
Why is it correct to remove semantic checking for vector operands?
Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{
Please clang-format the test case.
https://reviews.llvm.org/D21678
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl added a comment. Someone, please review this. https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl added a comment. Ping! https://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl updated this revision to Diff 62464.
http://reviews.llvm.org/D21678
Files:
llvm/tools/clang/lib/Sema/SemaExpr.cpp
llvm/tools/clang/test/Sema/shift.c
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8636,9 +8635,8 @@
}
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...
+// 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...
if (RHSVecTy->getNumElements() != LHSVecTy->getNumElements()) {
S.Diag(Loc, diag::err_typecheck_vector_lengths_not_equal)
<< LHS.get()->getType() << RHS.get()->getType()
@@ -8664,23 +8662,18 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHSVecType = RHS.get()->getType()->getAs())
if (RHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
+return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
// Shifts don't perform usual arithmetic conversions, they just do integer
Index: llvm/tools/clang/test/Sema/shift.c
===
--- llvm/tools/clang/test/Sema/shift.c
+++ llvm/tools/clang/test/Sema/shift.c
@@ -67,3 +67,24 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__ ((vector_size (16)));
+typedef unsigned vec8 __attribute__ ((vector_size (8)));
+
+void
+vect_shift_1 (vec16 *x)
+{
+ *x = *x << 4 ;
+}
+
+void
+vect_shift_2 (vec16 *x,vec16 y)
+{
+ *x = *x << y ;
+}
+
+void
+vect_shift_3 (vec16 *x,vec8 y)
+{
+ *x = *x << y ; // expected-error {{vector operands do not have the same
number of elements}}
+}
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when a vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8636,9 +8635,8 @@
}
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 th
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
Anastasia added a subscriber: Anastasia. Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8595 @@ -8594,3 +8594,3 @@ -/// \brief Return the resulting type when an OpenCL vector is shifted +/// \brief Return the resulting type when an vector is shifted ///by a scalar or vector shift amount. an vector -> a vector Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8675 @@ -8678,3 +8674,3 @@ return InvalidOperands(Loc, LHS, RHS); - return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign); + return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign); } it seems like you don't need this statement as the next one is exactly the same! http://reviews.llvm.org/D21678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values
vbyakovl updated this revision to Diff 62345.
http://reviews.llvm.org/D21678
Files:
llvm/tools/clang/lib/Sema/SemaExpr.cpp
llvm/tools/clang/test/Sema/shift.c
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when an vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8636,9 +8635,8 @@
}
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...
+// 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...
if (RHSVecTy->getNumElements() != LHSVecTy->getNumElements()) {
S.Diag(Loc, diag::err_typecheck_vector_lengths_not_equal)
<< LHS.get()->getType() << RHS.get()->getType()
@@ -8664,23 +8662,19 @@
// Vector shifts promote their scalar inputs to vector type.
if (LHS.get()->getType()->isVectorType() ||
RHS.get()->getType()->isVectorType()) {
-if (LangOpts.OpenCL)
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
if (LangOpts.ZVector) {
// The shift operators for the z vector extensions work basically
- // like OpenCL shifts, except that neither the LHS nor the RHS is
+ // like general shifts, except that neither the LHS nor the RHS is
// allowed to be a "vector bool".
if (auto LHSVecType = LHS.get()->getType()->getAs())
if (LHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
if (auto RHSVecType = RHS.get()->getType()->getAs())
if (RHSVecType->getVectorKind() == VectorType::AltiVecBool)
return InvalidOperands(Loc, LHS, RHS);
- return checkOpenCLVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
+ return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
- /*AllowBothBool*/true,
- /*AllowBoolConversions*/false);
+return checkVectorShift(*this, LHS, RHS, Loc, IsCompAssign);
}
// Shifts don't perform usual arithmetic conversions, they just do integer
Index: llvm/tools/clang/test/Sema/shift.c
===
--- llvm/tools/clang/test/Sema/shift.c
+++ llvm/tools/clang/test/Sema/shift.c
@@ -67,3 +67,24 @@
(void) (x >> 80); // no-warning
(void) (x >> 80); // expected-warning {{shift count >= width of type}}
}
+
+typedef unsigned vec16 __attribute__ ((vector_size (16)));
+typedef unsigned vec8 __attribute__ ((vector_size (8)));
+
+void
+vect_shift_1 (vec16 *x)
+{
+ *x = *x << 4 ;
+}
+
+void
+vect_shift_2 (vec16 *x,vec16 y)
+{
+ *x = *x << y ;
+}
+
+void
+vect_shift_3 (vec16 *x,vec8 y)
+{
+ *x = *x << y ; // expected-error {{vector operands do not have the same
number of elements}}
+}
Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8592,11 +8592,10 @@
<< RHS.get()->getSourceRange();
}
-/// \brief Return the resulting type when an OpenCL vector is shifted
+/// \brief Return the resulting type when an vector is shifted
///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema &S,
- ExprResult &LHS, ExprResult &RHS,
- SourceLocation Loc, bool IsCompAssign) {
+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()) {
S.Diag(Loc, diag::err_shift_rhs_only_vector)
@@ -8636,9 +8635,8 @@
}
if (RHSVecTy) {
-// OpenCL v1.1 s6.3.j says that for vector types, the operators
-// are applied component-wise. So if
