Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
jvesely added a comment. In http://reviews.llvm.org/D19932#465807, @yaxunl wrote: > > this is not true. as I pointed out earlier, take a look at libclc headers. > > a lot functions are defined for multiple types while maintaining type > > safety. > > > there is no problem having TYPE * to_global(TYPE *), for every permissible > > CLC type, declared in headers without any builtin. > > > This function allows user defined types. How do you declare that in a header > file? ah, now it makes sense, thanks. the void* argument let me astray. I'd say you can still do it using //typeof//, but I see how builtin would be preferable to that. Repository: rL LLVM http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl added a comment. > this is not true. as I pointed out earlier, take a look at libclc headers. a > lot functions are defined for multiple types while maintaining type safety. > there is no problem having TYPE * to_global(TYPE *), for every permissible > CLC type, declared in headers without any builtin. This function allows user defined types. How do you declare that in a header file? Repository: rL LLVM http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
jvesely added a comment. In http://reviews.llvm.org/D19932#465784, @yaxunl wrote: > In http://reviews.llvm.org/D19932#465781, @jvesely wrote: > > > In http://reviews.llvm.org/D19932#465763, @cfe-commits wrote: > > > > > The returned pointer should point to the same pointee type as the > > > argument. Header file cannot guarantee that. > > > > > > Sam > > > > > > how come? is there a possibility to have two different types using the same > > name? > > > Because the pointee type is arbitrary, you can only define it as > > global void* to_global(void*); > > > in the header file. Then you could have > > int *a; > global double *b = to_global(a); > > > without diagnostics, but the spec requires that to_global(a) should have > global int* type, therefore there should be some diagnostics. this is not true. as I pointed out earlier, take a look at libclc headers. a lot functions are defined for multiple types while maintaining type safety. there is no problem having TYPE * to_global(TYPE *), for every permissible CLC type, declared in headers without any builtin. Repository: rL LLVM http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl added a comment. In http://reviews.llvm.org/D19932#465781, @jvesely wrote: > In http://reviews.llvm.org/D19932#465763, @cfe-commits wrote: > > > The returned pointer should point to the same pointee type as the argument. > > Header file cannot guarantee that. > > > > Sam > > > how come? is there a possibility to have two different types using the same > name? Because the pointee type is arbitrary, you can only define it as global void* to_global(void*); in the header file. Then you could have int *a; global double *b = to_global(a); without diagnostics, but the spec requires that to_global(a) should have global int* type, therefore there should be some diagnostics. Repository: rL LLVM http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
jvesely added a comment. In http://reviews.llvm.org/D19932#465763, @cfe-commits wrote: > The returned pointer should point to the same pointee type as the argument. > Header file cannot guarantee that. > > Sam how come? is there a possibility to have two different types using the same name? Repository: rL LLVM http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
The returned pointer should point to the same pointee type as the argument.
Header file cannot guarantee that.
Sam
-Original Message-
From: Jan Vesely [mailto:jan.ves...@rutgers.edu]
Sent: Wednesday, June 22, 2016 10:20 PM
To: Liu, Yaxun (Sam) <yaxun@amd.com>; xiuli...@outlook.com;
anastasia.stul...@arm.com
Cc: jan.ves...@rutgers.edu; Stellard, Thomas <tom.stell...@amd.com>;
cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin
functions.
jvesely added a subscriber: jvesely.
jvesely added a comment.
Why couldn't this be declared in CLC header? There are multiple instances of
declaring functions for all available types in libclc (see for example convert
functions in convert.h).
Repository:
rL LLVM
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL270261: [OpenCL] Add to_{global|local|private} builtin
functions. (authored by yaxunl).
Changed prior to commit:
http://reviews.llvm.org/D19932?vs=57478=57972#toc
Repository:
rL LLVM
http://reviews.llvm.org/D19932
Files:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -1305,6 +1305,11 @@
LANGBUILTIN(get_pipe_num_packets, "Ui.", "tn", OCLC_LANG)
LANGBUILTIN(get_pipe_max_packets, "Ui.", "tn", OCLC_LANG)
+// OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+LANGBUILTIN(to_global, "v*v*", "tn", OCLC_LANG)
+LANGBUILTIN(to_local, "v*v*", "tn", OCLC_LANG)
+LANGBUILTIN(to_private, "v*v*", "tn", OCLC_LANG)
+
#undef BUILTIN
#undef LIBBUILTIN
#undef LANGBUILTIN
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7856,6 +7856,8 @@
def warn_opencl_attr_deprecated_ignored : Warning <
"%0 attribute is deprecated and ignored in OpenCL version %1">,
InGroup;
+def err_opencl_builtin_requires_version : Error<
+ "%0 requires OpenCL version %1%select{| or above}2">;
// OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
def err_opencl_builtin_pipe_first_arg : Error<
@@ -7887,6 +7889,11 @@
def err_opencl_extern_block_declaration : Error<
"invalid block variable declaration - using 'extern' storage class is disallowed">;
+// OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+def err_opencl_builtin_to_addr_arg_num : Error<
+ "invalid number of arguments to function: %0">;
+def err_opencl_builtin_to_addr_invalid_arg : Error<
+ "invalid argument %0 to function: %1, expecting a generic pointer argument">;
} // end of sema category
let CategoryName = "OpenMP Issue" in {
Index: cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
===
--- cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
+++ cfe/trunk/test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ constant int *con;
+ typedef constant int const_int_ty;
+ const_int_ty *con_typedef;
+
+ glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+ int x;
+ glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ glob = to_global(con_typedef);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con_typedef to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+ global char *glob_c = to_global(loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
+#endif
+
+}
Index: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
===
--- cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
+++ cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %[[A:.*]] = type { float, float, float }
+typedef struct {
+ float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl updated this revision to Diff 57478.
yaxunl marked 8 inline comments as done.
yaxunl added a comment.
Revised as Anastasia suggested.
http://reviews.llvm.org/D19932
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
test/CodeGenOpenCL/to_addr_builtin.cl
test/SemaOpenCL/to_addr_builtin.cl
Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ constant int *con;
+ typedef constant int const_int_ty;
+ const_int_ty *con_typedef;
+
+ glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+ int x;
+ glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ glob = to_global(con_typedef);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con_typedef to function: 'to_global', expecting a generic pointer argument}}
+#endif
+
+ loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+ global char *glob_c = to_global(loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' requires OpenCL version 2.0 or above}}
+#else
+ // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
+#endif
+
+}
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %[[A:.*]] = type { float, float, float }
+typedef struct {
+ float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ private int *priv;
+ generic int *gen;
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(glob);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(loc);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(priv);
+
+ //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(gen);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(glob);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(loc);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(priv);
+
+ //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)*
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
Anastasia added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:501
@@ -500,1 +500,3 @@
def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
+def err_builtin_needs_opencl_version
+: Error<"%0 needs OpenCL version %1%select{| or above}2">;
Could we move this to all the other OpenCL diagnostics to have them grouped
together?
Comment at: lib/Sema/SemaChecking.cpp:463
@@ +462,3 @@
+// \return True if a semantic error has been found, false otherwise.
+static bool SemaBuiltinToAddr(Sema , unsigned BuiltinID, CallExpr *Call) {
+ // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
Could we rename this please to SemaOpenCLBuiltinToAddr.
I think Pipe functions should be better renamed too at some point.
Comment at: lib/Sema/SemaChecking.cpp:478
@@ +477,3 @@
+ auto RT = Call->getArg(0)->getType();
+ if (!RT->isPointerType() || RT->getPointeeType().getCanonicalType()
+ .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
Could the second check be written a bit simpler? I have a feeling something
like this might work too:
RT->getPointeeType().getAddressSpace()
Comment at: lib/Sema/SemaChecking.cpp:485
@@ +484,3 @@
+
+ RT = RT->getPointeeType().getCanonicalType();
+ auto Qual = RT.getQualifiers();
Is canonical type really necessary here?
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
pxli168 accepted this revision.
pxli168 added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: test/SemaOpenCL/to_addr_builtin.cl:26
@@ +25,3 @@
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
Anastasia wrote:
> @Xiuli, I think Sam is right. Passing constant to generic violates AS
> conversion rules from s6.5.5. So error would be correct behavior of the
> compiler.
Yes, I think I just misunderstand some address space problem.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
Anastasia added inline comments.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:26
@@ +25,3 @@
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
@Xiuli, I think Sam is right. Passing constant to generic violates AS
conversion rules from s6.5.5. So error would be correct behavior of the
compiler.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl marked 8 inline comments as done.
Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+ .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() <<
Call->getSourceRange();
Anastasia wrote:
> I just think you should simply set the function return type in line 487 based
> on the passed argument type (RT variable declared in line 477).
>
> The situation I would like us to avoid here is:
>
> int *gptr = ..;
> local char *lptr = to_local(gptr);
>
> So basically by calling to_local here, we allow conversion of not only an
> address space but also an underlaying type silently.
>
> Could we add a test to make sure it's not happening.
>
> Similarly, in C such situations trigger a warning:
> int *ptr1 = ...;
> char *ptr2 = ptr1; //expected-warning{{incompatible pointer types
> initializing 'char *' with an expression of type 'int *'}}
I added the above test. The current implementation is able to diagnose it.
Your suggested approach also works, so I will remove dynamic generation of
to_addr declarations since it seems to be an overkill.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
pxli168 wrote:
> yaxunl wrote:
> > pxli168 wrote:
> > > But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
> > >
> > > > Returns a pointer that points to a region in the global address space
> > > > if to_global can cast ptr to the global address space. Otherwise it
> > > > returns NULL.
> > >
> > I think this is only for valid call expression. For constant pointer, it is
> > an invalid call expression due to s6.5.5, so an error of invalid argument
> > should be emitted.
> >
> > If we allow passing constant pointer to this function, we violates s6.5.5.
> >A pointer that points to the constant address space cannot be cast or
> >implicitly converted to the generic address space.
>
> So if we can not cast, we maybe should return NULL?
> @Anastasia What do you think about this?
>
>
What the function returns is implemented by builtin library and Clang does not
care about that. Clang only does syntax checking, type checking, etc. Since
this is still a function, it is subject to all these rules. Unless we want to
exempt this function from the type-checking rules. However I don't think the
spec implies that.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
pxli168 added inline comments.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
yaxunl wrote:
> pxli168 wrote:
> > But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
> >
> > > Returns a pointer that points to a region in the global address space if
> > > to_global can cast ptr to the global address space. Otherwise it returns
> > > NULL.
> >
> I think this is only for valid call expression. For constant pointer, it is
> an invalid call expression due to s6.5.5, so an error of invalid argument
> should be emitted.
>
> If we allow passing constant pointer to this function, we violates s6.5.5.
>A pointer that points to the constant address space cannot be cast or
>implicitly converted to the generic address space.
So if we can not cast, we maybe should return NULL?
@Anastasia What do you think about this?
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
Anastasia added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+ .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() <<
Call->getSourceRange();
I just think you should simply set the function return type in line 487 based
on the passed argument type (RT variable declared in line 477).
The situation I would like us to avoid here is:
int *gptr = ..;
local char *lptr = to_local(gptr);
So basically by calling to_local here, we allow conversion of not only an
address space but also an underlaying type silently.
Could we add a test to make sure it's not happening.
Similarly, in C such situations trigger a warning:
int *ptr1 = ...;
char *ptr2 = ptr1; //expected-warning{{incompatible pointer types
initializing 'char *' with an expression of type 'int *'}}
Comment at: lib/Sema/SemaExpr.cpp:5222-5232
@@ -5166,6 +5221,13 @@
if (FDecl && FDecl->getBuiltinID()) {
- // Rewrite the function decl for this builtin by replacing parameters
- // with no explicit address space with the address space of the arguments
- // in ArgExprs.
- if ((FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl,
ArgExprs))) {
+ // Rewrite the function decl for OpenCL to_addr builtin.
+ if (FunctionDecl *NFDecl = rewriteBuiltinFunctionDeclForOpenCLToAddr(
+this, Context, FDecl, ArgExprs))
+FDecl = NFDecl;
+ else {
+// Rewrite the function decl for this builtin by replacing parameters
+// with no explicit address space with the address space of the
arguments
+// in ArgExprs.
+FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl, ArgExprs);
+ }
+ if (FDecl) {
NDecl = FDecl;
I still don't understand the issue here.
In my understanding what we need is:
(1) Check the passed argument is a pointer and set return type to match the
pointer type exactly in SemaChecking.
(2) Generate to_addr with i8* in CGBuiltin casting to and back the original
type used in the call.
See my comment to lib/Sema/SemaChecking.cpp. I think if you set the return type
there correctly we won't need this change here.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:21
@@ +20,3 @@
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global'}}
+#endif
May be we could add here something like: expecting a generic pointer type...
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Updated summary.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
pxli168 wrote:
> But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
>
> > Returns a pointer that points to a region in the global address space if
> > to_global can cast ptr to the global address space. Otherwise it returns
> > NULL.
>
I think this is only for valid call expression. For constant pointer, it is an
invalid call expression due to s6.5.5, so an error of invalid argument should
be emitted.
If we allow passing constant pointer to this function, we violates s6.5.5.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
pxli168 added a comment.
You can update the SUMMARY of this diff as we are now using generic codegen
output.
And there are some inline comments about spec.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
> Returns a pointer that points to a region in the global address space if
> to_global can cast ptr to the global address space. Otherwise it returns NULL.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl updated this revision to Diff 56605.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Generate calls to void addr* to_addr(void*) with bitcasts.
http://reviews.llvm.org/D19932
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Decl.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaExpr.cpp
test/CodeGenOpenCL/to_addr_builtin.cl
test/SemaOpenCL/to_addr_builtin.cl
Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ constant int *con;
+
+ glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+ int x;
+ glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global'}}
+#endif
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con to function: 'to_global'}}
+#endif
+
+ loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+}
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %[[A:.*]] = type { float, float, float }
+typedef struct {
+ float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ private int *priv;
+ generic int *gen;
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(glob);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(loc);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(priv);
+
+ //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
+ glob = to_global(gen);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(glob);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(loc);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(priv);
+
+ //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
+ loc = to_local(gen);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
+ priv = to_private(glob);
+
+ //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
+ //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+ //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
+ priv =
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl marked 4 inline comments as done. Comment at: test/SemaOpenCL/to_addr_builtin.cl:24 @@ +23,3 @@ + + glob = to_global(con); +#if __OPENCL_C_VERSION__ < CL_VERSION_2_0 pxli168 wrote: > should this return a NULL or a build error? the function is declared as global gentype* to_addr(generic gentype*); since constant pointer cannot be implicitly casted to a generic pointer (OpenCL v2.0 s6.5.5), this should cause a compilation error. http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
pxli168 added a comment.
The pointer type seems to be not that important, we can just cast it to any
type we want.
Comment at: test/SemaOpenCL/to_addr_builtin.cl:24
@@ +23,3 @@
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
should this return a NULL or a build error?
Comment at: test/SemaOpenCL/to_addr_builtin.cl:35
@@ +34,3 @@
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes
address space of pointer}}
+#endif
I think this will be handled by pointer bitcast if we used some void* as well,
the address space is the important thing of these built-ins.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl marked 5 inline comments as done.
yaxunl added a comment.
> I agree with Xiuli, if we don't have generic implementation with i8*, the
> libraries would have to contain every possible implementation of each AS
> conversion builtin including user defined types (which we don't even know
> beforehand).
>
> I think that bitcast is not an issue in general as soon as passed type is
> casted from and back to the original type. This way we will disallow loosing
> the type information if let's say we compile:
>
> int *ptr = ...
> to_global(ptr) -> the return type is global int* (and not global void*)
>
>
> Because we can create a bitcast to the original type.
These builtin functions can be lowered by a module pass which tracing the
argument to determine the real addr space. In that case it does not matter that
it can take infinite number of names.
However, there may be platforms on which these builtin functions are
implemented in runtime library. I agree code generated as such will cause
difficulty for them. Therefore I will emit call to i8 addrspace(1)* to_global
(i8* addrspace(4)) and insert bitcasts. Also the name will not be mangled.
Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+ .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() <<
Call->getSourceRange();
Anastasia wrote:
> Should we also check that argument and return pointer type match?
>
> Also we should probably check that the return type is in the right AS.
The return type is generated based on the function and argument type. It always
matches the argument type and has right AS. If the context of this call expr
expects a different type, diagnostics will be emitted by the enclosing context.
Comment at: lib/Sema/SemaExpr.cpp:5222
@@ -5166,5 +5221,3 @@
if (FDecl && FDecl->getBuiltinID()) {
- // Rewrite the function decl for this builtin by replacing parameters
- // with no explicit address space with the address space of the arguments
- // in ArgExprs.
- if ((FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl,
ArgExprs))) {
+ // Rewrite the function decl for OpenCL to_addr builtin.
+ if (FunctionDecl *NFDecl = rewriteBuiltinFunctionDeclForOpenCLToAddr(
Anastasia wrote:
> Is there any reason not to do normal IR CodeGen in CGBuiltin.cpp like we did
> for pipes:
> http://reviews.llvm.org/D15914
>
> Otherwise, it seems like we add an extra overhead for all the builtins and it
> doesn't seem any simpler code either.
In that case the type of the call expr does not need to be dynamically
generated.
In this case, the type of the call expr depends on the argument type. If we
don't dynamically generate the prototype of the builtin function, we have to
insert bitcast, which results in an AST which is no longer a call expr and the
diagnostics will become ugly.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
Anastasia added a comment.
In http://reviews.llvm.org/D19932#422374, @yaxunl wrote:
> In http://reviews.llvm.org/D19932#421961, @pxli168 wrote:
>
> > Could we output a generic function in CodeGen?
> > This seems to have no big difference to have a lot of declaration in an
> > opencl c header file.
>
>
> What return type and argument type to use would you suggest for this generic
> function? If it is something like
>
> i8 addrspace(1)* to_global(i8 addrspace(4)*)
>
>
> then bitcasts will be needed, which is what we want to avoid.
>
> These functions accept pointers to arbitrary user types, so they cannot be
> declared in a header file.
I agree with Xiuli, if we don't have generic implementation with i8*, the
libraries would have to contain every possible implementation of each AS
conversion builtin including user defined types (which we don't even know
beforehand).
I think that bitcast is not an issue in general as soon as passed type is
casted from and back to the original type. This way we will disallow loosing
the type information if let's say we compile:
int *ptr = ...
to_global(ptr) -> the return type is global int* (and not global void*)
Because we can create a bitcast to the original type.
Comment at: include/clang/Basic/Builtins.def:1292
@@ +1291,3 @@
+// OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+LANGBUILTIN(to_global, "v*v*", "tn", OCLC_LANG)
+LANGBUILTIN(to_local, "v*v*", "tn", OCLC_LANG)
I think we should not enable those builtins for all OpenCL versions as it
prevents using those identifiers. I don't think they are reserved in the
earlier OpenCL versions?
But I have a patch fixing it for all CL2.0 builtins. I can upload later. No
need to fix now!
Comment at: lib/CodeGen/CGBuiltin.cpp:2127
@@ -2126,1 +2126,3 @@
+ // OpenCL v2.0 s6.13.9 Address space qualifier functions.
+ case Builtin::BIto_global:
Could you add "-" before "Address ..." to match general style.
Comment at: lib/CodeGen/CGBuiltin.cpp:2138
@@ +2137,3 @@
+Builder.CreateCall(CGM.CreateRuntimeFunction(FTy,
+ CGM.getMangledName(E->getDirectCallee())), {Arg0}));
+ }
I don't think mangling would work here at all. See my general comment to the
generated prototype discussion.
Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+ .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() <<
Call->getSourceRange();
Should we also check that argument and return pointer type match?
Also we should probably check that the return type is in the right AS.
Comment at: lib/Sema/SemaExpr.cpp:5222
@@ -5166,5 +5221,3 @@
if (FDecl && FDecl->getBuiltinID()) {
- // Rewrite the function decl for this builtin by replacing parameters
- // with no explicit address space with the address space of the arguments
- // in ArgExprs.
- if ((FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl,
ArgExprs))) {
+ // Rewrite the function decl for OpenCL to_addr builtin.
+ if (FunctionDecl *NFDecl = rewriteBuiltinFunctionDeclForOpenCLToAddr(
Is there any reason not to do normal IR CodeGen in CGBuiltin.cpp like we did
for pipes:
http://reviews.llvm.org/D15914
Otherwise, it seems like we add an extra overhead for all the builtins and it
doesn't seem any simpler code either.
http://reviews.llvm.org/D19932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl added a comment. In http://reviews.llvm.org/D19932#421961, @pxli168 wrote: > Could we output a generic function in CodeGen? > This seems to have no big difference to have a lot of declaration in an > opencl c header file. What return type and argument type to use would you suggest for this generic function? If it is something like i8 addrspace(1)* to_global(i8 addrspace(4)*) then bitcasts will be needed, which is what we want to avoid. These functions accept pointers to arbitrary user types, so they cannot be declared in a header file. http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
pxli168 added a comment. Could we output a generic function in CodeGen? This seems to have no big difference to have a lot of declaration in an opencl c header file. http://reviews.llvm.org/D19932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl updated this revision to Diff 56194.
yaxunl added a comment.
Update the test.
http://reviews.llvm.org/D19932
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Decl.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaExpr.cpp
test/CodeGenOpenCL/to_addr_builtin.cl
test/SemaOpenCL/to_addr_builtin.cl
Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ constant int *con;
+
+ glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+ int x;
+ glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global'}}
+#endif
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con to function: 'to_global'}}
+#endif
+
+ loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+}
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: %[[A:.*]] = type { float, float, float }
+typedef struct {
+ float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ private int *priv;
+ generic int *gen;
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS1i(i32 addrspace(1)* %{{.*}})
+ glob = to_global(glob);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS3i(i32 addrspace(3)* %{{.*}})
+ glob = to_global(loc);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPi(i32* %{{.*}})
+ glob = to_global(priv);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS4i(i32 addrspace(4)* %{{.*}})
+ glob = to_global(gen);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS1i(i32 addrspace(1)* %{{.*}})
+ loc = to_local(glob);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS3i(i32 addrspace(3)* %{{.*}})
+ loc = to_local(loc);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPi(i32* %{{.*}})
+ loc = to_local(priv);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS4i(i32 addrspace(4)* %{{.*}})
+ loc = to_local(gen);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS1i(i32 addrspace(1)* %{{.*}})
+ priv = to_private(glob);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS3i(i32 addrspace(3)* %{{.*}})
+ priv = to_private(loc);
+
+ //CHECK: call i32* @_Z10to_privatePi(i32* %{{.*}})
+ priv = to_private(priv);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS4i(i32 addrspace(4)* %{{.*}})
+ priv = to_private(gen);
+
+ //CHECK; call %[[A]] addrspace(1)* @_Z9to_globalP1A(%[[A]]* %{{.*}})
+ PA pA;
+ GA gA = to_global(pA);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5055,6 +5055,61 @@
return Callee->getMinRequiredArguments() <= NumArgs;
}
+/// OpenCL to_addr function accepts pointers to arbitrary type as argument.
+/// This function change the original declaration to match the argument.
+/// \return nullptr if this builtin is not OpenCL to_addr builtin function or
+/// there is no need to change the function declaration.
+static FunctionDecl *
+rewriteBuiltinFunctionDeclForOpenCLToAddr(Sema *Sema, ASTContext ,
+ const FunctionDecl *FDecl,
+ MultiExprArg ArgExprs) {
+ auto ID = FDecl->getBuiltinID();
+ if (ID != Builtin::BIto_global &&
+ ID != Builtin::BIto_local &&
+ ID != Builtin::BIto_private)
+return nullptr;
+
+ auto ArgT = ArgExprs[0]->getType();
+ if (!ArgT->isPointerType())
+return nullptr;
+
+ auto RT = ArgT->getPointeeType().getCanonicalType();
+ auto Qual = RT.getQualifiers();
+ switch (ID) {
+ case Builtin::BIto_global:
+Qual.setAddressSpace(LangAS::opencl_global);
+break;
+ case Builtin::BIto_local:
+Qual.setAddressSpace(LangAS::opencl_local);
+break;
+ default:
+Qual.removeAddressSpace();
+ }
+ RT =
[PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.
yaxunl created this revision.
yaxunl added reviewers: Anastasia, pxli168.
yaxunl added subscribers: cfe-commits, tstellarAMD.
OpenCL builtin functions to_{global|local|private} accepts argument of pointer
type to arbitrary pointee type, and return a pointer to the same pointee type
in different addr space, i.e.
global gentype *to_global(gentype *p);
It is not desirable to declare it as
global void *to_global(void *);
in opencl header file since it misses diagnostics and generates extra bitcasts.
This patch implements these builtin functions as Clang builtin functions. In
the builtin def file they are defined to have signature void*(void*). When
handling call expressions, their declarations are re-written to have correct
parameter type and return type corresponding to the call argument.
http://reviews.llvm.org/D19932
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Decl.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaExpr.cpp
test/CodeGenOpenCL/to_addr_builtin.cl
test/SemaOpenCL/to_addr_builtin.cl
Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only -cl-std=CL2.0 %s
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ constant int *con;
+
+ glob = to_global(glob, loc);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid number of arguments to function: 'to_global'}}
+#endif
+
+ int x;
+ glob = to_global(x);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument x to function: 'to_global'}}
+#endif
+
+ glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{invalid argument con to function: 'to_global'}}
+#endif
+
+ loc = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+ // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}
+#else
+ // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+#endif
+
+}
Index: test/CodeGenOpenCL/to_addr_builtin.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/to_addr_builtin.cl
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+
+typedef struct {
+ float x,y,z;
+} A;
+typedef private A *PA;
+typedef global A *GA;
+
+void test(void) {
+ global int *glob;
+ local int *loc;
+ private int *priv;
+ generic int *gen;
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS1v(i32 addrspace(1)* %{{.*}})
+ glob = to_global(glob);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS3v(i32 addrspace(3)* %{{.*}})
+ glob = to_global(loc);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPv(i32* %{{.*}})
+ glob = to_global(priv);
+
+ //CHECK: call i32 addrspace(1)* @_Z9to_globalPU3AS4v(i32 addrspace(4)* %{{.*}})
+ glob = to_global(gen);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS1v(i32 addrspace(1)* %{{.*}})
+ loc = to_local(glob);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS3v(i32 addrspace(3)* %{{.*}})
+ loc = to_local(loc);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPv(i32* %{{.*}})
+ loc = to_local(priv);
+
+ //CHECK: call i32 addrspace(3)* @_Z8to_localPU3AS4v(i32 addrspace(4)* %{{.*}})
+ loc = to_local(gen);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS1v(i32 addrspace(1)* %{{.*}})
+ priv = to_private(glob);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS3v(i32 addrspace(3)* %{{.*}})
+ priv = to_private(loc);
+
+ //CHECK: call i32* @_Z10to_privatePv(i32* %{{.*}})
+ priv = to_private(priv);
+
+ //CHECK: call i32* @_Z10to_privatePU3AS4v(i32 addrspace(4)* %{{.*}})
+ priv = to_private(gen);
+
+ PA pA;
+ GA gA = to_global(pA);
+ };
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5055,6 +5055,61 @@
return Callee->getMinRequiredArguments() <= NumArgs;
}
+/// OpenCL to_addr function accepts pointers to arbitrary type as argument.
+/// This function change the original declaration to match the argument.
+/// \return nullptr if this builtin is not OpenCL to_addr builtin function or
+/// there is no need to change the function declaration.
+static FunctionDecl *
+rewriteBuiltinFunctionDeclForOpenCLToAddr(Sema *Sema, ASTContext ,
+ const FunctionDecl *FDecl,
+ MultiExprArg ArgExprs) {
+ auto ID =
