[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

2018-09-06 Thread Andrew Savonichev via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341539: [OpenCL] Disallow negative attribute arguments 
(authored by asavonic, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50259?vs=159022=164190#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50259

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/SemaOpenCL/invalid-kernel-attrs.cl


Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -227,9 +227,13 @@
 
 /// If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
+///
+/// Negative argument is implicitly converted to unsigned, unless
+/// \p StrictlyUnsigned is true.
 template 
 static bool checkUInt32Argument(Sema , const AttrInfo , const Expr *Expr,
-uint32_t , unsigned Idx = UINT_MAX) {
+uint32_t , unsigned Idx = UINT_MAX,
+bool StrictlyUnsigned = false) {
   llvm::APSInt I(32);
   if (Expr->isTypeDependent() || Expr->isValueDependent() ||
   !Expr->isIntegerConstantExpr(I, S.Context)) {
@@ -249,6 +253,11 @@
 return false;
   }
 
+  if (StrictlyUnsigned && I.isSigned() && I.isNegative()) {
+S.Diag(getAttrLoc(AI), diag::err_attribute_argument_negative) << AI;
+return false;
+  }
+
   Val = (uint32_t)I.getZExtValue();
   return true;
 }
@@ -2766,7 +2775,8 @@
   uint32_t WGSize[3];
   for (unsigned i = 0; i < 3; ++i) {
 const Expr *E = AL.getArgAsExpr(i);
-if (!checkUInt32Argument(S, AL, E, WGSize[i], i))
+if (!checkUInt32Argument(S, AL, E, WGSize[i], i,
+ /*StrictlyUnsigned=*/true))
   return;
 if (WGSize[i] == 0) {
   S.Diag(AL.getLoc(), diag::err_attribute_argument_is_zero)
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2529,6 +2529,8 @@
   "constant|a string|an identifier}1">;
 def err_attribute_argument_outof_range : Error<
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_negative : Error<
+  "negative argument is not allowed for %0 attribute">;
 def err_init_priority_object_attr : Error<
   "can only use 'init_priority' attribute on file-scope definitions "
   "of objects of class type">;
Index: test/SemaOpenCL/invalid-kernel-attrs.cl
===
--- test/SemaOpenCL/invalid-kernel-attrs.cl
+++ test/SemaOpenCL/invalid-kernel-attrs.cl
@@ -37,3 +37,10 @@
 __attribute__((intel_reqd_sub_group_size(8))) void kernel14(){} // 
expected-error {{attribute 'intel_reqd_sub_group_size' can only be applied to 
an OpenCL kernel}}
 kernel __attribute__((intel_reqd_sub_group_size(0))) void kernel15(){} // 
expected-error {{'intel_reqd_sub_group_size' attribute must be greater than 0}}
 kernel __attribute__((intel_reqd_sub_group_size(8))) 
__attribute__((intel_reqd_sub_group_size(16))) void kernel16() {}  
//expected-warning{{attribute 'intel_reqd_sub_group_size' is already applied 
with different parameters}}
+
+__kernel __attribute__((work_group_size_hint(8,-16,32))) void neg1() {} 
//expected-error{{negative argument is not allowed for 'work_group_size_hint' 
attribute}}
+__kernel __attribute__((reqd_work_group_size(8,16,-32))) void neg2(){} // 
expected-error{{negative argument is not allowed for 'reqd_work_group_size' 
attribute}}
+
+// 4294967294 is a negative integer if treated as signed.
+// Should compile successfully, since we expect an unsigned.
+__kernel __attribute__((reqd_work_group_size(8,16,4294967294))) void ok1(){}


Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -227,9 +227,13 @@
 
 /// If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
