Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-03-01 Thread Anastasia Stulova via cfe-commits 
Anastasia accepted this revision.
Anastasia added a comment.

LGTM!


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-25 Thread Yaxun Liu via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-24 Thread Xiuli PAN via cfe-commits 
pxli168 updated this revision to Diff 49008.
pxli168 added a comment.

1. Make new indent and leave space for the incoming OpenCL C++.
2. Check for the index to see if they are integers.


http://reviews.llvm.org/D16876

Files:
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -5,51 +5,51 @@
   reserve_id_t rid;
 
   // read/write_pipe
-  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}}
-  read_pipe(p);   // expected-error {{invalid number of arguments to function: read_pipe}}
-  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}}
-  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}}
-  read_pipe(p, tmp);   // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}}
+  read_pipe(tmp, p);// expected-error {{first argument to 'read_pipe' must be a pipe type}}
+  read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'int *')}}
   write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
   write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
 
   // reserve_read/write_pipe
-  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}}
-  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}}
+  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function 'reserve_read_pipe' (expecting 'unsigned int')}}
+  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to 'work_group_reserve_read_pipe' must be a pipe type}}
   sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}}
 
   // commit_read/write_pipe
-  commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}}
-  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
+  commit_read_pipe(tmp, rid);// expected-error{{first argument to 'commit_read_pipe' must be a pipe type}}
+  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function 'work_group_commit_read_pipe' (expecting 'reserve_id_t')}}
   sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}}
 }
 
 void test2(write_only pipe int p, global int* ptr){
   int tmp;
   reserve_id_t rid;
 
   // read/write_pipe
-  write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}}
-  write_pipe(p);   // expected-error {{invalid number of arguments to function: write_pipe}}
-  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}}
-  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}}
-  write_pipe(p, tmp);   // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}}
+  write_pipe(tmp, p);// expected-error {{first argument to 'write_pipe' must be a pipe type}}
+  write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'int *')}}
   read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
   read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
 
   // reserve_read/write_pipe
-  reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting 'unsigned int')}}
-

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-23 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

In http://reviews.llvm.org/D16876#359786, @pekka.jaaskelainen wrote:

> In http://reviews.llvm.org/D16876#359781, @Anastasia wrote:
>
> > @Pekka, do you have any more comments?
>
>
> Nope. Looking forward to finally implementing proper pipe support to pocl.
>
> With the future Clang OpenCL patches I appreciate if you keep adding me to 
> the cc list of the reviews, but it might be pointless to wait for my LGTM as 
> I'm still quite a Clang noob, thus that "LGTM" might not have the "weight" it 
> should have :) In other words, I keep monitoring the patches and will yell if 
> I see something that sticks to my eye, but no point in blocking the progress 
> due to possibly slow acks from my side.


Sure, no problem! Thanks!


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-23 Thread Pekka Jääskeläinen via cfe-commits 
pekka.jaaskelainen accepted this revision.
pekka.jaaskelainen added a comment.
This revision is now accepted and ready to land.

In http://reviews.llvm.org/D16876#359781, @Anastasia wrote:

> @Pekka, do you have any more comments?


Nope. Looking forward to finally implementing proper pipe support to pocl.

With the future Clang OpenCL patches I appreciate if you keep adding me to the 
cc list of the reviews, but it might be pointless to wait for my LGTM as I'm 
still quite a Clang noob, thus that "LGTM" might not have the "weight" it 
should have :) In other words, I keep monitoring the patches and will yell if I 
see something that sticks to my eye, but no point in blocking the progress due 
to possibly slow acks from my side.


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-23 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

Could you please address number 1 from my previous comment?

Otherwise, I think we should try to proceed quickly here, it will be too hard 
to merge back in after long delay and also it would be nice to have as many 
corrections as possible ASAP.

Could we move Richard to subscribers for now, as he can give his feedback later 
too.

@Pekka, do you have any more comments?


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-09 Thread Anastasia Stulova via cfe-commits 
Anastasia added a comment.

Related to:

1. I think I would still add space. You can also reformat other lines. There 
are only 5 lines above.
2. Agree.
3. Feels like may be we should try to see if the passed argument is convertible 
to the function parameter type.

