Anastasia created this revision.
There is an issue with taking an address of captured variables, because
captures can be put in different locations depending on the vendor
implementation (and therefore they are passed as generic AS pointer to the
block).
The physical location for the captures can be:
(a) in case the block is called as a lambda function the captures are put on
the stack - in the private AS.
(b) in case the block is used in enqueue it depends on the vendor
implementation to put the captured content in the memory accessible for the
enqueued kernels. In general case global memory can be used which is accessible
everywhere.
Example:
void foo(){
int a;
void (^bl)(void) = ^(void) {
private int* b = &a; // a here is not the same object as a in foo
}
Currently Clang hits `unreachable` due to `bitcast` of generic to private AS
pointer because the original location of `a` is on the stack but the location
of the captures is in generic AS.
This patch disallows taking address of captures because the physical address
space can be different depending on the block use cases and vendor
implementations.
An alternative approached could be (in case we want to allow this code) to
assume captures are located in the generic AS implicitly. However, in this case
programmers should be advised that erroneous AS casts can occur further that
can't be diagnosed by the frontend (i.e. if capture address is used further in
the operations of a different address space to where the captures physical
location is).
Example:
void foo(){
int a;
void (^bl)(void) = ^(void) {
local int* b = (local int*)&a; //valid cast of implicitly generic to
local but &a will be located in private or global AS for the 2 uses below
}
b();
enqueue_kernel(..., b)
}
@Sam, @Alexey, does it make sense taking this route?
https://reviews.llvm.org/D36410
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
@@ -78,3 +78,14 @@
};
return;
}
+
+// Taking address of a capture is not allowed
+int g;
+kernel void f7(int a1) {
+ int a2;
+ void (^bl)(void) = ^(void) {
+ &g; //expected-warning{{expression result unused}}
+ &a1; //expected-error{{taking address of a capture is not allowed}}
+ &a2; //expected-error{{taking address of a capture is not allowed}}
+ };
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -526,14 +526,6 @@
assert(!Ty.isNull() && "DefaultFunctionArrayConversion - missing type");
if (Ty->isFunctionType()) {
- // If we are here, we are not calling a function but taking
- // its address (which is not allowed in OpenCL v1.0 s6.8.a.3).
- if (getLangOpts().OpenCL) {
- if (Diagnose)
- Diag(E->getExprLoc(), diag::err_opencl_taking_function_address);
- return ExprError();
- }
-
if (auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
if (auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl()))
if (!checkAddressOfFunctionIsAvailable(FD, Diagnose, E->getExprLoc()))
@@ -10690,10 +10682,17 @@
// Make sure to ignore parentheses in subsequent checks
Expr *op = OrigOp.get()->IgnoreParens();
- // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
- if (LangOpts.OpenCL && op->getType()->isFunctionType()) {
- Diag(op->getExprLoc(), diag::err_opencl_taking_function_address);
- return QualType();
+ // In OpenCL captures for blocks called as lambda functions
+ // are located in the private address space. Blocks used in
+ // enqueue_kernel can be located in a different address space
+ // depending on a vendor implementation. Thus preventing
+ // taking an address of the capture to avoid invalid AS casts.
+ if (LangOpts.OpenCL) {
+ auto* VarRef = dyn_cast<DeclRefExpr>(op);
+ if (VarRef && VarRef->refersToEnclosingVariableOrCapture()) {
+ Diag(op->getExprLoc(), diag::err_opencl_taking_address_capture);
+ return QualType();
+ }
}
if (getLangOpts().C99) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6948,8 +6948,8 @@
def err_opencl_function_pointer_variable : Error<
"pointers to functions are not allowed">;
-def err_opencl_taking_function_address : Error<
- "taking address of function is not allowed">;
+def err_opencl_taking_address_capture : Error<
+ "taking address of a capture is not allowed">;
def err_invalid_conversion_between_vector_and_scalar : Error<
"invalid conversion between vector type %0 and scalar type %1">;
Index: test/SemaOpenCL/invalid-block.cl
===================================================================
--- test/SemaOpenCL/invalid-block.cl
+++ test/SemaOpenCL/invalid-block.cl
@@ -78,3 +78,14 @@
};
return;
}
+
+// Taking address of a capture is not allowed
+int g;
+kernel void f7(int a1) {
+ int a2;
+ void (^bl)(void) = ^(void) {
+ &g; //expected-warning{{expression result unused}}
+ &a1; //expected-error{{taking address of a capture is not allowed}}
+ &a2; //expected-error{{taking address of a capture is not allowed}}
+ };
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -526,14 +526,6 @@
assert(!Ty.isNull() && "DefaultFunctionArrayConversion - missing type");
if (Ty->isFunctionType()) {
- // If we are here, we are not calling a function but taking
- // its address (which is not allowed in OpenCL v1.0 s6.8.a.3).
- if (getLangOpts().OpenCL) {
- if (Diagnose)
- Diag(E->getExprLoc(), diag::err_opencl_taking_function_address);
- return ExprError();
- }
-
if (auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
if (auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl()))
if (!checkAddressOfFunctionIsAvailable(FD, Diagnose, E->getExprLoc()))
@@ -10690,10 +10682,17 @@
// Make sure to ignore parentheses in subsequent checks
Expr *op = OrigOp.get()->IgnoreParens();
- // OpenCL v1.0 s6.8.a.3: Pointers to functions are not allowed.
- if (LangOpts.OpenCL && op->getType()->isFunctionType()) {
- Diag(op->getExprLoc(), diag::err_opencl_taking_function_address);
- return QualType();
+ // In OpenCL captures for blocks called as lambda functions
+ // are located in the private address space. Blocks used in
+ // enqueue_kernel can be located in a different address space
+ // depending on a vendor implementation. Thus preventing
+ // taking an address of the capture to avoid invalid AS casts.
+ if (LangOpts.OpenCL) {
+ auto* VarRef = dyn_cast<DeclRefExpr>(op);
+ if (VarRef && VarRef->refersToEnclosingVariableOrCapture()) {
+ Diag(op->getExprLoc(), diag::err_opencl_taking_address_capture);
+ return QualType();
+ }
}
if (getLangOpts().C99) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6948,8 +6948,8 @@
def err_opencl_function_pointer_variable : Error<
"pointers to functions are not allowed">;
-def err_opencl_taking_function_address : Error<
- "taking address of function is not allowed">;
+def err_opencl_taking_address_capture : Error<
+ "taking address of a capture is not allowed">;
def err_invalid_conversion_between_vector_and_scalar : Error<
"invalid conversion between vector type %0 and scalar type %1">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