+///
+/// Negative argument is implicitly converted to unsigned, unless
+/// \p StrictlyUnsigned is true.
 template 
 static bool checkUInt32Argument(Sema , const AttrInfo , const Expr *Expr,
-uint32_t , unsigned Idx = UINT_MAX) {
+uint32_t , unsigned Idx = UINT_MAX,
+bool StrictlyUnsigned = false) {
   llvm::APSInt I(32);
   if (Expr->isTypeDependent() || Expr->isValueDependent() ||
   !Expr->isIntegerConstantExpr(I, S.Context)) {
@@ -249,6 +253,11 @@
 return false;
   }
 
+  if (StrictlyUnsigned && I.isSigned() && I.isNegative()) {
+S.Diag(getAttrLoc(AI), diag::err_attribute_argument_negative) << AI;
+return false;
+  }
+

[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

2018-08-17 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D50259



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

2018-08-17 Thread Alexey Bader via Phabricator via cfe-commits 
bader accepted this revision.
bader added a comment.

LGTM.
Please, remove "Change-Id: I910b5c077f5f29e02a1572d9202f0fdbea5280fd" from the 
log message - it not relevant to the project.


Repository:
  rC Clang

https://reviews.llvm.org/D50259



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

2018-08-17 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D50259



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50259: [OpenCL] Disallow negative attribute arguments

2018-08-03 Thread Andrew Savonichev via Phabricator via cfe-commits 
asavonic created this revision.
asavonic added reviewers: Anastasia, yaxunl.
Herald added a subscriber: cfe-commits.

Negative arguments in kernel attributes are silently bitcast'ed to
unsigned, for example:

  __attribute__((reqd_work_group_size(1, -1, 1)))
  __kernel void k() {}

is a complete equivalent of:

  __attribute__((reqd_work_group_size(1, 4294967294, 1)))
  __kernel void k() {}

This is likely an error, so the patch forbids negative arguments in
several OpenCL attributes. Users who really want 4294967294 can still
use it as an unsigned representation.

Change-Id: I910b5c077f5f29e02a1572d9202f0fdbea5280fd


Repository:
  rC Clang

https://reviews.llvm.org/D50259

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclAttr.cpp
  test/SemaOpenCL/invalid-kernel-attrs.cl


Index: test/SemaOpenCL/invalid-kernel-attrs.cl
===
--- test/SemaOpenCL/invalid-kernel-attrs.cl
+++ test/SemaOpenCL/invalid-kernel-attrs.cl
@@ -37,3 +37,10 @@
 __attribute__((intel_reqd_sub_group_size(8))) void kernel14(){} // 
expected-error {{attribute 'intel_reqd_sub_group_size' can only be applied to 
an OpenCL kernel}}
 kernel __attribute__((intel_reqd_sub_group_size(0))) void kernel15(){} // 
expected-error {{'intel_reqd_sub_group_size' attribute must be greater than 0}}
 kernel __attribute__((intel_reqd_sub_group_size(8))) 
__attribute__((intel_reqd_sub_group_size(16))) void kernel16() {}  
//expected-warning{{attribute 'intel_reqd_sub_group_size' is already applied 
with different parameters}}
+
+__kernel __attribute__((work_group_size_hint(8,-16,32))) void neg1() {} 
//expected-error{{negative argument is not allowed for 'work_group_size_hint' 
attribute}}
+__kernel __attribute__((reqd_work_group_size(8,16,-32))) void neg2(){} // 
expected-error{{negative argument is not allowed for 'reqd_work_group_size' 
attribute}}
+
+// 4294967294 is a negative integer if treated as signed.
+// Should compile successfully, since we expect an unsigned.
+__kernel __attribute__((reqd_work_group_size(8,16,4294967294))) void ok1(){}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -239,9 +239,13 @@
 
 /// If Expr is a valid integer constant, get the value of the integer
 /// expression and return success or failure. May output an error.
+///
+/// Negative argument is implicitly converted to unsigned, unless
+/// \p StrictlyUnsigned is true.
 template 
 static bool checkUInt32Argument(Sema , const AttrInfo , const Expr *Expr,
-uint32_t , unsigned Idx = UINT_MAX) {
+uint32_t , unsigned Idx = UINT_MAX,
+bool StrictlyUnsigned = false) {
   llvm::APSInt I(32);
   if (Expr->isTypeDependent() || Expr->isValueDependent() ||
   !Expr->isIntegerConstantExpr(I, S.Context)) {
@@ -262,6 +266,12 @@
 return false;
   }
 
+  if (StrictlyUnsigned && I.isSigned() && I.isNegative()) {
+S.Diag(getAttrLoc(AI), diag::err_attribute_argument_negative)
+<< getAttrName(AI);
+return false;
+  }
+
   Val = (uint32_t)I.getZExtValue();
   return true;
 }
@@ -2793,7 +2803,8 @@
   uint32_t WGSize[3];
   for (unsigned i = 0; i < 3; ++i) {
 const Expr *E = AL.getArgAsExpr(i);
-if (!checkUInt32Argument(S, AL, E, WGSize[i], i))
+if (!checkUInt32Argument(S, AL, E, WGSize[i], i,
+ /*StrictlyUnsigned=*/true))
   return;
 if (WGSize[i] == 0) {
   S.Diag(AL.getLoc(), diag::err_attribute_argument_is_zero)
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2512,6 +2512,8 @@
   "constant|a string|an identifier}1">;
 def err_attribute_argument_outof_range : Error<
   "%0 attribute requires integer constant between %1 and %2 inclusive">;
+def err_attribute_argument_negative : Error<
+  "negative argument is not allowed for %0 attribute">;
 def err_init_priority_object_attr : Error<
   "can only use 'init_priority' attribute on file-scope definitions "
   "of objects of class type">;


Index: test/SemaOpenCL/invalid-kernel-attrs.cl
===
--- test/SemaOpenCL/invalid-kernel-attrs.cl
+++ test/SemaOpenCL/invalid-kernel-attrs.cl
@@ -37,3 +37,10 @@
 __attribute__((intel_reqd_sub_group_size(8))) void kernel14(){} // expected-error {{attribute 'intel_reqd_sub_group_size' can only be applied to an OpenCL kernel}}
 kernel __attribute__((intel_reqd_sub_group_size(0))) void kernel15(){} // expected-error {{'intel_reqd_sub_group_size' attribute must be greater than 0}}
 kernel __attribute__((intel_reqd_sub_group_size(8))) __attribute__((intel_reqd_sub_group_size(16))) void kernel16() {}