[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia marked an inline comment as done.
Anastasia added a comment.

Committed in r 293286


https://reviews.llvm.org/D28814



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/logical-ops.cl:1-3
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+

arsenm wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > Should this have a 2.0 run line for good measure?
> > > > 1.0 too I suppose
> > > Sure!
> > Apparently CL1.0 is not supported! I missed that
> That sounds like a bug?
Yeah, I can't see why it is this way. Probably nobody ever had 1.0 in their 
release...


https://reviews.llvm.org/D29038



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r294313


https://reviews.llvm.org/D29038



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@arsenm, do you think you could complete the review?


https://reviews.llvm.org/D29038



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 85890.
Anastasia marked an inline comment as done.
Anastasia retitled this revision from "[OpenCL] Accept logical NOT for pointer 
types in CL1.0 and CL1.1" to "[OpenCL] Accept logical NOT for pointer types in 
CL1.1".

https://reviews.llvm.org/D29038

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/invalid-logical-ops-1.1.cl
  test/SemaOpenCL/invalid-logical-ops-1.2.cl
  test/SemaOpenCL/logical-ops.cl

Index: test/SemaOpenCL/logical-ops.cl
===
--- /dev/null
+++ test/SemaOpenCL/logical-ops.cl
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL2.0 -triple x86_64-unknown-linux-gnu
+
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
+typedef __attribute__((ext_vector_type(4))) float float4;
+typedef __attribute__((ext_vector_type(4))) double double4;
+typedef __attribute__((ext_vector_type(4))) int int4;
+typedef __attribute__((ext_vector_type(4))) long long4;
+
+kernel void float_ops() {
+  int flaf = 0.0f && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0f || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float fbaf = 0.0f & 0.0f; // expected-error {{invalid operands}}
+  float fbof = 0.0f | 0.0f; // expected-error {{invalid operands}}
+  float fbxf = 0.0f ^ 0.0f; // expected-error {{invalid operands}}
+  int flai = 0.0f && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0f || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
+  float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
+  float bnf = ~0.0f;// expected-error {{invalid argument type}}
+  float lnf = !0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_float_ops() {
+  float4 f4 = (float4)(0, 0, 0, 0);
+  int4 f4laf = f4 && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int4 f4lof = f4 || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float4 f4baf = f4 & 0.0f; // expected-error {{invalid operands}}
+  float4 f4bof = f4 | 0.0f; // expected-error {{invalid operands}}
+  float4 f4bxf = f4 ^ 0.0f; // expected-error {{invalid operands}}
+  float bnf4 = ~f4; // expected-error {{invalid argument type}}
+  int4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void double_ops() {
+  int flaf = 0.0 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double fbaf = 0.0 & 0.0; // expected-error {{invalid operands}}
+  double fbof = 0.0 | 0.0; // expected-error {{invalid operands}}
+  double fbxf = 0.0 ^ 0.0; // expected-error {{invalid operands}}
+  int flai = 0.0 && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0 || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double ibaf = 0 & 0.0; // expected-error {{invalid operands}}
+  double ibof = 0 | 0.0; // expected-error {{invalid operands}}
+  double bnf = ~0.0; // expected-error {{invalid argument type}}
+  double lnf = !0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_double_ops() {
+  double4 f4 = (double4)(0, 0, 0, 0);
+  long4 f4laf = f4 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  long4 f4lof = f4 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double4 f4baf = f4 & 0.0; // expected-error {{invalid operands}}
+  double4 f4bof = f4 | 0.0; // expected-error {{invalid operands}}
+  double4 f4bxf = f4 ^ 0.0; // expected-error {{invalid operands}}
+  double bnf4 = ~f4; // expected-error {{invalid argument type}}
+  long4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void pointer_ops(){
+  global int* p;
+  bool b = !p;
+  b = p==0;
+  int i;
+  b = !
+  b = ==(int *)1;
+}
Index: test/SemaOpenCL/invalid-logical-ops-1.2.cl
===
--- test/SemaOpenCL/invalid-logical-ops-1.2.cl
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
-
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
-
-typedef __attribute__((ext_vector_type(4))) float float4;
-typedef __attribute__((ext_vector_type(4))) double double4;
-typedef 

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/logical-ops.cl:1-3
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+

Anastasia wrote:
> arsenm wrote:
> > arsenm wrote:
> > > Should this have a 2.0 run line for good measure?
> > 1.0 too I suppose
> Sure!
Apparently CL1.0 is not supported! I missed that


https://reviews.llvm.org/D29038



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.0 and CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 2 inline comments as done.
Anastasia added inline comments.



Comment at: test/SemaOpenCL/logical-ops.cl:1-3
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+

arsenm wrote:
> arsenm wrote:
> > Should this have a 2.0 run line for good measure?
> 1.0 too I suppose
Sure!


https://reviews.llvm.org/D29038



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.0 and CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 85878.
Anastasia edited reviewers, added: arsenm; removed: pekka.jaaskelainen.
Anastasia removed a subscriber: arsenm.
Anastasia added a comment.
Herald added a subscriber: wdng.

Added other CL versions to testing (from comment by Matt)!


https://reviews.llvm.org/D29038

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/invalid-logical-ops-1.1.cl
  test/SemaOpenCL/invalid-logical-ops-1.2.cl
  test/SemaOpenCL/logical-ops.cl

Index: test/SemaOpenCL/logical-ops.cl
===
--- /dev/null
+++ test/SemaOpenCL/logical-ops.cl
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.0 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL2.0 -triple x86_64-unknown-linux-gnu
+
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
+typedef __attribute__((ext_vector_type(4))) float float4;
+typedef __attribute__((ext_vector_type(4))) double double4;
+typedef __attribute__((ext_vector_type(4))) int int4;
+typedef __attribute__((ext_vector_type(4))) long long4;
+
+kernel void float_ops() {
+  int flaf = 0.0f && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0f || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float fbaf = 0.0f & 0.0f; // expected-error {{invalid operands}}
+  float fbof = 0.0f | 0.0f; // expected-error {{invalid operands}}
+  float fbxf = 0.0f ^ 0.0f; // expected-error {{invalid operands}}
+  int flai = 0.0f && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0f || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
+  float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
+  float bnf = ~0.0f;// expected-error {{invalid argument type}}
+  float lnf = !0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_float_ops() {
+  float4 f4 = (float4)(0, 0, 0, 0);
+  int4 f4laf = f4 && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int4 f4lof = f4 || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float4 f4baf = f4 & 0.0f; // expected-error {{invalid operands}}
+  float4 f4bof = f4 | 0.0f; // expected-error {{invalid operands}}
+  float4 f4bxf = f4 ^ 0.0f; // expected-error {{invalid operands}}
+  float bnf4 = ~f4; // expected-error {{invalid argument type}}
+  int4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void double_ops() {
+  int flaf = 0.0 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double fbaf = 0.0 & 0.0; // expected-error {{invalid operands}}
+  double fbof = 0.0 | 0.0; // expected-error {{invalid operands}}
+  double fbxf = 0.0 ^ 0.0; // expected-error {{invalid operands}}
+  int flai = 0.0 && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0 || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double ibaf = 0 & 0.0; // expected-error {{invalid operands}}
+  double ibof = 0 | 0.0; // expected-error {{invalid operands}}
+  double bnf = ~0.0; // expected-error {{invalid argument type}}
+  double lnf = !0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_double_ops() {
+  double4 f4 = (double4)(0, 0, 0, 0);
+  long4 f4laf = f4 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  long4 f4lof = f4 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double4 f4baf = f4 & 0.0; // expected-error {{invalid operands}}
+  double4 f4bof = f4 | 0.0; // expected-error {{invalid operands}}
+  double4 f4bxf = f4 ^ 0.0; // expected-error {{invalid operands}}
+  double bnf4 = ~f4; // expected-error {{invalid argument type}}
+  long4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void pointer_ops(){
+  global int* p;
+  bool b = !p;
+  b = p==0;
+  int i;
+  b = !
+  b = ==(int *)1;
+}
Index: test/SemaOpenCL/invalid-logical-ops-1.2.cl
===
--- test/SemaOpenCL/invalid-logical-ops-1.2.cl
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
-
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
-
-typedef 

[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 85888.
Anastasia added a comment.

Updated NULL ptr generation and added separate CodeGen test for checking 
amended Block generation behaviour in OpenCL!


https://reviews.llvm.org/D28814

Files:
  lib/AST/Expr.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/blocks.cl
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  test/SemaOpenCL/invalid-block.cl

Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -4,26 +4,34 @@
 void f0(int (^const bl)());
 // All blocks declarations must be const qualified and initialized.
 void f1() {
-  int (^bl1)() = ^() {return 1;};
-  int (^const bl2)() = ^(){return 1;};
+  int (^bl1)(void) = ^() {
+return 1;
+  };
+  int (^const bl2)(void) = ^() {
+return 1;
+  };
   f0(bl1);
   f0(bl2);
-  bl1 = bl2; // expected-error{{invalid operands to binary expression ('int (^const)()' and 'int (^const)()')}}
+  bl1 = bl2;  // expected-error{{invalid operands to binary expression ('int (__generic ^const)(void)' and 'int (__generic ^const)(void)')}}
   int (^const bl3)(); // expected-error{{invalid block variable declaration - must be initialized}}
 }
 
 // A block with extern storage class is not allowed.
-extern int (^bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+extern int (^bl)(void) = ^() { // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+  return 1;
+};
 void f2() {
-  extern int (^bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+  extern int (^bl)(void) = ^() { // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+return 1;
+  };
 }
 
 // A block cannot be the return value of a function.
 typedef int (^bl_t)(void);
-bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (^const)(void)') is not allowed}}
+bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (__generic ^const)(void)') is not allowed}}
 
 struct bl_s {
-  int (^bl)(void); // expected-error {{the 'int (^const)(void)' type cannot be used to declare a structure or union field}}
+  int (^bl)(void); // expected-error {{the 'int (__generic ^const)(void)' type cannot be used to declare a structure or union field}}
 };
 
 void f4() {
@@ -45,16 +53,16 @@
   bl2_t bl2 = ^(int i) {
 return 2;
   };
-  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (^const)(int)') type is invalid in OpenCL}}
+  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (__generic ^const)(int)') type is invalid in OpenCL}}
   int tmp = i ? bl1(i)  // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
   : bl2(i); // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
 }
 // A block pointer type and all pointer operations are disallowed
-void f6(bl2_t *bl_ptr) { // expected-error{{pointer to type '__generic bl2_t' (aka 'int (^const __generic)(int)') is invalid in OpenCL}}
+void f6(bl2_t *bl_ptr) { // expected-error{{pointer to type '__generic bl2_t' (aka 'int (__generic ^const __generic)(int)') is invalid in OpenCL}}
   bl2_t bl = ^(int i) {
 return 1;
   };
-  bl2_t *p; // expected-error {{pointer to type '__generic bl2_t' (aka 'int (^const __generic)(int)') is invalid in OpenCL}}
-  *bl;  // expected-error {{invalid argument type 'bl2_t' (aka 'int (^const)(int)') to unary expression}}
-// expected-error {{invalid argument type 'bl2_t' (aka 'int (^const)(int)') to unary expression}}
+  bl2_t *p; // expected-error {{pointer to type '__generic bl2_t' (aka 'int (__generic ^const __generic)(int)') is invalid in OpenCL}}
+  *bl;  // expected-error {{invalid argument type 'bl2_t' (aka 'int (__generic ^const)(int)') to unary expression}}
+// expected-error {{invalid argument type 'bl2_t' (aka 'int (__generic ^const)(int)') to unary expression}}
 }
Index: test/CodeGenOpenCL/cl20-device-side-enqueue.cl
===
--- test/CodeGenOpenCL/cl20-device-side-enqueue.cl
+++ test/CodeGenOpenCL/cl20-device-side-enqueue.cl
@@ -2,9 +2,8 @@
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple "spir64-unknown-unknown" | FileCheck %s --check-prefix=COMMON --check-prefix=B64
 
 typedef void (^bl_t)(local void *);
-
 // N.B. The check here only exists to set BL_GLOBAL
-// COMMON: @block_G = {{.*}}bitcast ([[BL_GLOBAL:[^@]+@__block_literal_global(\.[0-9]+)?]]
+// COMMON: @block_G =  addrspace(1) constant void 

[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 2 inline comments as done.
Anastasia added inline comments.



Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:3
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o 
- -triple "spir64-unknown-unknown" | FileCheck %s --check-prefix=COMMON 
--check-prefix=B64
 
 typedef void (^bl_t)(local void *);

yaxunl wrote:
> Can we add a run line for triple amdgcn-amd-amdhsa-opencl to make sure the 
> null pointer is generated correctly?
I added a new test (above). This one gets too complicated!


https://reviews.llvm.org/D28814



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

[PATCH] D28860: [OpenCL] Diagnose write_only image3d when extension is disabled

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r293050!


https://reviews.llvm.org/D28860



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

[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/CodeGen/CGBlocks.cpp:723
+? CGM.getNSConcreteStackBlock()
+: llvm::Constant::getNullValue(
+  CGM.getNSConcreteStackBlock()->getType());

yaxunl wrote:
> should use CGM.getNullPointer to create a null pointer.
Btw, does it mean we can no longer use generic llvm::Constant::getNullValue 
helper for PointerTypes? This feels wrong! Is it possible to extend the helper?

Also I find it a bit counter intuitive to use getNullPointer with the second 
argument QualType for the case like this where we don't have an actual AST 
type. Why is it needed? Some documentation might be helpful here. :) Could we 
extend this helper to use default second argument or an overload with one 
argument only. 


https://reviews.llvm.org/D28814



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

[PATCH] D29830: [OpenCL][Doc] Relase 4.0 notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.

https://reviews.llvm.org/D29830

Files:
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -17,7 +17,7 @@
 Introduction
 
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 4.0.0. Here we
 describe the status of Clang in some detail, including major
 improvements from the previous release and new feature work. For the
@@ -139,7 +139,76 @@
 OpenCL C Language Changes in Clang
 --
 
-...
+**The following bugs in the OpenCL header have been fixed:**
+
+* Added missing ``overloadable`` and ``convergent`` attributes.
+* Removed some erroneous extra ``native_*`` functions.
+
+**The following bugs in the generation of metadata have been fixed:**
+
+* Corrected the SPIR version depending on the OpenCL version.
+* Source level address spaces are taken from the SPIR specification.
+* Image types now contain no access qualifier.
+
+**The following bugs in the AMD target have been fixed:**
+
+* Corrected the bitwidth of ``size_t`` and NULL pointer value with respect to
+  address spaces.
+* Added ``cl_khr_subgroups``, ``cl_amd_media_ops`` and ``cl_amd_media_ops2``
+  extensions.
+* Added ``cl-denorms-are-zero`` support.
+* Changed address spaces for image objects to be ``constant``.
+* Added little-endian.
+
+**The following bugs in OpenCL 2.0 have been fixed:**
+
+* Fixed pipe builtin function return type, added extra argument to generated
+  IR intrinsics to propagate size and alignment information of the pipe packed
+  type.
+* Improved pipe type to accommodate access qualifiers.
+* Added correct address space to the ObjC block generation and 
``enqueue_kernel``
+  prototype.
+* Improved handling of integer parameters of ``enqueue_kernel`` prototype. We
+  now allow ``size_t`` instead of ``int`` for specifying block parameter sizes.
+* Allow using NULL (aka ``CLK_NULL_QUEUE``) with ``queue_t``.
+
+
+**Improved the following diagnostics:**
+
+* Disallow address spaces other than ``global`` for kernel pointer parameters.
+* Correct the use of half type argument and pointer assignment with
+  dereferencing.
+* Disallow variadic arguments in functions and blocks.
+* Allow partial initializer for array and struct.
+
+**Some changes to OpenCL extensions have been made:**
+
+* Added ``cl_khr_mipmap_image``.
+* Added ``-cl-ext`` flag to allow overwriting supported extensions otherwise
+  set by the target compiled for (Example: ``-cl-ext=-all,+cl_khr_fp16``).
+* New types and functions can now be flexibly added to extensions using the
+  following pragmas instead of modifying the Clang source code:
+
+  .. code-block:: c
+
+   #pragma OPENCL EXTENSION the_new_extension_name : begin
+   // declare types and functions associated with the extension here
+   #pragma OPENCL EXTENSION the_new_extension_name : end
+
+
+**Miscellaneous changes:**
+
+* Fix ``__builtin_astype`` to cast between different address space objects.
+* Allow using ``opencl_unroll_hint`` with earlier OpenCL versions than 2.0.
+* Improved handling of floating point literal to default to single precision if
+  fp64 extension is not enabled.
+* Refactor ``sampler_t`` implementation to simplify initializer representation
+  which is now handled as a compiler builtin function with an integer value
+  passed into it.
+* Change fake address space map to use the SPIR convention.
+* Added `the OpenCL manual
+  `_ to Clang
+  documentation.
 
 OpenMP Support in Clang
 --


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -17,7 +17,7 @@
 Introduction
 
 
-This document contains the release notes for the Clang C/C++/Objective-C
+This document contains the release notes for the Clang C/C++/Objective-C/OpenCL
 frontend, part of the LLVM Compiler Infrastructure, release 4.0.0. Here we
 describe the status of Clang in some detail, including major
 improvements from the previous release and new feature work. For the
@@ -139,7 +139,76 @@
 OpenCL C Language Changes in Clang
 --
 
-...
+**The following bugs in the OpenCL header have been fixed:**
+
+* Added missing ``overloadable`` and ``convergent`` attributes.
+* Removed some erroneous extra ``native_*`` functions.
+
+**The following bugs in the generation of metadata have been fixed:**
+
+* Corrected the SPIR version depending on the OpenCL version.
+* Source level address spaces are taken from the SPIR specification.
+* Image types now contain no access qualifier.
+
+**The following bugs in the AMD target have 

[PATCH] D29829: [OpenCL][Doc] Description for adding OpenCL vendor extension in user manual

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.

Adding a description of a new feature that allows to specify vendor extension 
in flexible way using compiler pragma instead of modifying source code directly.

Feature committed in Clang@r289979.


https://reviews.llvm.org/D29829

Files:
  docs/UsersManual.rst


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -2056,6 +2056,8 @@
 In this case the kernel code should contain ``#include `` just as a
 regular C include.
 
+.. _opencl_cl_ext:
+
 .. option:: -cl-ext
 
 Disables support of OpenCL extensions. All OpenCL targets provide a list
@@ -2177,6 +2179,41 @@
 
  $ clang -target spir-unknown-unknown -c -emit-llvm -Xclang 
-finclude-default-header -fmodules -fimplicit-module-maps 
-fmodules-cache-path= test.cl
 
+OpenCL Extensions
+-
+
+All of the ``cl_khr_*`` extensions from `the official OpenCL specification
+`_
+up to and including version 2.0 are available and set per target depending on 
the
+support available in the specific architecture.
+
+It is possible to alter the default extensions setting per target using
+``-cl-ext`` flag. (See :ref:`flags description ` for more 
details).
+
+Vendor extensions can be added flexibly by declaring the list of types and
+functions associated with each extensions enclosed within the following
+compiler pragma directives:
+
+  .. code-block:: c
+
+   #pragma OPENCL EXTENSION the_new_extension_name : begin
+   // declare types and functions associated with the extension here
+   #pragma OPENCL EXTENSION the_new_extension_name : end
+
+For example, parsing the following code adds ``my_t`` type and ``my_func``
+function to the custom ``my_ext`` extension.
+
+  .. code-block:: c
+
+   #pragma OPENCL EXTENSION my_ext : begin
+   typedef struct{
+ int a;
+   }my_t;
+   void my_func(my_t);
+   #pragma OPENCL EXTENSION my_ext : end
+
+Declaring the same types in different vendor extensions is disallowed.
+
 OpenCL Metadata
 ---
 
@@ -2215,7 +2252,7 @@
 `_
 
 
-opencl_hint_unroll
+opencl_unroll_hint
 ^^
 
 The implementation of this feature mirrors the unroll hint for C.


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -2056,6 +2056,8 @@
 In this case the kernel code should contain ``#include `` just as a
 regular C include.
 
+.. _opencl_cl_ext:
+
 .. option:: -cl-ext
 
 Disables support of OpenCL extensions. All OpenCL targets provide a list
@@ -2177,6 +2179,41 @@
 
  $ clang -target spir-unknown-unknown -c -emit-llvm -Xclang -finclude-default-header -fmodules -fimplicit-module-maps -fmodules-cache-path= test.cl
 
+OpenCL Extensions
+-
+
+All of the ``cl_khr_*`` extensions from `the official OpenCL specification
+`_
+up to and including version 2.0 are available and set per target depending on the
+support available in the specific architecture.
+
+It is possible to alter the default extensions setting per target using
+``-cl-ext`` flag. (See :ref:`flags description ` for more details).
+
+Vendor extensions can be added flexibly by declaring the list of types and
+functions associated with each extensions enclosed within the following
+compiler pragma directives:
+
+  .. code-block:: c
+
+   #pragma OPENCL EXTENSION the_new_extension_name : begin
+   // declare types and functions associated with the extension here
+   #pragma OPENCL EXTENSION the_new_extension_name : end
+
+For example, parsing the following code adds ``my_t`` type and ``my_func``
+function to the custom ``my_ext`` extension.
+
+  .. code-block:: c
+
+   #pragma OPENCL EXTENSION my_ext : begin
+   typedef struct{
+ int a;
+   }my_t;
+   void my_func(my_t);
+   #pragma OPENCL EXTENSION my_ext : end
+
+Declaring the same types in different vendor extensions is disallowed.
+
 OpenCL Metadata
 ---
 
@@ -2215,7 +2252,7 @@
 `_
 
 
-opencl_hint_unroll
+opencl_unroll_hint
 ^^
 
 The implementation of this feature mirrors the unroll hint for C.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29764: [OpenCL] Blocks cannot capture/reference another block

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D29764#673679, @yaxunl wrote:

> In https://reviews.llvm.org/D29764#673548, @Nicola wrote:
>
> > Looking at "Example 4" in the standard it looks like this should also be 
> > illegal.
> >
> >   int (^block1)(void) = ^int {return 1;};
> >   int foo() { return block1(); }
> >  
> >   __kernel void k(global int *z)
> >   {
> >int (^block2)(void) = ^int {
> > return foo(); // expected-error {{cannot refer to a block inside block}}
> >}; 
> >   }
> >  
> >
> >
> > Unless I missed something it's not erroring in this case.
>
>
> To diagnose this needs to traverse the AST tree. I think it is too much to do 
> it during parsing.
>
> It may be done through static analysis though.




In https://reviews.llvm.org/D29764#673679, @yaxunl wrote:

> In https://reviews.llvm.org/D29764#673548, @Nicola wrote:
>
> > Looking at "Example 4" in the standard it looks like this should also be 
> > illegal.
> >
> >   int (^block1)(void) = ^int {return 1;};
> >   int foo() { return block1(); }
> >  
> >   __kernel void k(global int *z)
> >   {
> >int (^block2)(void) = ^int {
> > return foo(); // expected-error {{cannot refer to a block inside block}}
> >}; 
> >   }
> >  
> >
> >
> > Unless I missed something it's not erroring in this case.
>
>
> To diagnose this needs to traverse the AST tree. I think it is too much to do 
> it during parsing.
>
> It may be done through static analysis though.


Yes, it will require quite some expensive checks to implement visiting the AST 
to trace back the block variables declared in other functions. Additionally 
this example doesn't seem to cause any issues really. It is mainly in the case 
with blocks captured from the stack:

  kernel void foo() {
bl2_t bl1 = ^(int i) {
  return 1;
};
void (^bl2)(void) = ^{
  int i = bl1(1); // expected-error {{cannot refer to a block inside block}}
};
  }

}
for which ObjC implementation creates copy/destroy helpers that use some 
symbols that are supposed to be defined elsewhere (presumably in the ObjC 
runtime) which causes issue in OpenCL because we suddenly end up with undefined 
symbols. I am guessing this is needed in order to promote stack allocated 
blocks into heap. Although the copy itself doesn't happen in our cases.




Comment at: test/SemaOpenCL/invalid-block.cl:71
+kernel void foobars() {
+  int v0;
+  bl2_t bl1 = ^(int i) {

v0 is not used!


https://reviews.llvm.org/D29764



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

[PATCH] D29764: [OpenCL] Blocks cannot capture/reference another block

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Herald added a subscriber: yaxunl.

Adding the last restriction from s6.12.5 OpenCL C v2.0.

"A Block cannot reference or capture another Block variable declared in the 
outer scope".


https://reviews.llvm.org/D29764

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/invalid-block.cl


Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -66,3 +66,19 @@
   *bl;  // expected-error {{invalid argument type 'bl2_t' (aka 'int 
(__generic ^const)(int)') to unary expression}}
 // expected-error {{invalid argument type 'bl2_t' (aka 'int 
(__generic ^const)(int)') to unary expression}}
 }
+// A block can't reference another block
+kernel void foobars() {
+  int v0;
+  bl2_t bl1 = ^(int i) {
+return 1;
+  };
+  void (^bl2)(void) = ^{
+int i = bl1(1); // expected-error {{cannot refer to a block inside block}}
+  };
+  void (^bl3)(void) = ^{
+  };
+  void (^bl4)(void) = ^{
+bl3(); // expected-error {{cannot refer to a block inside block}}
+  };
+  return;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13583,6 +13583,13 @@
 }
 return false;
   }
+  // OpenCL v2.0 s6.12.5: Blocks cannot reference/capture other blocks
+  if (S.getLangOpts().OpenCL && IsBlock &&
+  Var->getType()->isBlockPointerType()) {
+if (Diagnose)
+  S.Diag(Loc, diag::err_opencl_block_ref_block);
+return false;
+  }
 
   return true;
 }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8231,6 +8231,8 @@
   "invalid block variable declaration - must be %select{const 
qualified|initialized}0">;
 def err_opencl_extern_block_declaration : Error<
   "invalid block variable declaration - using 'extern' storage class is 
disallowed">;
+def err_opencl_block_ref_block : Error<
+  "cannot refer to a block inside block">;
 
 // OpenCL v2.0 s6.13.9 - Address space qualifier functions. 
 def err_opencl_builtin_to_addr_arg_num : Error<


Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -66,3 +66,19 @@
   *bl;  // expected-error {{invalid argument type 'bl2_t' (aka 'int (__generic ^const)(int)') to unary expression}}
 // expected-error {{invalid argument type 'bl2_t' (aka 'int (__generic ^const)(int)') to unary expression}}
 }
+// A block can't reference another block
+kernel void foobars() {
+  int v0;
+  bl2_t bl1 = ^(int i) {
+return 1;
+  };
+  void (^bl2)(void) = ^{
+int i = bl1(1); // expected-error {{cannot refer to a block inside block}}
+  };
+  void (^bl3)(void) = ^{
+  };
+  void (^bl4)(void) = ^{
+bl3(); // expected-error {{cannot refer to a block inside block}}
+  };
+  return;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13583,6 +13583,13 @@
 }
 return false;
   }
+  // OpenCL v2.0 s6.12.5: Blocks cannot reference/capture other blocks
+  if (S.getLangOpts().OpenCL && IsBlock &&
+  Var->getType()->isBlockPointerType()) {
+if (Diagnose)
+  S.Diag(Loc, diag::err_opencl_block_ref_block);
+return false;
+  }
 
   return true;
 }
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8231,6 +8231,8 @@
   "invalid block variable declaration - must be %select{const qualified|initialized}0">;
 def err_opencl_extern_block_declaration : Error<
   "invalid block variable declaration - using 'extern' storage class is disallowed">;
+def err_opencl_block_ref_block : Error<
+  "cannot refer to a block inside block">;
 
 // OpenCL v2.0 s6.13.9 - Address space qualifier functions. 
 def err_opencl_builtin_to_addr_arg_num : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@Pekka, do you think there is something more to be done here or can I close 
this revision now?

Apparently, I won't be able to pass` -target` in all cases because `-cc` has to 
be always the first parameter.

Also I am preparing additional text and some minor corrections in another 
review.


https://reviews.llvm.org/D28080



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

[PATCH] D28058: [OpenCL] Correct ndrange_t implementation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in 295311


https://reviews.llvm.org/D28058



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

[PATCH] D29830: [OpenCL][Doc] Relase 4.0 notes for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed to release40@295315


https://reviews.llvm.org/D29830



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

[PATCH] D29764: [OpenCL] Blocks cannot capture/reference another block

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r295307!


https://reviews.llvm.org/D29764



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

[PATCH] D29829: [OpenCL][Doc] Description for adding OpenCL vendor extension in user manual

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in 295313!


https://reviews.llvm.org/D29829



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

> (I should pass -finclude-default-header after -cc1 without -Xclang then it 
> works, but I think also the
>  latter version should work, no?)

In general -cc1 and -Xclang have similar effect so I either should work indeed.

> Also, I cannot add -S (even with -emit-llvm), then it complains about a 
> missing target:
> 
>   $ clang -cc1 -triple spir64-unknown-unknown ~/temp/local.cl -S
>   error: unable to create target: 'No available targets are compatible with 
> this triple.'
> 
> 
> I think -S used to work with -emit-llvm as a switch to output LLVM text 
> instead of bitcode, but now it seems to
>  output text by default so it should not be used.

I think this has to do with spir being a 'generic' target.

> Is there are reason to use/document the -cc1 -triple instead of
>  -target here?

Yes, because in some cases we have added frontend flag only, so we need to 
either pass -cc1 or -Xclang. But I could change the documentation to try to 
pass -target before the -cc1 flag.

> Also, for me the command without -emit-llvm doesn't output anything.

What targets do you use? I think the problem is that it's not clear what the 
default output of the standalone compiler should be for the most of the OpenCL 
targets, since it does't produce a valid binary. If let's say we emit an x86 
binary, would it be valid to run it for POCL directly?  Perhaps, we could try 
to emit the bitcode by default then.

Currently if I run:

  clang test.cl

I get an error:  undefined reference to `main'. But if I add a main (which 
isn't really valid for OpenCL) I get:
error: function cannot be called 'main'


https://reviews.llvm.org/D28080



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

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.0 and CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Herald added a subscriber: yaxunl.

A bug is reported regarding incorrect error given for logical NOT operation 
with a pointer type:
https://llvm.org/bugs/show_bug.cgi?id=30217

Detailed investigation shows that this has only been a problem for CL earlier 
than v1.2.

This patch fixes the issue as well as improves and simplifies related test.


https://reviews.llvm.org/D29038

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/invalid-logical-ops-1.1.cl
  test/SemaOpenCL/invalid-logical-ops-1.2.cl

Index: test/SemaOpenCL/invalid-logical-ops-1.2.cl
===
--- test/SemaOpenCL/invalid-logical-ops-1.2.cl
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
-
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
-
-typedef __attribute__((ext_vector_type(4))) float float4;
-typedef __attribute__((ext_vector_type(4))) double double4;
-typedef __attribute__((ext_vector_type(4))) int int4;
-typedef __attribute__((ext_vector_type(4))) long long4;
-
-kernel void float_ops() {
-  int flaf = 0.0f && 0.0f;
-  int flof = 0.0f || 0.0f;
-  float fbaf = 0.0f & 0.0f; // expected-error {{invalid operands}}
-  float fbof = 0.0f | 0.0f; // expected-error {{invalid operands}}
-  float fbxf = 0.0f ^ 0.0f; // expected-error {{invalid operands}}
-  int flai = 0.0f && 0;
-  int floi = 0.0f || 0;
-  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
-  float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
-  float bnf = ~0.0f;// expected-error {{invalid argument type}}
-  float lnf = !0.0f;
-}
-
-kernel void vec_float_ops() {
-  float4 f4 = (float4)(0, 0, 0, 0);
-  int4 f4laf = f4 && 0.0f;
-  int4 f4lof = f4 || 0.0f;
-  float4 f4baf = f4 & 0.0f; // expected-error {{invalid operands}}
-  float4 f4bof = f4 | 0.0f; // expected-error {{invalid operands}}
-  float4 f4bxf = f4 ^ 0.0f; // expected-error {{invalid operands}}
-  float bnf4 = ~f4; // expected-error {{invalid argument type}}
-  int4 lnf4 = !f4;
-}
-
-kernel void double_ops() {
-  int flaf = 0.0 && 0.0;
-  int flof = 0.0 || 0.0;
-  double fbaf = 0.0 & 0.0; // expected-error {{invalid operands}}
-  double fbof = 0.0 | 0.0; // expected-error {{invalid operands}}
-  double fbxf = 0.0 ^ 0.0; // expected-error {{invalid operands}}
-  int flai = 0.0 && 0;
-  int floi = 0.0 || 0;
-  double ibaf = 0 & 0.0; // expected-error {{invalid operands}}
-  double ibof = 0 | 0.0; // expected-error {{invalid operands}}
-  double bnf = ~0.0; // expected-error {{invalid argument type}}
-  double lnf = !0.0;
-}
-
-kernel void vec_double_ops() {
-  double4 f4 = (double4)(0, 0, 0, 0);
-  long4 f4laf = f4 && 0.0;
-  long4 f4lof = f4 || 0.0;
-  double4 f4baf = f4 & 0.0; // expected-error {{invalid operands}}
-  double4 f4bof = f4 | 0.0; // expected-error {{invalid operands}}
-  double4 f4bxf = f4 ^ 0.0; // expected-error {{invalid operands}}
-  double bnf4 = ~f4; // expected-error {{invalid argument type}}
-  long4 lnf4 = !f4;
-}
Index: test/SemaOpenCL/invalid-logical-ops-1.1.cl
===
--- test/SemaOpenCL/invalid-logical-ops-1.1.cl
+++ /dev/null
@@ -1,57 +0,0 @@
-// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
-
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
-typedef __attribute__((ext_vector_type(4))) float float4;
-typedef __attribute__((ext_vector_type(4))) double double4;
-typedef __attribute__((ext_vector_type(4))) int int4;
-typedef __attribute__((ext_vector_type(4))) long long4;
-
-kernel void float_ops() {
-  int flaf = 0.0f && 0.0f; // expected-error {{invalid operands}}
-  int flof = 0.0f || 0.0f; // expected-error {{invalid operands}}
-  float fbaf = 0.0f & 0.0f; // expected-error {{invalid operands}}
-  float fbof = 0.0f | 0.0f; // expected-error {{invalid operands}}
-  float fbxf = 0.0f ^ 0.0f; // expected-error {{invalid operands}}
-  int flai = 0.0f && 0; // expected-error {{invalid operands}}
-  int floi = 0.0f || 0; // expected-error {{invalid operands}}
-  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
-  float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
-  float bnf = ~0.0f; // expected-error {{invalid argument type}}
-  float lnf = !0.0f; // expected-error {{invalid argument type}}
-}
-
-kernel void vec_float_ops() {
-  float4 f4 = (float4)(0, 0, 0, 0);
-  int4 f4laf = f4 && 0.0f; // expected-error {{invalid operands}}
-  int4 f4lof = f4 || 0.0f; // expected-error {{invalid operands}}
-  float4 f4baf = f4 & 0.0f; // expected-error {{invalid operands}}
-  float4 f4bof = f4 | 0.0f; // expected-error {{invalid operands}}
-  float4 f4bxf = f4 ^ 0.0f; // expected-error {{invalid operands}}
-  float bnf4 = ~f4; // expected-error {{invalid argument type}}
-  int4 lnf4 = !f4; // expected-error {{invalid argument type}}
-}
-
-kernel void double_ops() {
-  int flaf = 0.0 && 0.0; // expected-error {{invalid operands}}
-  

[PATCH] D29038: [OpenCL] Accept logical NOT for pointer types in CL1.0 and CL1.1

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 85418.
Anastasia added a comment.

Added missing test!


https://reviews.llvm.org/D29038

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/invalid-logical-ops-1.1.cl
  test/SemaOpenCL/invalid-logical-ops-1.2.cl
  test/SemaOpenCL/logical-ops.cl

Index: test/SemaOpenCL/logical-ops.cl
===
--- /dev/null
+++ test/SemaOpenCL/logical-ops.cl
@@ -0,0 +1,116 @@
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.1 -triple x86_64-unknown-linux-gnu
+// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
+
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable
+
+typedef __attribute__((ext_vector_type(4))) float float4;
+typedef __attribute__((ext_vector_type(4))) double double4;
+typedef __attribute__((ext_vector_type(4))) int int4;
+typedef __attribute__((ext_vector_type(4))) long long4;
+
+kernel void float_ops() {
+  int flaf = 0.0f && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0f || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float fbaf = 0.0f & 0.0f; // expected-error {{invalid operands}}
+  float fbof = 0.0f | 0.0f; // expected-error {{invalid operands}}
+  float fbxf = 0.0f ^ 0.0f; // expected-error {{invalid operands}}
+  int flai = 0.0f && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0f || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float ibaf = 0 & 0.0f; // expected-error {{invalid operands}}
+  float ibof = 0 | 0.0f; // expected-error {{invalid operands}}
+  float bnf = ~0.0f;// expected-error {{invalid argument type}}
+  float lnf = !0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_float_ops() {
+  float4 f4 = (float4)(0, 0, 0, 0);
+  int4 f4laf = f4 && 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int4 f4lof = f4 || 0.0f;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  float4 f4baf = f4 & 0.0f; // expected-error {{invalid operands}}
+  float4 f4bof = f4 | 0.0f; // expected-error {{invalid operands}}
+  float4 f4bxf = f4 ^ 0.0f; // expected-error {{invalid operands}}
+  float bnf4 = ~f4; // expected-error {{invalid argument type}}
+  int4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void double_ops() {
+  int flaf = 0.0 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int flof = 0.0 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double fbaf = 0.0 & 0.0; // expected-error {{invalid operands}}
+  double fbof = 0.0 | 0.0; // expected-error {{invalid operands}}
+  double fbxf = 0.0 ^ 0.0; // expected-error {{invalid operands}}
+  int flai = 0.0 && 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  int floi = 0.0 || 0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double ibaf = 0 & 0.0; // expected-error {{invalid operands}}
+  double ibof = 0 | 0.0; // expected-error {{invalid operands}}
+  double bnf = ~0.0; // expected-error {{invalid argument type}}
+  double lnf = !0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void vec_double_ops() {
+  double4 f4 = (double4)(0, 0, 0, 0);
+  long4 f4laf = f4 && 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  long4 f4lof = f4 || 0.0;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid operands}}
+#endif
+  double4 f4baf = f4 & 0.0; // expected-error {{invalid operands}}
+  double4 f4bof = f4 | 0.0; // expected-error {{invalid operands}}
+  double4 f4bxf = f4 ^ 0.0; // expected-error {{invalid operands}}
+  double bnf4 = ~f4; // expected-error {{invalid argument type}}
+  long4 lnf4 = !f4;
+#if __OPENCL_C_VERSION__ < 120
+// expected-error@-2{{invalid argument type}}
+#endif
+}
+
+kernel void pointer_ops(){
+  global int* p;
+  bool b = !p;
+  b = p==0;
+  int i;
+  b = !
+  b = ==(int *)1;
+}
Index: test/SemaOpenCL/invalid-logical-ops-1.2.cl
===
--- test/SemaOpenCL/invalid-logical-ops-1.2.cl
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %clang_cc1 %s -verify -cl-std=CL1.2 -triple x86_64-unknown-linux-gnu
-
-#pragma OPENCL EXTENSION cl_khr_fp64 : enable
-
-typedef __attribute__((ext_vector_type(4))) float float4;
-typedef __attribute__((ext_vector_type(4))) double double4;
-typedef __attribute__((ext_vector_type(4))) int int4;
-typedef __attribute__((ext_vector_type(4))) long long4;
-
-kernel void float_ops() {
-  int flaf = 0.0f && 0.0f;
-  int flof = 0.0f || 0.0f;
-  float fbaf = 0.0f & 0.0f; // expected-error {{invalid 

[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Ping! @yaxunl, Sam do you think you will have time to look into this?


https://reviews.llvm.org/D28814



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

[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.

ObjC IR generation for Blocks currently:
I. Generates local to block variable declaration block literal in case it 
contains captures.
II. Global variable block literal in case it doesn't have any captures.

The address spaces are missing however if we use this generation for OpenCL.

This patch:

- keeps default private address space for blocks generated as local variables 
(case I from above);
- adds global address space for global block literals (case II from the above);
- makes the block invoke function and enqueue_kernel prototype with the generic 
AS block pointer parameter to accommodate both private and global AS cases.
- adds block handling into default AS because it's implemented as a special 
pointer type (BlockPointer) in the frontend and therefore it is used as a 
pointer everywhere. This is also needed to accommodate both private and global 
address space blocks for the two cases described above.
- removes ObjC RT specific symbols (NSConcreteStackBlock and 
NSConcreteGlobalBlock) in the OpenCL mode.


https://reviews.llvm.org/D28814

Files:
  lib/AST/Expr.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCL/cl20-device-side-enqueue.cl
  test/SemaOpenCL/invalid-block.cl

Index: test/SemaOpenCL/invalid-block.cl
===
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -4,26 +4,34 @@
 void f0(int (^const bl)());
 // All blocks declarations must be const qualified and initialized.
 void f1() {
-  int (^bl1)() = ^() {return 1;};
-  int (^const bl2)() = ^(){return 1;};
+  int (^bl1)(void) = ^() {
+return 1;
+  };
+  int (^const bl2)(void) = ^() {
+return 1;
+  };
   f0(bl1);
   f0(bl2);
-  bl1 = bl2; // expected-error{{invalid operands to binary expression ('int (^const)()' and 'int (^const)()')}}
+  bl1 = bl2;  // expected-error{{invalid operands to binary expression ('int (__generic ^const)(void)' and 'int (__generic ^const)(void)')}}
   int (^const bl3)(); // expected-error{{invalid block variable declaration - must be initialized}}
 }
 
 // A block with extern storage class is not allowed.
-extern int (^bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+extern int (^bl)(void) = ^() {
+  return 1;
+}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
 void f2() {
-  extern int (^bl)() = ^(){return 1;}; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
+  extern int (^bl)(void) = ^() {
+return 1;
+  }; // expected-error{{invalid block variable declaration - using 'extern' storage class is disallowed}}
 }
 
 // A block cannot be the return value of a function.
 typedef int (^bl_t)(void);
-bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (^const)(void)') is not allowed}}
+bl_t f3(bl_t bl); // expected-error{{declaring function return value of type 'bl_t' (aka 'int (__generic ^const)(void)') is not allowed}}
 
 struct bl_s {
-  int (^bl)(void); // expected-error {{the 'int (^const)(void)' type cannot be used to declare a structure or union field}}
+  int (^bl)(void); // expected-error {{the 'int (__generic ^const)(void)' type cannot be used to declare a structure or union field}}
 };
 
 void f4() {
@@ -45,16 +53,16 @@
   bl2_t bl2 = ^(int i) {
 return 2;
   };
-  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (^const)(int)') type is invalid in OpenCL}}
+  bl2_t arr[] = {bl1, bl2}; // expected-error {{array of 'bl2_t' (aka 'int (__generic ^const)(int)') type is invalid in OpenCL}}
   int tmp = i ? bl1(i)  // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
   : bl2(i); // expected-error {{block type cannot be used as expression in ternary expression in OpenCL}}
 }
 // A block pointer type and all pointer operations are disallowed
-void f6(bl2_t *bl_ptr) { // expected-error{{pointer to type '__generic bl2_t' (aka 'int (^const __generic)(int)') is invalid in OpenCL}}
+void f6(bl2_t *bl_ptr) { // expected-error{{pointer to type '__generic bl2_t' (aka 'int (__generic ^const __generic)(int)') is invalid in OpenCL}}
   bl2_t bl = ^(int i) {
 return 1;
   };
-  bl2_t *p; // expected-error {{pointer to type '__generic bl2_t' (aka 'int (^const __generic)(int)') is invalid in OpenCL}}
-  *bl;  // expected-error {{invalid argument type 'bl2_t' (aka 'int (^const)(int)') to unary expression}}
-// expected-error {{invalid argument type 'bl2_t' (aka 'int (^const)(int)') to unary expression}}
+  bl2_t *p; // expected-error {{pointer to type '__generic bl2_t' (aka 'int (__generic ^const __generic)(int)') is invalid in OpenCL}}
+  *bl;  // expected-error {{invalid argument type 

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: docs/UsersManual.rst:2065
+
+ $ clang -cc1 -triple spir64-unknown-unknown -cl-ext=-cl_khr_fp64 test.cl
+

pekka.jaaskelainen wrote:
> Is this correct? I cannot make it work:
> 
> ```
> ~/local/stow/llvm-4.0-unpatched-Debug/bin/clang -Xclang 
> -finclude-default-header -emit-llvm -cc1 -triple spir64-unknown-unknown 
> kernel/test_halfs.cl -c -S -o -
> clang-4.0: error: unknown argument: '-cc1'
> clang-4.0: error: unknown argument: '-triple'
> clang-4.0: error: no such file or directory: 'spir64-unknown-unknown'
> ```
> 
> -target works instead. (But reveals another issue, there's no printf() 
> declaration, should I file a bug?)
> 
> ```
> ~/local/stow/llvm-4.0-unpatched-Debug/bin/clang -Xclang 
> -finclude-default-header -emit-llvm -target spir64-unknown-unknown 
> kernel/test_halfs.cl -c -S -o -
> kernel/test_halfs.cl:10:9: warning: implicit declaration of function 'printf' 
> is invalid in C99 [-Wimplicit-function-declaration]
> printf("max(a,b)[0] type=uint2 a=0x15b348c9 b=0xf88e7d07 
> want=0xf88e7d07 got=%x\n", max_[0]);
> ^
> kernel/test_halfs.cl:10:9: error: function with no prototype cannot use the 
> spir_function calling convention
> 1 warning and 1 error generated.
> ```
> 
> With 3.9 that kernel which calls printf() passes:
> ```
> /local/stow/llvm-3.9-debug/bin/clang -Xclang -finclude-default-header 
> -emit-llvm -target spir64-unknown-unknown kernel/test_halfs.cl -c -S -o -
> ; ModuleID = 'kernel/test_halfs.cl'
> source_filename = "kernel/test_halfs.cl"
> target datalayout = 
> "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
> target triple = "spir64-unknown-unknown"
> 
> ```
> 
> 
> 
It should be -triple with -cc1 and -target without.

  clang -cc1 -triple ...
  clang -target ...

We have disabled the C99 builtin functions as per spec v1.2 s6.9.f, but the 
OpenCL builtins should be available via the header now.

As I can see declaration of printf is in the header under CL1.2 and higher.

Could you try this command instead:
  clang -std=CL1.2 test.cl -Xclang -finclude-default-header



https://reviews.llvm.org/D28080



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

@pekka.jaaskelainen, I just compiled the release 4.0 branch and it all works 
for me as expected:
clang -cc1 -triple spir64-unknown-unknown test.cl
clang  -target spir64-unknown-unknown test.cl


https://reviews.llvm.org/D28080



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

[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:5924
 
-  DeclContext *OriginalDC = DC;
-  bool IsLocalExternDecl = SC == SC_Extern &&
-   adjustContextForLocalExternDecl(DC);
+// OpenCL 1.2 spec, p6.9 r:
+// The event type cannot be used to declare a program scope variable.

The format is typically:
OpenCL v1.2 s6.9.r



Comment at: lib/Sema/SemaDecl.cpp:5926
+// The event type cannot be used to declare a program scope variable.
+// OpenCL v2.0 s6.9.q The clk_event_t and reserve_id_t types cannot be 
declared in program scope.
+if (NULL == S->getParent()) {

This line looks inconsistent with above! Put ":" after OpenCL v2.0 s6.9.q.

Also formatting seems off. At least the line width.


https://reviews.llvm.org/D27099



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

[PATCH] D27109: [OpenCL] Prevent generation of globals in non-constant address space for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 79403.
Anastasia added a comment.

Merged if statements into one!


https://reviews.llvm.org/D27109

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenOpenCL/constant-addr-space-globals.cl


Index: test/CodeGenOpenCL/constant-addr-space-globals.cl
===
--- test/CodeGenOpenCL/constant-addr-space-globals.cl
+++ test/CodeGenOpenCL/constant-addr-space-globals.cl
@@ -6,3 +6,22 @@
 kernel void test(global float *out) {
   *out = array[0];
 }
+
+// Test that we don't use directly initializers for const aggregates
+// but create a copy in the original address space (unless a variable itself is
+// in the constant address space).
+
+void foo(constant const int *p1, const int *p2, const int *p3);
+// CHECK: @k.arr1 = internal addrspace(3) constant [3 x i32] [i32 1, i32 2, 
i32 3]
+// CHECK: @k.arr2 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 
4, i32 5, i32 6]
+// CHECK: @k.arr3 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 
7, i32 8, i32 9]
+kernel void k(void) {
+  // CHECK-NOT: %arr1 = alloca [3 x i32]
+  constant const int arr1[] = {1, 2, 3};
+  // CHECK: %arr2 = alloca [3 x i32]
+  const int arr2[] = {4, 5, 6};
+  // CHECK: %arr3 = alloca [3 x i32]
+  int arr3[] = {7, 8, 9};
+
+  foo(arr1, arr2, arr3);
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -948,8 +948,12 @@
   // If the variable's a const type, and it's neither an NRVO
   // candidate nor a __block variable and has no mutable members,
   // emit it as a global instead.
-  if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
-  CGM.isTypeConstant(Ty, true)) {
+  // Exception is if a variable is located in non-constant address space
+  // in OpenCL.
+  if ((!getLangOpts().OpenCL ||
+   Ty.getAddressSpace() == LangAS::opencl_constant) &&
+  (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
+   CGM.isTypeConstant(Ty, true))) {
 EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
 
 // Signal this condition to later callbacks.


Index: test/CodeGenOpenCL/constant-addr-space-globals.cl
===
--- test/CodeGenOpenCL/constant-addr-space-globals.cl
+++ test/CodeGenOpenCL/constant-addr-space-globals.cl
@@ -6,3 +6,22 @@
 kernel void test(global float *out) {
   *out = array[0];
 }
+
+// Test that we don't use directly initializers for const aggregates
+// but create a copy in the original address space (unless a variable itself is
+// in the constant address space).
+
+void foo(constant const int *p1, const int *p2, const int *p3);
+// CHECK: @k.arr1 = internal addrspace(3) constant [3 x i32] [i32 1, i32 2, i32 3]
+// CHECK: @k.arr2 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 4, i32 5, i32 6]
+// CHECK: @k.arr3 = private unnamed_addr addrspace(3) constant [3 x i32] [i32 7, i32 8, i32 9]
+kernel void k(void) {
+  // CHECK-NOT: %arr1 = alloca [3 x i32]
+  constant const int arr1[] = {1, 2, 3};
+  // CHECK: %arr2 = alloca [3 x i32]
+  const int arr2[] = {4, 5, 6};
+  // CHECK: %arr3 = alloca [3 x i32]
+  int arr3[] = {7, 8, 9};
+
+  foo(arr1, arr2, arr3);
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -948,8 +948,12 @@
   // If the variable's a const type, and it's neither an NRVO
   // candidate nor a __block variable and has no mutable members,
   // emit it as a global instead.
-  if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
-  CGM.isTypeConstant(Ty, true)) {
+  // Exception is if a variable is located in non-constant address space
+  // in OpenCL.
+  if ((!getLangOpts().OpenCL ||
+   Ty.getAddressSpace() == LangAS::opencl_constant) &&
+  (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
+   CGM.isTypeConstant(Ty, true))) {
 EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
 
 // Signal this condition to later callbacks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27099: [OpenCL] Prohibit using reserve_id_t in program scope.

[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! Thanks! Please, address the small nitpick before committing.




Comment at: lib/Sema/SemaDecl.cpp:5965
+
+// OpenCL 1.2 spec, p6.9 r:
+// The event type cannot be used with the __local, __constant and __global

 OpenCL 1.2 spec, p6.9 r ->  OpenCL v1.2 s6.9.r


https://reviews.llvm.org/D27099



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

[PATCH] D27453: [OpenCL] Enable unroll hint for OpenCL 1.x.

[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! Thanks!


https://reviews.llvm.org/D27453



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

[PATCH] D27569: [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/queue_t_overload.cl:10
+  foo(0, src2);
+  foo(q, src3); // expected-error {{call to 'foo' is ambiguous}}
+}

could we also add something non-convertible for queue i.e.
  foo(1, src3);


https://reviews.llvm.org/D27569



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

[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[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! Small nitpicks below can be done before committing. Also it would be nice 
to double check the compile time is still fine after the last rebase.

Thanks!




Comment at: include/clang/Basic/OpenCLOptions.h:28
+unsigned Avail; // Option starts to be available in this OpenCL version
+unsigned Core;  // Options becomes (optional) core feature in this OpenCL
+// version

Options -> Option



Comment at: include/clang/Basic/OpenCLOptions.h:43
 
-  // Enable or disable all options.
-  void setAll(bool Enable = true) {
-#define OPENCLEXT(nm)   nm = Enable;
-#include "clang/Basic/OpenCLExtensions.def"
+  // Is supported OpenCL extension or (optional) core feature for OpenCL 
version
+  // \p CLVer.

Did you mean "and (optional) core feature?"



Comment at: test/SemaOpenCL/extension-begin.cl:5
+// Test with pch.
+// RUN: %clang_cc1 %s -DHEADER -triple spir-unknown-unknown -emit-pch 
-DHEADER_ONLY -o %t -verify -pedantic
+// RUN: %clang_cc1 %s -DHEADER_USER -triple spir-unknown-unknown -include-pch 
%t -fsyntax-only -verify -pedantic

Do we need -DHEADER_ONLY here?


https://reviews.llvm.org/D21698



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

[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Sema/Sema.h:8081
+  /// \brief Set current OpenCL extensions for a type which can only be used
+  /// when these OpenCL extensions are enabled. If current OpenCL Extsion is
+  /// empty, do nothing.

Extsion -> extension



Comment at: include/clang/Sema/Sema.h:8120
 
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.

or declaration -> or a declaration



Comment at: include/clang/Sema/Sema.h:8121
+  /// Checks if a type or declaration is disabled due to the owning extension
+  /// is disabled, and emits diagnostic messages if it is disabled.
+  /// \param D type or declaration to be checked.

is disabled -> being disabled



Comment at: lib/Serialization/ASTReader.cpp:3167
 case OPENCL_EXTENSIONS:
-  // Later tables overwrite earlier ones.
-  OpenCLExtensions.swap(Record);

Btw, OpenCLTypeExtMap and OpenCLTypeDeclMap don't have to be serialized?



Comment at: test/Parser/opencl-atomics-cl20.cl:51
 // expected-error@-28 {{use of type 'atomic_double' (aka '_Atomic(double)') 
requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-27 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
-// expected-error-re@-28 {{use of type 'atomic_uintptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-29 {{use of type 'atomic_uintptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
-// expected-error-re@-29 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') 
requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-30 {{use of type 'atomic_size_t' (aka '_Atomic({{.+}})') 
requires cl_khr_int64_extended_atomics extension to be enabled}}
-// expected-error-re@-30 {{use of type 'atomic_ptrdiff_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}
-// expected-error-re@-31 {{use of type 'atomic_ptrdiff_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_extended_atomics extension to be 
enabled}}
+#if __LP64__
+// expected-error-re@-28 {{use of type 'atomic_intptr_t' (aka 
'_Atomic({{.+}})') requires cl_khr_int64_base_atomics extension to be enabled}}

yaxunl wrote:
> Anastasia wrote:
> > Why this change?
> atomic_intptr_t etc. requires cl_khr_int64_extended_atomics only on 64 bit 
> platforms.
> 
> This is a bug which was fixed by this patch.
The spec says:
"If the device address space is 64-bits, the data types atomic_intptr_t, 
atomic_uintptr_t,
atomic_size_t and atomic_ptrdiff_t are supported if the 
cl_khr_int64_base_atomics and
cl_khr_int64_extended_atomics extensions are supported."

This seems to be the same as long and double?


https://reviews.llvm.org/D21698



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

[PATCH] D27453: [OpenCL] Enable unroll hint for OpenCL 1.x.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaStmtAttr.cpp:232
 
-  if (S.getLangOpts().OpenCLVersion < 200) {
-S.Diag(A.getLoc(), diag::err_attribute_requires_opencl_version)

The comment above refers to v2.0, could we update it reflecting the fact that 
we now allow the feature in all OpenCL versions.


https://reviews.llvm.org/D27453



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

[PATCH] D27569: [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9624
 
+  if (getLangOpts().OpenCL && getLangOpts().OpenCLVersion >= 200) {
+if (LHSIsNull && RHSType->isQueueT()) {

getLangOpts().OpenCL is redundant because getLangOpts().OpenCLVersion is only 
set for OpenCL.

I would like us to minimize number of this checks in the future.



Comment at: test/CodeGenOpenCL/null_queue.cl:7
+bool f() {
+  return CLK_NULL_QUEUE == get_default_queue() &&
+ get_default_queue() == CLK_NULL_QUEUE;

I think this doesn't handle initialization yet:
  queue_t q = 0;
which should also be possible! 



Comment at: test/CodeGenOpenCL/null_queue.cl:8
+  return CLK_NULL_QUEUE == get_default_queue() &&
+ get_default_queue() == CLK_NULL_QUEUE;
+  // CHECK: icmp eq %opencl.queue_t* null, %{{.*}}

could we just compare directly to 0 to make it simpler?



Comment at: test/CodeGenOpenCL/null_queue.cl:9
+ get_default_queue() == CLK_NULL_QUEUE;
+  // CHECK: icmp eq %opencl.queue_t* null, %{{.*}}
+}

Could we check for exactly two occurrences of icmp?


https://reviews.llvm.org/D27569



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

[PATCH] D21698: [OpenCL] Allow disabling types and declarations associated with extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8127
   "invalid prototype, variadic arguments are not allowed in OpenCL">;
+def err_requires_extension : Error<
+  "use of %select{declaration|type }0%1 requires %2 extension to be enabled">;

Could we rename err_requires_extension -> err_opencl_requires_extension



Comment at: lib/Parse/ParsePragma.cpp:461
+  };
+  typedef std::pair OpenCLExtData;
 }

yaxunl wrote:
> Anastasia wrote:
> > yaxunl wrote:
> > > Anastasia wrote:
> > > > Could we use PointerIntPair still but with 2 bits?
> > > Using PointerIntPair with 2 bits requires IdentifierInfo to be aligned at 
> > > 4 bytes, however this is not true. IdentifierInfo has no explicit 
> > > alignment specifier, therefore it can only hold 1 bit in PointerIntPair.
> > Based on its structure with having a pointer member I think it's reasonable 
> > to assume at least 4 bytes alignment... Although this doesn't seem to 
> > affect the performance anyways.
> You are right. The alignment of IdentifierInfo should be 8 on most systems.
> 
> However in IdentifierTable.h there is
> 
> 
> ```
> // Provide PointerLikeTypeTraits for IdentifierInfo pointers, which
> // are not guaranteed to be 8-byte aligned.
> template<>
> class PointerLikeTypeTraits {
> public:
>   static inline void *getAsVoidPointer(clang::IdentifierInfo* P) {
> return P;
>   }
>   static inline clang::IdentifierInfo *getFromVoidPointer(void *P) {
> return static_cast(P);
>   }
>   enum { NumLowBitsAvailable = 1 };
> };
> 
> ```
> Due to the above code there is only 1 bit available for when using 
> PointerIntPair with IdentifierInfo*. My guess is that alignment of 
> IdentifierInfo is not 8 on certain systems so some one intentionally defined 
> the above code to prevent using PointerIntPair with IdentifierInfo* for more 
> than 1 bit.
> 
Not too critical, but it's certainly not 8 byte aligned for 32 bit 
architectures. It should although be at least 4 bytes aligned in my opinion.

This code seems to be old, and the comment is not very precise to be honest. 



Comment at: lib/Sema/Sema.cpp:1614
+Decl = TagT->getDecl();
+  auto DL = OpenCLDeclExtMap.find(Decl);
+  if (DL != OpenCLDeclExtMap.end()) {

this bit (up to line 1623) seems to be repeated in multiple places... wondering 
if we could use a helper function instead perhaps with the templates?



Comment at: lib/Serialization/ASTReader.cpp:3170
+OpenCLExtensions.OptMap[Name] = {
+  static_cast(Record[I++]),
+  static_cast(Record[I++]),

Can you put a comment explaining which element each line corresponds to?


https://reviews.llvm.org/D21698



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

[PATCH] D27671: [OpenCL] Improve address space diagnostics.

[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! Thanks!


https://reviews.llvm.org/D27671



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D28080#644222, @yaxunl wrote:

> > @yaxunl Sam, I think it would be nice to describe your recent change for 
> > adding OpenCL extensions. Would you like to write up some text? Otherwise I 
> > can draft something and you can review. :)
>
> Anastasia, I am busy with some other work. If you can add it I will be happy 
> to review it. Thanks!


Sure. No worries!


https://reviews.llvm.org/D28080



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 83824.
Anastasia added a comment.

Updated with comments from Mats!


https://reviews.llvm.org/D28080

Files:
  docs/UsersManual.rst
  www/index.html

Index: www/index.html
===
--- www/index.html
+++ www/index.html
@@ -15,10 +15,10 @@
   clang: a C language family frontend for LLVM
   
   
-  The goal of the Clang project is to create a new C, C++, Objective C and
-  Objective C++ front-end for the http://www.llvm.org/;>LLVM
-  compiler.  You can get and build the source
-  today.
+  The goal of the Clang project is to create a new C based language
+  front-end: C, C++, Objective C/C++, OpenCL and others for the
+  http://www.llvm.org/;>LLVM compiler.  You can
+  get and build the source  today.
   
   
   Features and Goals
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -41,6 +41,7 @@
variants depending on base language.
 -  :ref:`C++ Language `
 -  :ref:`Objective C++ Language `
+-  :ref:`OpenCL Language `: v1.0, v1.1, v1.2, v2.0.
 
 In addition to these base languages and their dialects, Clang supports a
 broad variety of language extensions, which are documented in the
@@ -1973,6 +1974,348 @@
  is provided or target does not support TLS, code generation for threadprivate
  variables relies on OpenMP runtime library.
 
+.. _opencl:
+
+OpenCL Features
+===
+
+Clang can be used to compile OpenCL kernels for execution on a device
+(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMD or
+Nvidia targets) that can be uploaded to run directly on a device (e.g. using
+`clCreateProgramWithBinary
+`_) or
+into generic bitcode files loadable into other toolchains.
+
+Compiling to a binary using the default target from the installation can be done
+as follows:
+
+   .. code-block:: console
+
+ $ echo "kernel void k(){}" > test.cl
+ $ clang test.cl
+
+Compiling for a specific target can be done by specifying the triple corresponding
+to the target, for example:
+
+   .. code-block:: console
+
+ $ clang -target nvptx64-unknown-unknown test.cl
+ $ clang -target amdgcn-unknown-amdhsa test.cl
+
+Compiling to bitcode can be done as follows:
+
+   .. code-block:: console
+
+ $ clang test.cl -c -emit-llvm
+
+This will produce a generic test.bc file that can be used in vendor toolchains
+to perform machine code generation.
+
+Clang currently supports OpenCL standards up to v2.0.
+
+OpenCL Specific Options
+---
+
+Most of the OpenCL build options from `the specification v2.0 section 5.8.4
+`_ are available.
+
+Examples:
+
+   .. code-block:: console
+
+ $ clang -cl-std=CL2.0 -cl-single-precision-constant test.cl
+
+Some extra options are available to support special OpenCL features.
+
+.. option:: -finclude-default-header
+
+Loads standard includes during compilations. By default OpenCL headers are not
+loaded and therefore standard library includes are not available. To load them
+automatically a flag has been added to the frontend (see also OpenCL Header
+section):
+
+   .. code-block:: console
+
+ $ clang -Xclang -finclude-default-header test.cl
+
+Alternatively ``-include`` or ``-I`` followed by the path to the header location
+can be given manually.
+
+   .. code-block:: console
+
+ $ clang -I/lib/Headers/opencl-c.h test.cl
+
+In this case the kernel code should contain ``#include `` just as a
+regular C include.
+
+.. option:: -cl-ext
+
+Disables standard target extensions. All OpenCL targets provide a list
+of extensions that they support. Clang allows to amend this using the ``-cl-ext``
+flag with a comma-separated list of extensions prefixed with ``'+'`` or ``'-'``.
+The syntax: ``-cl-ext=<(['-'|'+'][,])+>``,  where extensions
+can be either one of `the OpenCL specification extensions
+`_
+or any known vendor extension. Alternatively, ``'all'`` can be used to enable or disable
+all known extensions.
+Example disabling double support for the 64-bit SPIR target:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple spir64-unknown-unknown -cl-ext=-cl_khr_fp64 test.cl
+
+Enabling all extensions except double support in R600 AMD GPU can be done using:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple r600-unknown-unknown -cl-ext=-all,+cl_khr_fp16 test.cl
+
+.. _opencl_fake_address_space_map:
+
+.. option:: -ffake-address-space-map
+
+Overrides the target address space map with a fake map.
+This allows adding address spaces for architectures that do not provide default
+memory segment support. Passing ``-ffake-address-space-map`` will add/override
+address spaces of the target compiled for with the following 

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 83962.
Anastasia added a comment.

Addressed comments from Alexey, Pekka, and Sam!


https://reviews.llvm.org/D28080

Files:
  docs/UsersManual.rst
  www/index.html

Index: www/index.html
===
--- www/index.html
+++ www/index.html
@@ -15,10 +15,10 @@
   clang: a C language family frontend for LLVM
   
   
-  The goal of the Clang project is to create a new C, C++, Objective C and
-  Objective C++ front-end for the http://www.llvm.org/;>LLVM
-  compiler.  You can get and build the source
-  today.
+  The goal of the Clang project is to create a new C based language
+  front-end: C, C++, Objective C/C++, OpenCL C and others for the
+  http://www.llvm.org/;>LLVM compiler.  You can
+  get and build the source  today.
   
   
   Features and Goals
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -41,6 +41,7 @@
variants depending on base language.
 -  :ref:`C++ Language `
 -  :ref:`Objective C++ Language `
+-  :ref:`OpenCL C Language `: v1.0, v1.1, v1.2, v2.0.
 
 In addition to these base languages and their dialects, Clang supports a
 broad variety of language extensions, which are documented in the
@@ -1973,6 +1974,361 @@
  is provided or target does not support TLS, code generation for threadprivate
  variables relies on OpenMP runtime library.
 
+.. _opencl:
+
+OpenCL Features
+===
+
+Clang can be used to compile OpenCL kernels for execution on a device
+(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMD or
+Nvidia targets) that can be uploaded to run directly on a device (e.g. using
+`clCreateProgramWithBinary
+`_) or
+into generic bitcode files loadable into other toolchains.
+
+Compiling to a binary using the default target from the installation can be done
+as follows:
+
+   .. code-block:: console
+
+ $ echo "kernel void k(){}" > test.cl
+ $ clang test.cl
+
+Compiling for a specific target can be done by specifying the triple corresponding
+to the target, for example:
+
+   .. code-block:: console
+
+ $ clang -target nvptx64-unknown-unknown test.cl
+ $ clang -target amdgcn-amd-amdhsa-opencl test.cl
+
+Compiling to bitcode can be done as follows:
+
+   .. code-block:: console
+
+ $ clang -c -emit-llvm test.cl
+
+This will produce a generic test.bc file that can be used in vendor toolchains
+to perform machine code generation.
+
+Clang currently supports OpenCL C language standards up to v2.0.
+
+OpenCL Specific Options
+---
+
+Most of the OpenCL build options from `the specification v2.0 section 5.8.4
+`_ are available.
+
+Examples:
+
+   .. code-block:: console
+
+ $ clang -cl-std=CL2.0 -cl-single-precision-constant test.cl
+
+Some extra options are available to support special OpenCL features.
+
+.. option:: -finclude-default-header
+
+Loads standard includes during compilations. By default OpenCL headers are not
+loaded and therefore standard library includes are not available. To load them
+automatically a flag has been added to the frontend (see also OpenCL Header
+section):
+
+   .. code-block:: console
+
+ $ clang -Xclang -finclude-default-header test.cl
+
+Alternatively ``-include`` or ``-I`` followed by the path to the header location
+can be given manually.
+
+   .. code-block:: console
+
+ $ clang -I/lib/Headers/opencl-c.h test.cl
+
+In this case the kernel code should contain ``#include `` just as a
+regular C include.
+
+.. option:: -cl-ext
+
+Disables support of OpenCL extensions. All OpenCL targets provide a list
+of extensions that they support. Clang allows to amend this using the ``-cl-ext``
+flag with a comma-separated list of extensions prefixed with ``'+'`` or ``'-'``.
+The syntax: ``-cl-ext=<(['-'|'+'][,])+>``,  where extensions
+can be either one of `the OpenCL specification extensions
+`_
+or any known vendor extension. Alternatively, ``'all'`` can be used to enable
+or disable all known extensions.
+Example disabling double support for the 64-bit SPIR target:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple spir64-unknown-unknown -cl-ext=-cl_khr_fp64 test.cl
+
+Enabling all extensions except double support in R600 AMD GPU can be done using:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple r600-unknown-unknown -cl-ext=-all,+cl_khr_fp16 test.cl
+
+.. _opencl_fake_address_space_map:
+
+.. option:: -ffake-address-space-map
+
+Overrides the target address space map with a fake map.
+This allows adding explicit address space IDs to the bitcode for non-segmented
+memory architectures that don't have separate IDs for each of the OpenCL
+logical address spaces by 

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 16 inline comments as done.
Anastasia added inline comments.



Comment at: docs/UsersManual.rst:2013
+to perform machine code generation.
+
+Clang currently supports OpenCL standards up to v2.0.

pekka.jaaskelainen wrote:
> I'd like to add a sentence or two for instructing how to use it for non-SPMD 
> targets and of course advertise pocl here too ;) 
> 
> Something along the lines of:
> 
> "For "non-SPMD" targets which cannot spawn multiple work-items on the fly 
> using hardware, which covers practically all non-GPU devices such as CPUs and 
> DSPs, additional processing is needed for the kernels to support multiple WI 
> execution. For this, a 3rd party project such as `pocl 
> `_ can be used."
Cool! I have added this later in the description of x86 target use for OpenCL. 
:)



Comment at: docs/UsersManual.rst:2053
+
+Disables standard target extensions. All OpenCL targets provide a list
+of extensions that they support. Clang allows to amend this using the 
``-cl-ext``

yaxunl wrote:
> Maybe rephrase as "Disables support of OpenCL extensions" instead of "Disable 
> standard target extensions" since "standard target extension" is confusing 
> and also to avoid confusion with the effect of disabling OpenCL extension as 
> in '#pragam OPENCL EXTENSION X: disable'. Disabling support of an extension 
> means that they cannot be enabled by pragmas in the OpenCL kernels since they 
> are deemed as non-supported.
Good point!



Comment at: docs/UsersManual.rst:2114
+ $ clang -target nvptx64-unknown-unknown test.cl
+
+Generic Targets

pekka.jaaskelainen wrote:
> This is an alternative location for my pocl advertisement above with a 
> subtitle:
> 
> - For CPU/DSP architectures such as x86, ARM or TCE:
> ...
> 
Added later in the x86 description. 



Comment at: docs/UsersManual.rst:2120
+  that can be used across GPU toolchains. The implementation follows `the SPIR
+  specification `_. There are two flavors 
available
+  for 32 and 64 bits.

bader wrote:
> pekka.jaaskelainen wrote:
> > Which version of SPIR is generated?
> For -cl-std=CL1.x (where x is 0, 1 or 2), SPIR version is 1.2.
> For -cl-std=CL2.0, SPIR version is 2.0.
Is it worth mentioning this in this doc?



Comment at: docs/UsersManual.rst:2130
+
+- x86 is used by some implementations that are x86 compatible
+  (e.g. `POCL `_) and currently remains for backwards

pekka.jaaskelainen wrote:
> This is a bit confusing paragraph, probably due to my confusing explanations 
> of the problems with pocl. Currently pocl tries not to use FASM for 
> preserving logical AS IDs to LLVM IR due to the bunch of problems it 
> constantly produces with seemingly little benefits for common CPUs. My patch 
> related to this considered only the argument info switch. Now pocl only 
> derives the logical iDS from kernel arguments (and all other IDs within the 
> body of the IR function are lost for flat machines).  In my patch, the 
> argument info's address space IDs were made constant and identical to SPIR's 
> as previously they were the target's (which meant losing the AS IDs 
> altogether for flat AS machines).
> 
> You seem to document the arg-info md switch later, so this paragraph might be 
> removed or converted to my pocl blurb which mentions the need for further 
> processing for CPUs.
Yes, it's perhaps a bit confusing indeed. I find the usage of the x86 backend 
for OpenCL is generally a bit confusing. Also there are a lot of program paths 
even in Clang that don't play well for OpenCL purposes or are not handled 
properly and are therefore a source of bugs.

I was just wondering whether it would be possible to switch to using SPIR as a 
generic target at some point in the future and "deprecate" the x86 target for 
OpenCL . Do you think this might work for POCL? At the same time we have 
upcoming SPIR-V support as a new feature that might reshape things.

Regarding the address space I just know that it's common to use fake address 
space map with the x86 backend for some out of tree implementations to add 
missing memory segments to this target. That's why I added this text here.





Comment at: docs/UsersManual.rst:2142-2143
+
+By default Clang will not include standard headers and therefore OpenCL builtin
+functions are unknown. The default CL header is, however, provided in the Clang
+installation and can be enabled by passing the ``-finclude-default-header`` 
flag

bader wrote:
> Not sure it's worth to mention, but even built-in vector types are defined in 
> the opencl-c.h.
Good point!



Comment at: www/index.html:19
+  The goal of the Clang project is to create a new C based language
+  front-end: C, C++, Objective C/C++, OpenCL and others for the
+ 

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 4 inline comments as done.
Anastasia added a comment.

Ping! Any more comments here? It would be nice to commit this before the 4.0 
rel branch is taken (on Jan 12).


https://reviews.llvm.org/D28080



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 84136.
Anastasia added a comment.

Update based on comments from Pekka.


https://reviews.llvm.org/D28080

Files:
  docs/UsersManual.rst
  www/index.html

Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -41,6 +41,7 @@
variants depending on base language.
 -  :ref:`C++ Language `
 -  :ref:`Objective C++ Language `
+-  :ref:`OpenCL C Language `: v1.0, v1.1, v1.2, v2.0.
 
 In addition to these base languages and their dialects, Clang supports a
 broad variety of language extensions, which are documented in the
@@ -1973,6 +1974,365 @@
  is provided or target does not support TLS, code generation for threadprivate
  variables relies on OpenMP runtime library.
 
+.. _opencl:
+
+OpenCL Features
+===
+
+Clang can be used to compile OpenCL kernels for execution on a device
+(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMD or
+Nvidia targets) that can be uploaded to run directly on a device (e.g. using
+`clCreateProgramWithBinary
+`_) or
+into generic bitcode files loadable into other toolchains.
+
+Compiling to a binary using the default target from the installation can be done
+as follows:
+
+   .. code-block:: console
+
+ $ echo "kernel void k(){}" > test.cl
+ $ clang test.cl
+
+Compiling for a specific target can be done by specifying the triple corresponding
+to the target, for example:
+
+   .. code-block:: console
+
+ $ clang -target nvptx64-unknown-unknown test.cl
+ $ clang -target amdgcn-amd-amdhsa-opencl test.cl
+
+Compiling to bitcode can be done as follows:
+
+   .. code-block:: console
+
+ $ clang -c -emit-llvm test.cl
+
+This will produce a generic test.bc file that can be used in vendor toolchains
+to perform machine code generation.
+
+Clang currently supports OpenCL C language standards up to v2.0.
+
+OpenCL Specific Options
+---
+
+Most of the OpenCL build options from `the specification v2.0 section 5.8.4
+`_ are available.
+
+Examples:
+
+   .. code-block:: console
+
+ $ clang -cl-std=CL2.0 -cl-single-precision-constant test.cl
+
+Some extra options are available to support special OpenCL features.
+
+.. option:: -finclude-default-header
+
+Loads standard includes during compilations. By default OpenCL headers are not
+loaded and therefore standard library includes are not available. To load them
+automatically a flag has been added to the frontend (see also :ref:`the section
+on the OpenCL Header `):
+
+   .. code-block:: console
+
+ $ clang -Xclang -finclude-default-header test.cl
+
+Alternatively ``-include`` or ``-I`` followed by the path to the header location
+can be given manually.
+
+   .. code-block:: console
+
+ $ clang -I/lib/Headers/opencl-c.h test.cl
+
+In this case the kernel code should contain ``#include `` just as a
+regular C include.
+
+.. option:: -cl-ext
+
+Disables support of OpenCL extensions. All OpenCL targets provide a list
+of extensions that they support. Clang allows to amend this using the ``-cl-ext``
+flag with a comma-separated list of extensions prefixed with ``'+'`` or ``'-'``.
+The syntax: ``-cl-ext=<(['-'|'+'][,])+>``,  where extensions
+can be either one of `the OpenCL specification extensions
+`_
+or any known vendor extension. Alternatively, ``'all'`` can be used to enable
+or disable all known extensions.
+Example disabling double support for the 64-bit SPIR target:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple spir64-unknown-unknown -cl-ext=-cl_khr_fp64 test.cl
+
+Enabling all extensions except double support in R600 AMD GPU can be done using:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple r600-unknown-unknown -cl-ext=-all,+cl_khr_fp16 test.cl
+
+.. _opencl_fake_address_space_map:
+
+.. option:: -ffake-address-space-map
+
+Overrides the target address space map with a fake map.
+This allows adding explicit address space IDs to the bitcode for non-segmented
+memory architectures that don't have separate IDs for each of the OpenCL
+logical address spaces by default. Passing ``-ffake-address-space-map`` will
+add/override address spaces of the target compiled for with the following values:
+``1-global``, ``2-constant``, ``3-local``, ``4-generic``. The private address
+space is represented by the absence of an address space attribute in the IR (see
+also :ref:`the section on the address space attribute `).
+
+   .. code-block:: console
+
+ $ clang -ffake-address-space-map test.cl
+
+Some other flags used for the compilation for C can also be passed while
+compiling for OpenCL, examples: ``-c``, ``-O<1-4|s>``, ``-o``, ``-emit-llvm``, etc.
+
+OpenCL Targets
+--
+
+OpenCL targets are 

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: docs/UsersManual.rst:2130
+
+- x86 is used by some implementations that are x86 compatible
+  (e.g. `POCL `_) and currently remains for backwards

pekka.jaaskelainen wrote:
> Anastasia wrote:
> > pekka.jaaskelainen wrote:
> > > This is a bit confusing paragraph, probably due to my confusing 
> > > explanations of the problems with pocl. Currently pocl tries not to use 
> > > FASM for preserving logical AS IDs to LLVM IR due to the bunch of 
> > > problems it constantly produces with seemingly little benefits for common 
> > > CPUs. My patch related to this considered only the argument info switch. 
> > > Now pocl only derives the logical iDS from kernel arguments (and all 
> > > other IDs within the body of the IR function are lost for flat machines). 
> > >  In my patch, the argument info's address space IDs were made constant 
> > > and identical to SPIR's as previously they were the target's (which meant 
> > > losing the AS IDs altogether for flat AS machines).
> > > 
> > > You seem to document the arg-info md switch later, so this paragraph 
> > > might be removed or converted to my pocl blurb which mentions the need 
> > > for further processing for CPUs.
> > Yes, it's perhaps a bit confusing indeed. I find the usage of the x86 
> > backend for OpenCL is generally a bit confusing. Also there are a lot of 
> > program paths even in Clang that don't play well for OpenCL purposes or are 
> > not handled properly and are therefore a source of bugs.
> > 
> > I was just wondering whether it would be possible to switch to using SPIR 
> > as a generic target at some point in the future and "deprecate" the x86 
> > target for OpenCL . Do you think this might work for POCL? At the same time 
> > we have upcoming SPIR-V support as a new feature that might reshape things.
> > 
> > Regarding the address space I just know that it's common to use fake 
> > address space map with the x86 backend for some out of tree implementations 
> > to add missing memory segments to this target. That's why I added this text 
> > here.
> > 
> > 
> We have considered using SPIR internally, but it is likely to bring the same 
> problems as with the FASM as we need to convert to the target bitcode to 
> ensure all target specific IR level optimizations (especially vectorization) 
> is applied properly. Why do you think using the real target when generating 
> from OpenCL C kernels is problematic? 
Perhaps, it would be nice to understand better how you use x86 backend in your 
toolchain. What I have seen in some projects that some invalid IR was produced 
or Clang hit an asset in the place which has to be done differently in some 
CodeGen paths just for OpenCL. So the typical fix for this to specialize on the 
OpenCL is not that easy to get into upstream because the target hasn't been 
created for what it's being used for and people like to see more modular code 
in general anyways.

The target is ideally not just a processor architecture but a combination of 
hardware and runtime software layers (i.e. OpenCL runtime in our case). Btw 
what triple do you use exactly while compiling OpenCL for x86?



Comment at: docs/UsersManual.rst:2133
+
+- x86 is used by some implementations that are x86 compatible and currently
+  remains for backwards compatibility (with older implementations prior to

pekka.jaaskelainen wrote:
> ARM (and others) are treated similarly in pocl: OpenCL C is just yet another 
> frontend language in this picture, like C/C++. I would not "deprecate" that 
> path as of yet as forcing to another intermediate representation (which SPIR 
> basically is, albeit derived from LLVM IR) always loses some information on 
> the way.
I have removed the deprecation bit from the documentation now. Do you want me 
to change anything else here?

I think the idea of SPIR target was not to force the developers to yet another 
IR but rather to create a common generic target that is specifically tailored 
to OpenCL needs (not just as a language but RT as well). Also the intention of 
SPIR was to add missing semantics of OpenCL to LLVM IR, so it's supposed not to 
lose the information ideally even though I am not completely sure the goal was 
achieved. But since SPIR-V is gaining more popularity I am not sure how much 
SPIR will continue to be relevant in the community.

One more problem with x86 target is that there are currently not many 
contributors in the community that are willing to support it for OpenCL. That's 
why I am a bit reluctant to put it in the document as fully supported target. 
Do you have any relevant fixes in your internal branches btw? 


https://reviews.llvm.org/D28080



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Since everyone agreed on the core part of this documentation I have committed 
the current revision in 291780 to make it easier for the release.

Feel free  to give me more feedback (if any) as small changes can still be 
merged in.

@yaxunl Sam, I think it would be nice to describe your recent change for adding 
OpenCL extensions. Would you like to write up some text? Otherwise I can draft 
something and you can review. :)


https://reviews.llvm.org/D28080



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

[PATCH] D27109: [OpenCL] Prevent generation of globals in non-constant address space for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r288163.


https://reviews.llvm.org/D27109



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

[PATCH] D27334: [OpenCL] Ambiguous function call.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D27334#612858, @bader wrote:

> In https://reviews.llvm.org/D27334#611703, @Anastasia wrote:
>
> > This change seems to modify normal C behavior again. Is there any strong 
> > motivation for doing this and if yes could it be done generically with C?
>
>
> Motivation:
>
>   // Non-portable OpenCL 1.2 code 
>   __kernel void foo(global float* out) {
> out[get_global_id(0)] = sin(get_global_id(0));
>   }
>
>
> This program compiles fine on OpenCL platform w/o doubles support and fails 
> otherwise.
>  If OpenCL driver supports doubles it provides at least two versions of 'sin' 
> built-in math function and compiler will not be able to choose the right one 
> for 'size_t' argument.
>  The goal of this warning is to let OpenCL developer know about potential 
> issues with code portability.


I would argue this improves the portability much as it can also be misleading 
in some situations (because it refers to a potentially hypothetical problem). 
For example there can be builtin functions that only have a float parameter 
(without a double version of it). This is for example the case with read_image 
functions that take a float coordinate value between 0 and 1. Unfortunately 
this warning won't be triggered on read_image functions because there is an 
overload candidate with an int type of the same parameter too. But we can't 
exclude this situations to appear in the future or from some vendor extensions 
or even custom OpenCL code.


https://reviews.llvm.org/D27334



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

[PATCH] D27300: [OpenCL] Fix SPIR version generation.

[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! Thanks!


https://reviews.llvm.org/D27300



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

[PATCH] D27403: [OpenCL] Added a LIT test for ensuring address space mangling is done the same both in OpenCL1.2 and OpenCL2.0.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/CodeGenOpenCL/address-spaces-mangling.cl:36
+
+__attribute__((overloadable)) void foo(private char *);
+__attribute__((overloadable)) void foo(global char *);

Just trying to understand what we are missing in the current testing. Does the 
additional code test something different from the previous one? Could we reuse 
the old test but just run it with the other OpenCL versions?

Also do we have any different behavior in the mangler depending on OpenCL 
versions? 


https://reviews.llvm.org/D27403



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

[PATCH] D27334: [OpenCL] Ambiguous function call.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

This change seems to modify normal C behavior again. Is there any strong 
motivation for doing this and if yes could it be done generically with C?




Comment at: lib/Sema/SemaChecking.cpp:2479
+// integer values.
+if (FDecl->hasAttr()) {
+  for (const auto* Arg : Args) {

How does it check that this is a built-in function?


https://reviews.llvm.org/D27334



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

[PATCH] D27300: [OpenCL] Fix SPIR version generation.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/CodeGenOpenCL/spir_version.cl:22
+
 // CHECK-SPIR-CL20: !opencl.spir.version = !{[[SPIR:![0-9]+]]}
 // CHECK-SPIR-CL20: !opencl.ocl.version = !{[[SPIR:![0-9]+]]}

Could we then use VER here too?



Comment at: test/CodeGenOpenCL/spir_version.cl:26
 
 // CHECK-AMDGCN-CL10: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
 // CHECK-AMDGCN-CL10: [[OCL]] = !{i32 1, i32 0}

Btw, would it make sense to add for AMD targets?
  CHECK-NOT: !opencl.spir.version




https://reviews.llvm.org/D27300



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

[PATCH] D28257: [OpenCL] Re-enable supported core extensions based on opencl version when disabling all extensions using pragma

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.

Ok, LGTM! Thanks!


https://reviews.llvm.org/D28257



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

[PATCH] D28257: [OpenCL] Re-enable supported core extensions based on opencl version when disabling all extensions using pragma

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/extensions.cl:47
 
+#ifndef _OPENCL_H_
 int isnan(float x) {

Why do you need this?


https://reviews.llvm.org/D28257



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

[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

This has been discussed during the initial review for the header:
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160425/157187.html

The main issue is after preprocessing the header the original function name is 
no longer available in diagnostics reported. The spec defines as_type as a 
builtin function and not a macro. Additionally your patch would allow as_type 
to be used with extra type (not only those defined in spec). Also I don't see 
the problem to implement as_type with just simply calling a builtin. It should 
be inlined later anyways.


https://reviews.llvm.org/D28136



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

[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

Excellent! Thanks!


https://reviews.llvm.org/D28048



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

[PATCH] D28080: [Docs][OpenCL] Added OpenCL feature description to user manual.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: yaxunl, pekka.jaaskelainen, bader.
Anastasia added subscribers: cfe-commits, rsmith.

Adding information on OpenCL to Clang documentation. This currently contain the 
main features only. I am hoping we can improve it with time documenting more 
features.

Can be build by:

  make  -DLLVM_ENABLE_SPHINX=ON -DLLVM_BUILD_DOCS=ON ...
  make sphinx


https://reviews.llvm.org/D28080

Files:
  docs/UsersManual.rst
  www/index.html

Index: www/index.html
===
--- www/index.html
+++ www/index.html
@@ -15,10 +15,10 @@
   clang: a C language family frontend for LLVM
   
   
-  The goal of the Clang project is to create a new C, C++, Objective C and
-  Objective C++ front-end for the http://www.llvm.org/;>LLVM
-  compiler.  You can get and build the source
-  today.
+  The goal of the Clang project is to create a new C based language
+  front-end: C, C++, Objective C/C++, OpenCL and others for the
+  http://www.llvm.org/;>LLVM compiler.  You can
+  get and build the source  today.
   
   
   Features and Goals
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -41,6 +41,7 @@
variants depending on base language.
 -  :ref:`C++ Language `
 -  :ref:`Objective C++ Language `
+-  :ref:`OpenCL Language `: v1.0, v1.1, v1.2, v2.0.
 
 In addition to these base languages and their dialects, Clang supports a
 broad variety of language extensions, which are documented in the
@@ -1973,6 +1974,348 @@
  is provided or target does not support TLS, code generation for threadprivate
  variables relies on OpenMP runtime library.
 
+.. _opencl:
+
+OpenCL Features
+===
+
+Clang can be used to compile OpenCL kernels for execution on a device
+(e.g. GPU). It is possible to compile the kernel into a binary (e.g. for AMD or
+Nvidia targets) that can be uploaded to run directly on a device (e.g. using
+`clCreateProgramWithBinary
+`_) or
+into generic bitcode files loadable into other toolchains.
+
+Compiling to a binary using the default target from the installation can be done
+as follows:
+
+   .. code-block:: console
+
+ $ echo "kernel void k(){}" > test.cl
+ $ clang test.cl
+
+Compiling for a specific target can be done by specifying the triple corresponding
+to the target, for example:
+
+   .. code-block:: console
+
+ $ clang -target nvptx64-unknown-unknown test.cl
+ $ clang -target amdgcn-unknown-amdhsa test.cl
+
+Compiling to bitcode can be done as follows:
+
+   .. code-block:: console
+
+ $ clang test.cl -c -emit-llvm
+
+This will produce a generic test.bc file that can be used in vendor toolchains
+to perform machine code generation.
+
+Clang currently supports OpenCL standards up to v2.0.
+
+OpenCL Specific Options
+---
+
+Most of the OpenCL build options from `the specification v2.0 section 5.8.4
+`_ are available.
+
+Examples:
+
+   .. code-block:: console
+
+ $ clang -cl-std=CL2.0 -cl-single-precision-constant test.cl
+
+Some extra options are available to support special OpenCL features.
+
+.. option:: -finclude-default-header
+
+Loads standard includes during compilations. By default OpenCL headers are not
+loaded and therefore standard library includes are not available. To load them
+automatically a flag has been added to the frontend (see also OpenCL Header
+section):
+
+   .. code-block:: console
+
+ $ clang -Xclang -finclude-default-header test.cl
+
+Alternatively ``-include`` or ``-I`` followed by the path to the header location
+can be given manually.
+
+   .. code-block:: console
+
+ $ clang -I/lib/Headers/opencl-c.h test.cl
+
+In this case the kernel code should contain ``#include `` just as a
+regular C include.
+
+.. option:: -cl-ext
+
+Disables standard target extensions. All OpenCL targets provide a list
+of extensions that they support. Clang allows to amend this using the ``-cl-ext``
+flag with a comma-separated list of extensions prefixed with ``'+'`` or ``'-'``.
+The syntax: ``-cl-ext=<(['-'|'+'][,])+>``,  where extensions
+can be either one of `the OpenCL specification extensions
+`_
+or any known vendor extension. Alternatively, ``'all'`` can be used to enable or disable
+all known extensions.
+Example disabling double support for the 64-bit SPIR target:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple spir64-unknown-unknown -cl-ext=-cl_khr_fp64 test.cl
+
+Enabling all extensions except double support in R600 AMD GPU can be done using:
+
+   .. code-block:: console
+
+ $ clang -cc1 -triple r600-unknown-unknown -cl-ext=-all,+cl_khr_fp16 test.cl
+
+.. _opencl_fake_address_space_map:
+
+.. option:: 

[PATCH] D28058: [OpenCL] Correct ndrange_t implementation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:2491
+llvm::Value *Range = NDRangeL.getAddress().getPointer();
+llvm::Type *RangeTy = NDRangeL.getAddress().getType();
 

I think following standard Clang CodeGen convention we should add byval 
attribute for structs. 

However, I am not sure if the best approach would be to use classifyType(...) 
of ABIInfo and then check what type of argument passing ABIArgInfo returns 
(direct/indirect/byval). Based on this we could add an attr 
llvm::Attribute::ByVal to the argument. Perhaps, this should be done with all 
arguments then. Or I am wondering if better approach would be to just use 
GetOrCreateLLVMFunction() helper from CodeGen!

Does anyone have any suggestion here?



https://reviews.llvm.org/D28058



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

[PATCH] D27981: Fix problems in "[OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand."

[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! If you checked there are no warnings any more, it seems fine to commit! :)


https://reviews.llvm.org/D27981



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

[PATCH] D28048: [OpenCL] Align fake address space map with the SPIR target maps.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: pekka.jaaskelainen.
Anastasia added a comment.

I believe it's not that important what those numbers actually are, but having 
the consistency with SPIR might be better indeed at least even for 
documentation purposes!

@pekka.jaaskelainen, I think you are using x86 target in the POCL toolchain, do 
you envision any issues with this change?


https://reviews.llvm.org/D28048



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

[PATCH] D27569: [OpenCL] Enabling the usage of CLK_NULL_QUEUE as compare operand.

[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! Thanks!


https://reviews.llvm.org/D27569



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

[PATCH] D27334: [OpenCL] Ambiguous function call.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D27334#614389, @bader wrote:

> In https://reviews.llvm.org/D27334#613504, @Anastasia wrote:
>
> > In https://reviews.llvm.org/D27334#612858, @bader wrote:
> >
> > > In https://reviews.llvm.org/D27334#611703, @Anastasia wrote:
> > >
> > > > This change seems to modify normal C behavior again. Is there any 
> > > > strong motivation for doing this and if yes could it be done 
> > > > generically with C?
> > >
> > >
> > > Motivation:
> > >
> > >   // Non-portable OpenCL 1.2 code 
> > >   __kernel void foo(global float* out) {
> > > out[get_global_id(0)] = sin(get_global_id(0));
> > >   }
> > >
> > >
> > > This program compiles fine on OpenCL platform w/o doubles support and 
> > > fails otherwise.
> > >  If OpenCL driver supports doubles it provides at least two versions of 
> > > 'sin' built-in math function and compiler will not be able to choose the 
> > > right one for 'size_t' argument.
> > >  The goal of this warning is to let OpenCL developer know about potential 
> > > issues with code portability.
> >
> >
> > I would argue this improves the portability much as it can also be 
> > misleading in some situations (because it refers to a potentially 
> > hypothetical problem). For example there can be builtin functions that only 
> > have a float parameter (without a double version of it). This is for 
> > example the case with read_image functions that take a float coordinate 
> > value between 0 and 1. Unfortunately this warning won't be triggered on 
> > read_image functions because there is an overload candidate with an int 
> > type of the same parameter too. But we can't exclude this situations to 
> > appear in the future or from some vendor extensions or even custom OpenCL 
> > code.
>
>
> As much as any other warning it's not always means that there is an error in 
> the code. It just means that developer should inspect the construction 
> triggering a warning.
>  Passing integer value to a function with floating point parameters is not 
> always an error, but some times it might be so.
>  Do you suggest dropping the diagnostics at all or changing the diagnostics 
> message?


I agree warnings don't always signal a definite issue (even thought it's good 
to make them as precise as we can). We could try to reword the diagnostic 
message. However, the biggest issue I have here is that the message can be 
given in the situations that are unrelated to the problem (i.e. the overload 
candidates that don't have anything to do with the parameter being diagnosed or 
don't overload with the double precision). Therefore, it feels like the 
diagnostic can be confusing in some cases even though they are not very 
probable ones.


https://reviews.llvm.org/D27334



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

[PATCH] D27403: [OpenCL] Added a LIT test for ensuring address space mangling is done the same both in OpenCL1.2 and OpenCL2.0.

[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! Thanks!


https://reviews.llvm.org/D27403



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D30810#706676, @Anastasia wrote:

> In https://reviews.llvm.org/D30810#699428, @Anastasia wrote:
>
> > Would you be able to update ScalarExprEmitter::VisitAsTypeExpr 
> > implementation accordingly to make things consistent?
>
>
> Not sure it got lost somewhere... do you think you could address this too?


I guess due to 4 element alignment the generated casts as vec4 should be fine 
for vec3, but it is worth checking...


https://reviews.llvm.org/D30810



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D30810#699428, @Anastasia wrote:

> Would you be able to update ScalarExprEmitter::VisitAsTypeExpr implementation 
> accordingly to make things consistent?


Not sure it got lost somewhere... do you think you could address this too?


https://reviews.llvm.org/D30810



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.



> Hi Anastasia, Bruno,
> 
> Do you still have other opinion? or Can we go ahead and commit this patch?

I would like to see this patch committed. I see clear evidence of it improving 
existing GPU targets in the master repo as well as outside of the main tree 
implementations. Bruno, do you have any more concerns?


https://reviews.llvm.org/D30810



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

[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Although nothing wrong with the implementation apart from it complicates a bit 
the understanding of conventional C parse flow by adding extra corner case, I 
don't see anything in the spec that states explicitly how the vector components 
should be parsed. I guess with extra parenthesis the parsing problem can easily 
be avoided:

  ((int3)(32, 32, 32)).x 




Comment at: test/Parser/vector-cast-define.cl:11
+{
+int index = dgSize.x * dgSize.y;
+}

I would prefer to simply the test and avoid using macros... it will be easier 
for us to maintain it in the future.


https://reviews.llvm.org/D31183



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

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 93226.
Anastasia added a comment.

Removed check of implicit for new type.


https://reviews.llvm.org/D31397

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaOpenCL/types.cl


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:2157
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||

ahatanak wrote:
> Is it necessary to check whether New is implicit? I was just wondering when 
> or how an implicit definition would redefine a typedef.
I had a thought on it too, and I am not sure actually. This can happen if we 
implicitly define something from the standard headers. But I believe the Sema 
initialization should always happen before parsing the standard header or even 
loading them from the PCH. So I guess this shouldn't ever happen really? 
Perhaps, I should just remove this?


https://reviews.llvm.org/D31397



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

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 93225.
Anastasia added a comment.

Updated with the full diff.


https://reviews.llvm.org/D31397

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaOpenCL/types.cl


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:2157
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||

Anastasia wrote:
> ahatanak wrote:
> > Is it necessary to check whether New is implicit? I was just wondering when 
> > or how an implicit definition would redefine a typedef.
> I had a thought on it too, and I am not sure actually. This can happen if we 
> implicitly define something from the standard headers. But I believe the Sema 
> initialization should always happen before parsing the standard header or 
> even loading them from the PCH. So I guess this shouldn't ever happen really? 
> Perhaps, I should just remove this?
Actually in case of implicit typedefs we don't seem to follow this program path 
at all. So I am removing this.


https://reviews.llvm.org/D31397



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

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/atomic-init.cl:6
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant and to variables in global adress space}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can only be assigned 
to a compile time constant and to variables in global adress space}}

Btw, you could keep "initialized" here by using 'select' in the diagnostic 
message. 



Comment at: test/SemaOpenCL/atomic-init.cl:10
+  local atomic_int a4 = 0; // expected-error {{'__local' variable cannot have 
an initializer}}
+  global atomic_int a5 = 0; // expected-error {{function scope variable cannot 
be declared in global address space}}
+}

Could we also add a check for 'static global' variable?


https://reviews.llvm.org/D30643



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

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:2157
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||

bader wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > ahatanak wrote:
> > > > Is it necessary to check whether New is implicit? I was just wondering 
> > > > when or how an implicit definition would redefine a typedef.
> > > I had a thought on it too, and I am not sure actually. This can happen if 
> > > we implicitly define something from the standard headers. But I believe 
> > > the Sema initialization should always happen before parsing the standard 
> > > header or even loading them from the PCH. So I guess this shouldn't ever 
> > > happen really? Perhaps, I should just remove this?
> > Actually in case of implicit typedefs we don't seem to follow this program 
> > path at all. So I am removing this.
> So something like this will also work?
> ```
> typedef float float4 __attribute((ext_vector_type(4)));
> typedef float4 atomic_int;
> ```
> 
An error will be given for this because the definitions are conflicting 
(incompatible).



Comment at: test/SemaOpenCL/types.cl:6
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;

bader wrote:
> Can we check that -Wtypedef-redefinition will emit a warning for this 
> expression?
> This typedef seems to be unnecessary since clang implicitly defines 
> atomic_flag for OpenCL. User is not supposed to re-define it, so warning 
> would be helpful here.
A warning is given only with -Wsystem-headers in this case. I don't modify this 
change here. But I can add a test if necessary. Verify wouldn't work here 
though because a note about the old location of the typedef will be given 
without source information. But I can add a FileCheck test.


https://reviews.llvm.org/D31397



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

[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.

[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!


https://reviews.llvm.org/D31183



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

[PATCH] D31321: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/CodeGenOpenCL/kernel-arg-info.cl:66
 // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"}
 // ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}

Could we modify the test to check that const and volatile are added for the 
pointers though?


https://reviews.llvm.org/D31321



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

[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D31183#709566, @echuraev wrote:

> In https://reviews.llvm.org/D31183#708833, @yaxunl wrote:
>
> > I think this is a good feature for the convenience of user. I've seen usage 
> > like this.
>
>
> I agree. I don't see any reasons why this case doesn't have the right to 
> exist. I don't think that using extra parenthesis is a good solution for 
> solving this problem.


I am just saying that I don't see a big use case for this. I am guessing it can 
largely come from the macro expansions, but those are generally good style to 
parenthesize.


https://reviews.llvm.org/D31183



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

[PATCH] D31324: [OpenCL] Extended mapping of parcing CodeGen arguments

[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! Thanks!


https://reviews.llvm.org/D31324



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

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/atomic-init.cl:6
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant and to variables in global adress space}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can only be assigned 
to a compile time constant and to variables in global adress space}}

Anastasia wrote:
> Btw, you could keep "initialized" here by using 'select' in the diagnostic 
> message. 
Btw you still keep "assigned" in the error message. What I mean is that we 
could put "initialized" instead in this case.


https://reviews.llvm.org/D30643



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

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I can't see clearly why the alloca has to be extended to accommodate the 
address space too? Couldn't  the address space for alloca just be taken 
directly from the data layout?

In fact is seems from the LLVM review, an address space for alloca doesn't go 
into the bitcode.




Comment at: include/clang/Basic/AddressSpaces.h:28
 enum ID {
-  Offset = 0x7FFF00,
+  Default = 0,
 

Somehow I wish that opencl_private would be represented explicitly instead and 
then an absence of an address space attribute would signify the default one to 
be used. But since opencl_private has always been represented as an absence of 
an address space attribute not only in AST but in IR as well, I believe it 
might be a bigger change now. However, how does this default address space 
align with default AS we put during type parsing in processTypeAttrs 
(https://reviews.llvm.org/D13168). I think after this step we shouldn't need 
default AS explicitly any longer? 



Comment at: include/clang/Basic/AddressSpaces.h:41
+
+  target_first = Count
 };

I don't entirely understand the motivation for this. I think the idea of LangAS 
is to represent the source ASes while target ASes are reflected in the Map of 
Targets.cpp.



Comment at: lib/AST/ASTContext.cpp:9553
+  // alloca.
+  if (AS == LangAS::Default && LangOpts.OpenCL)
+return getTargetInfo().getDataLayout().getAllocaAddrSpace();

Here it seems that LangAS::Default is indeed opencl_private?



Comment at: test/Sema/invalid-assignment-constant-address-space.c:4
-#define OPENCL_CONSTANT 8388354
-int __attribute__((address_space(OPENCL_CONSTANT))) c[3] = {0};
 

Is this test even correct? I don't think we can assume that C address spaces 
inherit the same restrictions as OpenCL. Especially that the notion of 
private/local/constant/global is an OpenCL specific thing.

I feel like Clang doesn't behave correctly for C address spaces now.

As for OpenCL I don't see why would anyone use __attribute__((address_space())) 
for constant AS. Especially that it's not part of the spec.


https://reviews.llvm.org/D31404



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

[PATCH] D31321: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args

[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!




Comment at: test/CodeGenOpenCL/kernel-arg-info.cl:66
 // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
-// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"}
 // ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}

echuraev wrote:
> Anastasia wrote:
> > Could we modify the test to check that const and volatile are added for the 
> > pointers though?
> Added test case only for volatile pointer because const is checked in the Z 
> argument of foo function.
I think it would still be nice to add an argument with explicit const because 
the current test doesn't have it explicitly.


https://reviews.llvm.org/D31321



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D30810#707176, @jaykang10 wrote:

> In https://reviews.llvm.org/D30810#706677, @Anastasia wrote:
>
> > In https://reviews.llvm.org/D30810#706676, @Anastasia wrote:
> >
> > > In https://reviews.llvm.org/D30810#699428, @Anastasia wrote:
> > >
> > > > Would you be able to update ScalarExprEmitter::VisitAsTypeExpr 
> > > > implementation accordingly to make things consistent?
> > >
> > >
> > > Not sure it got lost somewhere... do you think you could address this too?
> >
> >
> > I guess due to 4 element alignment the generated casts as vec4 should be 
> > fine for vec3, but it is worth checking...
>
>
> Let's look at examples.
>
> Source code
>
>   void kernel float4_to_float3(global float3 *a, global float4 *b) {
> //float4 --> float3
> *a = __builtin_astype(*b, float3);
>   }
>  
>   void kernel float3_to_float4(global float3 *a, global float4 *b) {
> //float3 --> float4
> *b = __builtin_astype(*a, float4);
>   }
>
>
> LLVM IR
>
>   ; Function Attrs: norecurse nounwind
>   define void @float4_to_float3(<3 x float>* nocapture %a, <4 x float>* 
> nocapture readonly %b) {
>   entry:
> %0 = load <4 x float>, <4 x float>* %b, align 16, !tbaa !6
> %storetmp = bitcast <3 x float>* %a to <4 x float>*
> store <4 x float> %0, <4 x float>* %storetmp, align 16, !tbaa !6
> ret void
>   }
>  
>   ; Function Attrs: norecurse nounwind
>   define void @float3_to_float4(<3 x float>* nocapture readonly %a, <4 x 
> float>* nocapture %b) {
>   entry:
> %castToVec4 = bitcast <3 x float>* %a to <4 x float>*
> %loadVec4 = load <4 x float>, <4 x float>* %castToVec4, align 16
> store <4 x float> %loadVec4, <4 x float>* %b, align 16, !tbaa !6
> ret void
>   }
>
>
> We could change above IR with vec3 as following:
>
>   ; Function Attrs: norecurse nounwinddefine void @float4_to_float3(<3 x 
> float>* nocapture %a, <4 x float>* nocapture readonly %b) {
>   entry:
> %0 = load <4 x float>, <4 x float>* %b, align 16
> %1 = shufflevector <4 x float> %0, <4 x float> undef, <3 x i32> i32 1, i32 2>
> store <3 x float> %1, <3 x float>* %a, align 16
> ret void
>   }
>  
>   ; Function Attrs: norecurse nounwinddefine void @float3_to_float4(<3 x 
> float>* nocapture readonly %a, <4 x float>* nocapture %b){
>   entry:
> %0 = load <3 x float>, <3 x float>* %a, align 16  %1 = shufflevector <3 x 
> float> %0, <3 x float> undef, <4 x i32> 
> store <4 x float> %1, <4 x float>* %b, align 16
> ret void
>   }
>
>
> I guess it is what you want on LLVM IR. I have compiled above IRs with amdgcn 
> target.
>
> vec4 float4_to_float3 assembly code
>
>   float4_to_float3:   ; @float4_to_float3
>   ; BB#0: ; %entry
>   s_load_dword s2, s[0:1], 0x9 
>   s_load_dword s0, s[0:1], 0xa 
>   s_mov_b32 s4, SCRATCH_RSRC_DWORD0
>   s_mov_b32 s5, SCRATCH_RSRC_DWORD1
>   s_mov_b32 s6, -1
>   s_mov_b32 s8, s3
>   s_mov_b32 s7, 0xe8f000
>   s_waitcnt lgkmcnt(0)
>   v_mov_b32_e32 v0, s0
>   buffer_load_dword v2, v0, s[4:7], s8 offen
>   buffer_load_dword v3, v0, s[4:7], s8 offen offset:4
>   buffer_load_dword v4, v0, s[4:7], s8 offen offset:8
>   buffer_load_dword v0, v0, s[4:7], s8 offen offset:12
>   v_mov_b32_e32 v1, s2
>   s_waitcnt vmcnt(0)
>   buffer_store_dword v0, v1, s[4:7], s8 offen offset:12
>   buffer_store_dword v4, v1, s[4:7], s8 offen offset:8
>   buffer_store_dword v3, v1, s[4:7], s8 offen offset:4
>   buffer_store_dword v2, v1, s[4:7], s8 offen
>   s_endpgm
>  
>
>
> vec3 float4_to_float3 assembly code
>
>   float4_to_float3:   ; @float4_to_float3
>   ; BB#0: ; %entry
>   s_load_dword s2, s[0:1], 0x9 
>   s_load_dword s0, s[0:1], 0xa 
>   s_mov_b32 s4, SCRATCH_RSRC_DWORD0
>   s_mov_b32 s5, SCRATCH_RSRC_DWORD1
>   s_mov_b32 s6, -1
>   s_mov_b32 s8, s3
>   s_mov_b32 s7, 0xe8f000
>   s_waitcnt lgkmcnt(0)
>   v_mov_b32_e32 v0, s0
>   buffer_load_dword v2, v0, s[4:7], s8 offen
>   buffer_load_dword v3, v0, s[4:7], s8 offen offset:8
>   buffer_load_dword v0, v0, s[4:7], s8 offen offset:4
>   v_mov_b32_e32 v1, s2
>   s_waitcnt vmcnt(0)
>   buffer_store_dword v0, v1, s[4:7], s8 offen offset:4
>   buffer_store_dword v2, v1, s[4:7], s8 offen
>   buffer_store_dword v3, v1, s[4:7], s8 offen offset:8
>   s_endpgm
>
>
> I think it is what we expect because there are 3 load/store for vec instead 
> of 4 load/store.
>
> The float3_to_float4's output is different with float4_to_float3.
>
> vec4 float3_to_float4 assembly code
>
>   float3_to_float4:   ; @float3_to_float4
>   ; BB#0: ; %entry
> 

[PATCH] D31183: [OpenCL] Added parsing for OpenCL vector types.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I don't think that diagnostics can always be very clear. This is not the case 
neither for C  nor C++.

As I said I don't see any issue to continue with this patch. I would just like 
to see the test simplified a bit.


https://reviews.llvm.org/D31183



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

[PATCH] D31397: [Bug 25404] Fix crash on typedef in OpenCL 2.0

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Herald added a subscriber: yaxunl.

Fixing the assertion due to absence of source location for implicitly defined 
types (using addImplicitTypedef()). During Sema checks the source location is 
being expected and therefore an assertion is triggered.

The change is not specific to OpenCL. But it is particular common for OpenCL 
types to be declared implicitly in Clang to support the mode without standard 
header.


https://reviews.llvm.org/D31397

Files:
  lib/Sema/SemaDecl.cpp
  test/SemaOpenCL/types.cl


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 


Index: test/SemaOpenCL/types.cl
===
--- /dev/null
+++ test/SemaOpenCL/types.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only
+
+// expected-no-diagnostics
+
+// Check redefinition of standard types
+typedef atomic_int atomic_flag;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2154,7 +2154,9 @@
   // -Wtypedef-redefinition.  If either the original or the redefinition is
   // in a system header, don't emit this for compatibility with GCC.
   if (getDiagnostics().getSuppressSystemWarnings() &&
-  (Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+  // Some standard types are defined implicitly in Clang (e.g. OpenCL).
+  (Old->isImplicit() || New->isImplicit() ||
+   Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
Context.getSourceManager().isInSystemHeader(New->getLocation(
 return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D31404#714906, @yaxunl wrote:

> In https://reviews.llvm.org/D31404#714244, @Anastasia wrote:
>
> > I can't see clearly why the alloca has to be extended to accommodate the 
> > address space too? Couldn't  the address space for alloca just be taken 
> > directly from the data layout?
> >
> > In fact is seems from the LLVM review, an address space for alloca doesn't 
> > go into the bitcode.
>
>
> In the latest comments of the LLVM review, reviewers have agreed that address 
> space of alloca goes into the bitcode.
>
> I am not quite get your first question. Do you mean why the API of alloca has 
> to have an address space parameter? Or do you question the necessity to let 
> alloca returning a pointer pointing to non-zero address space?


I am asking mainly about the motivation of extending alloca instruction syntax 
with an extra address space. It seems to me that the address space for alloca 
could just be taken from the data layout without any modification to the 
instruction itself.




Comment at: include/clang/Basic/AddressSpaces.h:28
 enum ID {
-  Offset = 0x7FFF00,
+  Default = 0,
 

yaxunl wrote:
> Anastasia wrote:
> > Somehow I wish that opencl_private would be represented explicitly instead 
> > and then an absence of an address space attribute would signify the default 
> > one to be used. But since opencl_private has always been represented as an 
> > absence of an address space attribute not only in AST but in IR as well, I 
> > believe it might be a bigger change now. However, how does this default 
> > address space align with default AS we put during type parsing in 
> > processTypeAttrs (https://reviews.llvm.org/D13168). I think after this step 
> > we shouldn't need default AS explicitly any longer? 
> Currently in Clang having an address space qualifier value of 0 is equivalent 
> to having no address space qualifier. This is due to the internal 
> representation of address space qualifier as bits in a mask. Therefore 
> although there are separate API's for hasAddressSpace() and 
> getAddressSpace(), in many many places people just do not use 
> hasAddressSpace() and only use getAddressSpace(). In a way, this is 
> convenient, since it allows people to use just one unsigned to represent that 
> whether a type has an address space qualifier and the value of the qualifier 
> if it has one. That's why value 0 of address space qualifier is called 
> `Default`, since it indicates `no address space qualifier`, which is the 
> default situation. Here we give it the name `Default`, just to emphasise the 
> existing reality, that is, 0 is truely the default value of address space 
> qualifier. This also matches most languages' view of address space, that is, 
> if not explicitly specified, 0 is the default address space qualifier since 
> it means `no address space qualifier`.
> 
> For OpenCL 1.2, this matches perfectly to private address space, since if no 
> address space qualifier implies private address space. For OpenCL 2.0, things 
> become complicated. 'no address space qualifier' in the source code no longer 
> ends up with a fixed address space qualifier in AST. What address space 
> qualifier we get in AST depends on scope of the variable. To be consistent 
> with the AST of OpenCL 1.2, we continue to use 'no address space qualifier 
> (or 0 value address space qualifier)' in AST to represent private address 
> space in OpenCL source language. This is non-ideal but it works fine. 
> Therefore although it is not written, in fact opencl_private is 0.
> 
> Since address space 0 in AST always represents the private address space in 
> OpenCL and the default address space in other languages, it cannot be used 
> for other address spaces of OpenCL. Also, when mapped to target address 
> space, for OpenCL, address space 0 in AST should map to target private 
> address space or alloca address space; for other languages, address space 0 
> in AST should map to target generic address space. It would be clearer to 
> have an enum value for 0 instead of using 0 directly.
I see a little value adding //Default// explicitly here, also because default 
AS is an OpenCL specific thing in my opinion. In C objects are just allowed to 
have no AS and it's fine. In fact the only use of //Default// in your patch is 
synonym to //opencl_private//. Unless we find another use for this, I would 
prefer not to add code for potential future use cases because it adds confusion 
and can be easily misused or create complications for adding new features.

I would rather say that Clang is missing an explicit representation of 
//opencl_private// at the moment. Mainly because private was used as default AS 
before OpenCL 2.0. So it was just the same thing. In OpenCL2.0 we worked around 
the fact that private is not represented explicitly by adding 'missing' ASes in 
//processTypeAttrs//. But it created some issues (for example in 

[PATCH] D30643: [OpenCL] Extended diagnostics for atomic initialization

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/atomic-init.cl:6
+kernel void test_atomic_initialization() {
+  a1 = 1; // expected-error {{atomic variable can only be assigned to a 
compile time constant}}
+  atomic_int a2 = 0; // expected-error {{atomic variable can be initialized to 
a variable only in global address space}}

This diagnostic seems wrong! How is 1 not a compile time constant?

Didn't we agree to drop the constant bit from the error message and just keep 
reporting about the program (global visibility) scope? I think the restriction 
about the scope holds for both initialization and assignment and therefore I 
have asked to unify the diagnostic...


https://reviews.llvm.org/D30643



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

[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D28136#701508, @bader wrote:

> > From all the above arguments, I feel like the right approach would be to 
> > implement it as Clang builtin which closely matches the operator semantic 
> > in my opinion. We could of course reuse the implementation of  
> > __bultin_astype to avoid unnecessary extra work and code duplication.
> > 
> > Using the macro seems to me more like a workaround solution and overloaded 
> > functions don't seem to be entirely the right thing either.  What do you 
> > think about it?
>
> I don't think we need another Clang built-in. __builtin_astype was added 
> specifically for OpenCL needs (see rev. 132612).
>  Do you know better way to map astype operators (there are a lot of them 
> as_#) to single Clang built-in?


Unfortunately,  __builtin_astype doesn't match the astype syntax from the spec 
exactly, which I wish it would. I don't feel strongly neither with macro (which 
messes up proper error reporting) nor with function declaration (for which 
implicit conversion would apply just like to other C functions). Having Clang 
builtin would feel the most natural way to me but I agree the way it is defined 
with the multiple distinct names for each type I am not going to advocate for 
it strongly.




Comment at: lib/Headers/opencl-c.h:6588
-char __ovld __cnfn as_char(char);
-char __ovld __cnfn as_char(uchar);
-

Why do we have some of the overloads omitted? Would this cause any extra 
conversions? uchar -> char in this case?


https://reviews.llvm.org/D28136



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

[PATCH] D30830: [OpenCL] Fix extension guards for atomic functions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Sorry, I overlooked this indeed. Committed in r298256!


https://reviews.llvm.org/D30830



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

[PATCH] D30937: [OpenCL] Added diagnostic for checking length of vector

[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! Thanks!


https://reviews.llvm.org/D30937



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

[PATCH] D30816: [OpenCL] Added implicit conversion rank for overloading functions with vector data type in OpenCL

[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! Thanks!


https://reviews.llvm.org/D30816



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I don't think there is anything wrong with the generation of vec3->vec4 in 
Clang. I believe the motivation for this was the OpenCL spec treating vec3 as 
vec4 aligned type (see section 6.1.5: 
https://www.khronos.org/registry/OpenCL/specs/opencl-2.0-openclc.pdf#12). So in 
terms of memory layout vec3 wouldn't be any different to vec4. But in terms of 
operations (including loads/stores) there can be potential gain from not 
handling the 4th element.  This can be exploited by some targets. I think 
generating the vec3 from the frontend would be a better choice in the first 
place. Because backend can decide how to handle this. Including for 
architectures with no SIMD support it would just generate 3 separate loads. 
Right now it seems that it will be forced to generate 4 loads.

But considering that transformation to vec4 has been the default implementation 
for quite a while in the frontend, I think we would need a stronger motivation 
for switching to original vec3. So current approach with a special flag for 
preserving vec3 should be good enough to fit all needs.


https://reviews.llvm.org/D30810



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

[PATCH] D28136: [OpenCL] Implement as_type operator as alias of __builtin_astype.

[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! Thanks!




Comment at: lib/Headers/opencl-c.h:6588
-char __ovld __cnfn as_char(char);
-char __ovld __cnfn as_char(uchar);
-

bader wrote:
> Anastasia wrote:
> > Why do we have some of the overloads omitted? Would this cause any extra 
> > conversions? uchar -> char in this case?
> It's specific to how builtin_astype works. It drops first argument type and 
> only cares about matching first argument size with the data type size 
> provided as a second argument. So basically with single line we get all 
> possible and impossible overloads of as_char(*).
> This define will pass any variable type char,uchar to builtin_astype.
Ok, I see. I hope we won't get the "impossible" ones though. :)


https://reviews.llvm.org/D28136



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

[PATCH] D30810: Preserve vec3 type.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In https://reviews.llvm.org/D30810#702443, @bruno wrote:

> > As a result, I think it would be good for clang to have both of features 
> > and I would like to stick to the option "-fpresereve-vec3' to change the 
> > behavior easily.
>
> The motivation doesn't seem solid to me, who else is going to benefit from 
> this flag?


There are some off the main tree implementation that would benefit. But in the 
case of AMD GPU 3 loads/stores will be produced instead of 4. Sounds like a 
good optimization to me. As I said in my previous comment I think it should 
have been the default behavior from the beginning, but since different 
implementation landed first we can integrate this one now with an additional 
option.


https://reviews.llvm.org/D30810



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

[PATCH] D26794: [OpenCL] Blocks are allowed to capture arrays in OpenCL 2.0 and higher.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:13645
   
   // Blocks are not allowed to capture arrays.
+  if (!S.getLangOpts().OpenCL && CaptureType->isArrayType()) {

I think the comment should be updated.



Comment at: test/SemaOpenCL/blocks_with_array.cl:9
+  block_t b = ^() {
+// CHECK:   %{{.*}} = getelementptr inbounds [3 x i32], [3 x i32] 
addrspace(4)* %{{.*}}, i64 0, i64 0
+return a[0];

Does this test anything different from the function `foo` below?



Comment at: test/SemaOpenCL/blocks_with_array.cl:21
+  block_t bl1 = ^(){return s.arr[1];};
+// CHECK: define internal spir_func i32 @__foo_block_invoke(i8 addrspace(4)* 
%.block_descriptor)
+// CHECK:   %{{.*}} = getelementptr inbounds %struct.v, %struct.v 
addrspace(4)* %{{.*}}, i32 0, i32 0

Can we check there is a memcpy here?



Comment at: test/SemaOpenCL/blocks_with_array.cl:27
+  block_t bl2 = ^(){return arr[1];};
+// CHECK: define internal spir_func i32 @__foo_block_invoke_2(i8 addrspace(4)* 
%.block_descriptor)
+// CHECK:   %{{.*}} = getelementptr inbounds [2 x i32], [2 x i32] 
addrspace(4)* %{{.*}}, i64 0, i64 1

Can we CHECK-NOT memcpy?



Comment at: test/SemaOpenCL/blocks_with_array.cl:30
+  // array decayed to pointer while captured
+  s.arr[1] = arr[1] = 877;
+}

Do we need this line as well?


https://reviews.llvm.org/D26794



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

[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8254
   "%0 cannot be used as the type of a kernel parameter">;
+def err_opencl_implicit_function_decl : Error<
+  "implicit declaration of function %0 is invalid in OpenCL">;

echuraev wrote:
> Anastasia wrote:
> > Could this be in OpenCL group please?
> But this is already in OpenCL group. Please, see line 8232. In this line is a 
> comment that OpenCL warnings and errors are below.
Ah yes! Sorry, I got confused by the absence of opencl in the neighboring 
diagnostics. :)



Comment at: test/SemaOpenCL/clang-builtin-version.cl:32
+  work_group_reserve_write_pipe(tmp, tmp); // expected-error{{implicit 
declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL}}
+  // expected-note@-1{{did you mean 'work_group_reserve_read_pipe'?}}
+  // expected-note@-2{{'work_group_reserve_write_pipe' declared here}}

echuraev wrote:
> Anastasia wrote:
> > Why do we get this note now? I believe work_group_reserve_read_pipe 
> > shouldn't be available either?
> May be I don't understand something but I think that it is the right 
> diagnostic message. We called work_group_reserve_read_pipe in line 29. So for 
> this case we will get the following message: 
> //clang-builtin-version.cl: 31 error: implicit declaration of function 
> 'work_group_reserve_write_pipe' is invalid in OpenCL
> clang-builtin-version.cl: 31 note: did you mean 
> 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl: 29 note: 'work_group_reserve_read_pipe' declared 
> here//
But there is an error now given for the call to 'work_group_reserve_read_pipe'. 
Why is it still added to the declarations? I think we should prevent this.


https://reviews.llvm.org/D31745



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

[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/clang-builtin-version.cl:32
+  work_group_reserve_write_pipe(tmp, tmp); // expected-error{{implicit 
declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL}}
+  // expected-note@-1{{did you mean 'work_group_reserve_read_pipe'?}}
+  // expected-note@-2{{'work_group_reserve_write_pipe' declared here}}

Anastasia wrote:
> echuraev wrote:
> > Anastasia wrote:
> > > Why do we get this note now? I believe work_group_reserve_read_pipe 
> > > shouldn't be available either?
> > May be I don't understand something but I think that it is the right 
> > diagnostic message. We called work_group_reserve_read_pipe in line 29. So 
> > for this case we will get the following message: 
> > //clang-builtin-version.cl: 31 error: implicit declaration of function 
> > 'work_group_reserve_write_pipe' is invalid in OpenCL
> > clang-builtin-version.cl: 31 note: did you mean 
> > 'work_group_reserve_read_pipe'?
> > clang-builtin-version.cl: 29 note: 'work_group_reserve_read_pipe' declared 
> > here//
> But there is an error now given for the call to 
> 'work_group_reserve_read_pipe'. Why is it still added to the declarations? I 
> think we should prevent this.
Also do you know why we didn't have these notes before? I don't see anything 
related in your change.


https://reviews.llvm.org/D31745



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

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/Basic/AddressSpaces.h:32
+  // QualType represents private address space in OpenCL source code.
+  Default = 0,
+

yaxunl wrote:
> Anastasia wrote:
> > The alloca AS is not taken from the target AS map but from the DataLayout. 
> > This keep me wonder whether the explicit Default item is actually needed 
> > here
> For OpenCL, the default addr space is mapped to alloca addr space. For other 
> languages, it is mapped by the address space mapping table.
Ok. BTW, why is it done differently for other languages than OpenCL now? Is it 
something we missed in the programming model before? Or is it something 
specific to AMD target support?


https://reviews.llvm.org/D31404



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

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

LGTM! Thanks!


https://reviews.llvm.org/D31404



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

[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: include/clang/AST/ASTContext.h:2328
+return AddrSpaceMapMangling || 
+   AS >= LangAS::target_first;
   }

So we couldn't use the  LangAS::Count instead?

I have the same comment in other places that use LangAS::target_first. Why 
couldn't we simply use LangAS::Count? It there any point in having two tags?

Another comment is why do we need ASes specified by 
`__attribute__((address_space(n)))` to be unique enum number at the end of 
named ASes of OpenCL and CUDA? I think conceptually the full range of ASes can 
be used in C because the ASes from OpenCL and CUDA are not available there 
anyways.



Comment at: include/clang/AST/Type.h:339-340
+auto Addr = getAddressSpace();
+if (Addr == 0)
+  return 0;
+return Addr - LangAS::target_first;

t-tye wrote:
> Since you mention this is only used for  `__attribute__((address_space(n)))`, 
> why is this check for 0 needed?
> 
> If it is needed then to match other places should it simply be:
> 
> ```
> if (Addr)
>   return Addr - LangAS::target_first;
> return 0;
> ```
Could we use LangAS::Count instead?



Comment at: lib/AST/ASTContext.cpp:8732
+Type = Context.getAddrSpaceQualType(Type, AddrSpace +
+LangAS::target_first);
 Str = End;

Also here, could we use LangAS::Count instead?



Comment at: lib/AST/ASTContext.cpp:9556
+  if (AS == LangAS::Default && LangOpts.OpenCL)
+return getTargetInfo().getDataLayout().getAllocaAddrSpace();
+  if (AS >= LangAS::target_first)

t-tye wrote:
> An alternative to doing this would be to add an opencl_private to LangAS and 
> have each target map it accordingly. Then this could be:
> 
> ```
> // If a target specific address space was specified, simply return it.
> if (AS >= LangAS::target_first)
>   return AS - LangAS::target_first;
> // For OpenCL, only function local variables are not explicitly marked with
> // an address space in the AST, so treat them as the OpenCL private address 
> space.
> if (!AS && LangOpts.OpenCL)
>   AS = LangAS::opencl_private;
> return (*AddrSpaceMap)[AS];
> ```
> This seems to better express what is happening here. If no address space was 
> specified, and the language is OpenCL, then treat it as OpenCL private and 
> map it according to the target mapping.
> 
> If wanted to eliminate the LangAS::Default named constant then that would be 
> possible as it is no longer being used by name. However, would need to ensure 
> that the first named enumerators starts at 1 so that 0 is left as the "no 
> value explicitly specified" value that each target must map to the target 
> specific generic address space.
I would very much like to see `opencl_private` represented explicitly. This 
would allow us to simplify some parsing and also enable proper support of `NULL 
` literal (that has no AS by the spec).

We can of course do this refactoring work as a separate step.



Comment at: lib/Sema/SemaType.cpp:5537
+llvm::APSInt Offset(addrSpace.getBitWidth(), false);
+Offset = LangAS::target_first;
+addrSpace += Offset;

Do we need this temporary variable here?


https://reviews.llvm.org/D31404



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

[PATCH] D31594: [OpenCL] Enables passing sampler initializer to function argument

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/sampler_t.cl:68
   foo(argsmp);
-  foo(5); // expected-error{{sampler_t variable required - got 'int'}}
+  foo(CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | 
CLK_FILTER_LINEAR);
   sampler_t sa[] = {argsmp, const_smp}; // expected-error {{array of 
'sampler_t' type is invalid in OpenCL}}

I think we could keep just an int value (i.e. 5) as well.

Could we add a case with an error though. To make sure we are testing the 
diagnostic too.


https://reviews.llvm.org/D31594



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