For example, I see that some builtins in SemaChecking.cpp use 
DefaultFunctionArrayLvalueConversion to try converting to a pointer type. I am 
not sure what we could do for other types though  especially for the OpenCL 
types.

@Richard, would you be able to give us more information here:

  > +  case 4: {
  > +if (checkOpenCLPipeArg(S, Call))
  > +  return true;
  > +// The call with 4 arguments should be
  > +// read/write_pipe(pipe T, reserve_id_t, uint, T*)
  > +// check reserve_id_t
  > +if (!Call->getArg(1)->getType()->isReserveIDT()) {
  
  You should attempt to implicitly convert to the desired type here, rather 
than demanding the right type, to match the normal call semantics. Likewise 
elsewhere in this patch.
   


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-05 Thread Xiuli PAN via cfe-commits 
pxli168 requested a review of this revision.
pxli168 added a reviewer: rsmith.
pxli168 added a comment.

Hi Richard/Anastasia,

I replied in the commit email, and here are some explains:

1. Without the space after comma the "//" will be aligned.
2. We want generic prototypes of the builtin functions, and we generate them in 
CodeGen. So there will be no use to perform conversion in sema for the function 
prototype.
3. We used generic prototypes and varadic function declration, the conversion 
in CodeGen use cast and won't change the original type for some arguments that 
maybe used elsewhere. (Also I do not understand very clear about how to convert 
in Sema)

Thanks
Xiuli



Comment at: lib/Sema/SemaChecking.cpp:294
@@ +293,3 @@
+ diag::err_opencl_builtin_pipe_invalid_access_modifier)
+  << "read_only" << Arg0->getSourceRange();
+  return true;

Anastasia wrote:
> Anastasia wrote:
> > Could we use getName() instead?
> > 
> > We could then also move this statement after the switch and just set an 
> > error flag here.
> Just to be clear getName() of the attr instead of passing the string (i.e. 
> "read_only").
Here we need a different access qualifier then we have. I seems to complicated 
to generated a useless attribute here just for a name. 


http://reviews.llvm.org/D16876



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

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-04 Thread Xiuli PAN via cfe-commits 
pxli168 updated this revision to Diff 46991.

http://reviews.llvm.org/D16876

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -5,51 +5,51 @@
   reserve_id_t rid;
 
   // read/write_pipe
-  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}}
-  read_pipe(p);   // expected-error {{invalid number of arguments to function: read_pipe}}
-  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}}
-  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}}
-  read_pipe(p, tmp);   // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}}
+  read_pipe(tmp, p);// expected-error {{first argument to 'read_pipe' must be a pipe type}}
+  read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'int *')}}
   write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
   write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
 
   // reserve_read/write_pipe
-  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}}
-  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}}
+  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function 'reserve_read_pipe' (expecting 'unsigned int')}}
+  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to 'work_group_reserve_read_pipe' must be a pipe type}}
   sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}}
 
   // commit_read/write_pipe
-  commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}}
-  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
+  commit_read_pipe(tmp, rid);// expected-error{{first argument to 'commit_read_pipe' must be a pipe type}}
+  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function 'work_group_commit_read_pipe' (expecting 'reserve_id_t')}}
   sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}}
 }
 
 void test2(write_only pipe int p, global int* ptr){
   int tmp;
   reserve_id_t rid;
 
   // read/write_pipe
-  write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}}
-  write_pipe(p);   // expected-error {{invalid number of arguments to function: write_pipe}}
-  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}}
-  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}}
-  write_pipe(p, tmp);   // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}}
+  write_pipe(tmp, p);// expected-error {{first argument to 'write_pipe' must be a pipe type}}
+  write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'int *')}}
   read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
   read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
 
   // reserve_read/write_pipe
-  reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting 'unsigned int')}}
-  work_group_reserve_write_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_write_pipe must be a pipe type}}
+  reserve_write_pipe(p, ptr);// expected-error{{invalid

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-04 Thread Xiuli PAN via cfe-commits 
pxli168 requested a review of this revision.


