VS: r228382 - OpenCL: handle shift operator with vector operands
Pekka Jääskeläinen
pekka.jaaskelainen at tut.fi
Tue Feb 24 21:17:19 PST 2015
ok. LGTM then.
Pekka
----- Alkuperäinen viesti -----
Lähettäjä: "Sameer Sahasrabuddhe" <sameer.sahasrabuddhe at amd.com>
Lähetetty: 25.2.2015 7:11
Vastaanottaja: "Pekka Jääskeläinen" <pekka.jaaskelainen at tut.fi>; "Jeroen Ketema" <j.ketema at imperial.ac.uk>
Kopio: "llvm cfe" <cfe-commits at cs.uiuc.edu>; "Tom Stellard" <tom at stellard.net>
Aihe: Re: r228382 - OpenCL: handle shift operator with vector operands
Hello Pekka,
I had a look at the current CodeGen tests ... they already cover valid combinations of vector/scalar operands. The change improves Sema to make sure that invalid combinations error out, and valid combinations reach CodeGen. So in short, I can't think of any new test to add to the CodeGen tests, and we should simply submit the new change.
Sameer.
On 2/10/2015 10:13 PM, Pekka Jääskeläinen wrote:
Hello,
I'm not knowledgeable enough on the Clang internals either (still
learning). Anyways, should this test that code is generated
correctly for <<=, not just that Sema doesn't complain about it?
Other than this LGTM.
Pekka
On 02/10/2015 05:54 PM, Sahasrabuddhe, Sameer wrote:
Hello Jeroen,
Thanks for the feedback. Now waiting for an LGTM from Pekka.
Sameer.
------------------------------------------------------------------------------
*From:* Jeroen Ketema [j.ketema at imperial.ac.uk]
*Sent:* Tuesday, February 10, 2015 5:25 PM
*To:* Sahasrabuddhe, Sameer
*Cc:* llvm cfe; Tom Stellard; Pekka Jääskeläinen
*Subject:* Re: r228382 - OpenCL: handle shift operator with vector operands
Hi,
I’m not knowledgeable enough to review the updated patch, but all my test
cases now pass again.
Jeroen
On 09 Feb 2015, at 11:00, Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com
<mailto:sameer.sahasrabuddhe at amd.com>> wrote:
Hi all,
Attached an updated patch that fixes the earlier patch for vector shift
operators in OpenCL. Note the new argument "IsCompAssign" for function
checkOpenCLVectorShift(), which was not handled in earlier patch.
Sameer.
On 2/6/2015 11:02 PM, Tom Stellard wrote:
On Fri, Feb 06, 2015 at 04:05:22PM +0000, Sahasrabuddhe, Sameer wrote:
Hello Jeroen,
Thanks for pointing this out! I will fix this as soon as possible, but in the meantime, request you or anyone with commit access to please revert my commit! I do not have access to my work computer until Monday morning India time. (Reminder to self: *never* commit on a Friday!).
Hi,
I have reverted this. Can you add this test case when you recommit.
Thanks,
Tom
Sameer.
________________________________________
From: Jeroen Ketema [j.ketema at imperial.ac.uk]
Sent: Friday, February 06, 2015 9:15 PM
To: Sahasrabuddhe, Sameer
Cc: llvm cfe
Subject: Re: r228382 - OpenCL: handle shift operator with vector operands
Hi Sameer,
This commit breaks augmented assignment in combination with shifting.
Consider the following:
typedef __attribute__((ext_vector_type(3))) char char3;
void foo() {
char3 v = {1,1,1};
char3 w = {1,2,3};
w <<= v;
}
If I compile with:
clang -x cl file.c
Then an error is produced:
file.c:10:5: error: expression is not assignable
w <<= v;
~ ^
1 error generated.
This while the OpenCL 1.2 spec says that “w <<= v” is short for “w = w << v”,
and which does not produce an error.
Thanks,
Jeroen
On 06 Feb 2015, at 05:44, Sameer Sahasrabuddhe <sameer.sahasrabuddhe atamd.com <http://amd.com>> wrote:
Author: sameerds
Date: Thu Feb 5 23:44:55 2015
New Revision: 228382
URL:http://llvm.org/viewvc/llvm-project?rev=228382&view=rev
Log:
OpenCL: handle shift operator with vector operands
Introduce a number of checks:
1. If LHS is a scalar, then RHS cannot be a vector.
2. Operands must be of integer type.
3. If both are vectors, then the number of elements must match.
Relax the requirement for "usual arithmetic conversions":
When LHS is a vector, a scalar RHS can simply be expanded into a
vector; OpenCL does not require that its rank be lower than the LHS.
For example, the following code is not an error even if the implicit
type of the constant literal is "int".
char2 foo(char2 v) { return v << 1; }
Consolidate existing tests under CodeGenOpenCL, and add more tests
under SemaOpenCL.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CodeGenOpenCL/shifts.cl
cfe/trunk/test/SemaOpenCL/shifts.cl
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 5 23:44:55 2015
@@ -2031,6 +2031,8 @@ def err_typecheck_vector_not_convertable
"can't convert between vector values of different size (%0 and %1)">;
def err_typecheck_vector_not_convertable_non_scalar : Error<
"can't convert between vector and non-scalar values (%0 and %1)">;
+def err_typecheck_vector_lengths_not_equal : Error<
+ "vector operands do not have the same number of elements (%0 and %1)">;
def err_ext_vector_component_exceeds_length : Error<
"vector component access exceeds type %0">;
def err_ext_vector_component_name_illegal : Error<
@@ -4884,6 +4886,8 @@ def err_ivar_reference_type : Error<
"instance variables cannot be of reference type">;
def err_typecheck_illegal_increment_decrement : Error<
"cannot %select{decrement|increment}1 value of type %0">;
+def err_typecheck_expect_int : Error<
+ "used type %0 where integer is required">;
def err_typecheck_arithmetic_incomplete_type : Error<
"arithmetic on a pointer to an incomplete type %0">;
def err_typecheck_pointer_arith_function_type : Error<
@@ -5047,6 +5051,9 @@ def warn_null_in_comparison_operation :
"comparison between NULL and non-pointer "
"%select{(%1 and NULL)|(NULL and %1)}0">,
InGroup<NullArithmetic>;
+def err_shift_rhs_only_vector : Error<
+ "requested shift is a vector of type %0 but the first operand is not a "
+ "vector (%1)">;
def warn_logical_not_on_lhs_of_comparison : Warning<
"logical not is only applied to the left hand side of this comparison">,
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=228382&r1=228381&r2=228382&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 5 23:44:55 2015
@@ -7772,6 +7772,67 @@ static void DiagnoseBadShiftValues(Sema&
<< RHS.get()->getSourceRange();
}
+/// \brief Return the resulting type when an OpenCL vector is shifted
+/// by a scalar or vector shift amount.
+static QualType checkOpenCLVectorShift(Sema &S,
+ ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc) {
+ // 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)
+ << RHS.get()->getType() << LHS.get()->getType()
+ << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+ return QualType();
+ }
+
+ LHS = S.UsualUnaryConversions(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->getAs<VectorType>();
+ QualType LHSEleType = LHSVecTy->getElementType();
+
+ // Note that RHS might not be a vector.
+ QualType RHSType = RHS.get()->getType();
+ const VectorType *RHSVecTy = RHSType->getAs<VectorType>();
+ QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType;
+
+ // OpenCL v1.1 s6.3.j says that the operands need to be integers.
+ if (!LHSEleType->isIntegerType()) {
+ S.Diag(Loc, diag::err_typecheck_expect_int)
+ << LHS.get()->getType() << LHS.get()->getSourceRange();
+ return QualType();
+ }
+
+ if (!RHSEleType->isIntegerType()) {
+ S.Diag(Loc, diag::err_typecheck_expect_int)
+ << RHS.get()->getType() << RHS.get()->getSourceRange();
+ return QualType();
+ }
+
+ 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...
+ if (RHSVecTy->getNumElements() != LHSVecTy->getNumElements()) {
+ S.Diag(Loc, diag::err_typecheck_vector_lengths_not_equal)
+ << LHS.get()->getType() << RHS.get()->getType()
+ << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+ return QualType();
+ }
+ } else {
+ // ...else expand RHS to match the number of elements in LHS.
+ QualType VecTy =
+ �
[Alkuperäinen viesti ei sisälly mukaan kokonaisuudessaan.]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150225/40cbdd02/attachment.html>
More information about the cfe-commits
mailing list