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

2016-03-03 Thread Xiuli PAN via cfe-commits
pxli168 updated this revision to Diff 49797. 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 ==

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/DiagnosticSemaKi

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

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

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 t

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 bui

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 functio

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

2016-02-05 Thread Anastasia Stulova via cfe-commits
Anastasia added inline comments. 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: > Could we use ge

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

2016-02-05 Thread Anastasia Stulova via cfe-commits
Anastasia added a comment. I think some comments from Richad's feedback are missing, specifically: - Missing space after comma. - Why are these conversions performed here rather than in Sema? - You should attempt to implicitly convert to the desired type here, rather than demanding the right typ

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

2016-02-05 Thread Yaxun Liu via cfe-commits
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks. http://reviews.llvm.org/D16876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

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: >

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 ===

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 //

[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