Comment at: lib/Sema/SemaChecking.cpp:343
@@ -332,3 +342,3 @@
   // Two kinds of read/write pipe
   // From OpenCL C Specification 6.13.16.2 the built-in read/write
   // functions have following forms.

yaxunl wrote:
> should this follow the convention? e.g.
> OpenCL v2.0 s6.13.16.2 - 
right, this will be more clear.


http://reviews.llvm.org/D16876



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

[PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-04 Thread Xiuli PAN via cfe-commits 
pxli168 created this revision.
pxli168 added reviewers: Anastasia, pekka.jaaskelainen, yaxunl.
pxli168 added a subscriber: cfe-commits.

Refine the type builtin support as the request with
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160201/148637.html

http://reviews.llvm.org/D16876

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

Index: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
===
--- test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
+++ test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl
@@ -5,51 +5,51 @@
   reserve_id_t rid;
 
   // read/write_pipe
-  read_pipe(tmp, p);// expected-error {{first argument to read_pipe must be a pipe type}}
-  read_pipe(p);   // expected-error {{invalid number of arguments to function: read_pipe}}
-  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'reserve_id_t')}}
-  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function read_pipe (expecting 'unsigned int')}}
-  read_pipe(p, tmp);   // expected-error {{invalid argument type to function read_pipe (expecting 'int *')}}
+  read_pipe(tmp, p);// expected-error {{first argument to 'read_pipe' must be a pipe type}}
+  read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'int *')}}
   write_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
   write_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting write_only)}}
 
   // reserve_read/write_pipe
-  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_read_pipe (expecting 'unsigned int')}}
-  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to work_group_reserve_read_pipe must be a pipe type}}
+  reserve_read_pipe(p, ptr);// expected-error{{invalid argument type to function 'reserve_read_pipe' (expecting 'unsigned int')}}
+  work_group_reserve_read_pipe(tmp, tmp);// expected-error{{first argument to 'work_group_reserve_read_pipe' must be a pipe type}}
   sub_group_reserve_write_pipe(p, tmp);// expected-error{{invalid pipe access modifier (expecting write_only)}}
 
   // commit_read/write_pipe
-  commit_read_pipe(tmp, rid);// expected-error{{first argument to commit_read_pipe must be a pipe type}}
-  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
+  commit_read_pipe(tmp, rid);// expected-error{{first argument to 'commit_read_pipe' must be a pipe type}}
+  work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument type to function 'work_group_commit_read_pipe' (expecting 'reserve_id_t')}}
   sub_group_commit_write_pipe(p, tmp);// expected-error{{nvalid pipe access modifier (expecting write_only)}}
 }
 
 void test2(write_only pipe int p, global int* ptr){
   int tmp;
   reserve_id_t rid;
 
   // read/write_pipe
-  write_pipe(tmp, p);// expected-error {{first argument to write_pipe must be a pipe type}}
-  write_pipe(p);   // expected-error {{invalid number of arguments to function: write_pipe}}
-  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'reserve_id_t')}}
-  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function write_pipe (expecting 'unsigned int')}}
-  write_pipe(p, tmp);   // expected-error {{invalid argument type to function write_pipe (expecting 'int *')}}
+  write_pipe(tmp, p);// expected-error {{first argument to 'write_pipe' must be a pipe type}}
+  write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'int *')}}
   read_pipe(p, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
   read_pipe(p, rid, tmp, ptr);// expected-error {{invalid pipe access modifier (expecting read_only)}}
 
   // reserve_read/write_pipe
-  reserve_write_pipe(p, ptr);// expected-error{{invalid argument type to function reserve_write_pipe (expecting

Re: [PATCH] D16876: [OpenCL] Refine pipe builtin support

2016-02-04 Thread Yaxun Liu via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM



Comment at: lib/Sema/SemaChecking.cpp:343
@@ -332,3 +342,3 @@
   // Two kinds of read/write pipe
   // From OpenCL C Specification 6.13.16.2 the built-in read/write
   // functions have following forms.

should this follow the convention? e.g.
OpenCL v2.0 s6.13.16.2 - 


http://reviews.llvm.org/D16876



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