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
