[PATCH] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks
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] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks! 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 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] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks
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
yaxunl added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:723 +? CGM.getNSConcreteStackBlock() +: llvm::Constant::getNullValue( + CGM.getNSConcreteStackBlock()->getType()); Anastasia wrote: > 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. The LLVM type may not have sufficient information, so in general situation QualType is needed. The comment before getNullPointer declaration explains the meaning of the parameters: ``` /// Get target specific null pointer. /// \param T is the LLVM type of the null pointer. /// \param QT is the clang QualType of the null pointer. /// \return ConstantPointerNull with the given type \p T. /// Each target can override it to return its own desired constant value. virtual llvm::Constant *getNullPointer(const CodeGen::CodeGenModule , llvm::PointerType *T, QualType QT) const; ``` 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
yaxunl added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:726 + CGM.getNSConcreteStackBlock()->getType()), + QualType()); isa = llvm::ConstantExpr::getBitCast(isa, VoidPtrTy); The QualType needs to be the real QualType corresponding to the LLVM type, otherwise it won't work on targets with non-zero null pointer. Comment at: lib/CodeGen/CGBlocks.cpp:1129 + CGM.getNSConcreteGlobalBlock()->getType()), + QualType())); same issue as above. 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 *); Can we add a run line for triple amdgcn-amd-amdhsa-opencl to make sure the null pointer is generated correctly? 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 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] D28814: [OpenCL] Add missing address spaces in IR generation of Blocks
yaxunl added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:723 +? CGM.getNSConcreteStackBlock() +: llvm::Constant::getNullValue( + CGM.getNSConcreteStackBlock()->getType()); should use CGM.getNullPointer to create a null pointer. Comment at: lib/CodeGen/CGBlocks.cpp:1124 + ? CGM.getNSConcreteGlobalBlock() + : llvm::Constant::getNullValue( + CGM.getNSConcreteGlobalBlock()->getType())); should use CGM.getNullPointer to create a null pointer. 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 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 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
