Anastasia added a comment.
Since it has been in review for quite a while, should we try to commit it ASAP?
I think Richard can give us his feedback later as well.
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
Anastasia accepted this revision.
Anastasia added a comment.
LGTM! Thanks!
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl added inline comments.
Comment at: lib/Headers/opencl-c.h:143
@@ +142,3 @@
+#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#define NULL ((void*)0)
+#endif
Anastasia wrote:
> indentation seems wrong!
fixed.
Comment
Anastasia added a comment.
Sam, there are a few small comments, otherwise it seems to be in a good shape.
@rsmith, do you think you could take a look here? The OpenCL side is fine, but
I was just wondering if you see any issue with us adding a header of ~17K
lines. It is all part of OpenCL stan
pxli168 accepted this revision.
pxli168 added a comment.
LGTM!
I think we may add optimization on this base later.
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
Anastasia added a comment.
Looking good already! I just have one general concern about committing 16K
lines of code with no tests. But perhaps testing all the declarations isn't an
option actually.
Could we at least have a test that includes the header and makes sure it's
compiled successfully
yaxunl added inline comments.
Comment at: lib/Headers/opencl-c.h:17051
@@ +17050,3 @@
+#define CLK_SUCCESS 0
+#define CLK_ENQUEUE_FAILURE -101
+#define CLK_INVALID_QUEUE -102
jprice
jprice added inline comments.
Comment at: lib/Headers/opencl-c.h:17051
@@ +17050,3 @@
+#define CLK_SUCCESS 0
+#define CLK_ENQUEUE_FAILURE -101
+#define CLK_INVALID_QUEUE -102
yaxunl
yaxunl added inline comments.
Comment at: lib/Headers/opencl-c.h:17051
@@ +17050,3 @@
+#define CLK_SUCCESS 0
+#define CLK_ENQUEUE_FAILURE -101
+#define CLK_INVALID_QUEUE -102
Anastas
pxli168 added a comment.
Comment at: lib/Headers/opencl-c.h:9110
@@ +9109,3 @@
+ */
+float __const_func remainder(float x, float y);
+float2 __const_func remainder(float2 x, float2 y);
Anastasia wrote:
> Overloadable here and in other places too?
It seems macro
Anastasia added inline comments.
Comment at: lib/Headers/opencl-c.h:14057
@@ +14056,3 @@
+event_t __attribute__((overloadable)) async_work_group_copy(__local float2
*dst, const __global float2 *src, size_t num_elements, event_t event);
+event_t __attribute__((overloadable)) async
Anastasia added inline comments.
Comment at: lib/Headers/opencl-c.h:7452
@@ +7451,3 @@
+
+// OpenCL v1.2 s6.12.2, v2.0 s6.13.2 - Math functions
+
Could you put OpenCL v1.1 section too?
Comment at: lib/Headers/opencl-c.h:7767
@@ +7766,3 @@
+/**
+
yaxunl marked 3 inline comments as done.
Comment at: lib/Headers/opencl-c.h:14057
@@ +14056,3 @@
+event_t __attribute__((overloadable)) async_work_group_copy(__local float2
*dst, const __global float2 *src, size_t num_elements, event_t event);
+event_t __attribute__((overloadable
Anastasia added a comment.
In http://reviews.llvm.org/D18369#424617, @yaxunl wrote:
> In http://reviews.llvm.org/D18369#424347, @pxli168 wrote:
>
> > In http://reviews.llvm.org/D18369#422367, @yaxunl wrote:
> >
> > > In http://reviews.llvm.org/D18369#421963, @pxli168 wrote:
> > >
> > > > If we wa
yaxunl added a comment.
In http://reviews.llvm.org/D18369#424347, @pxli168 wrote:
> In http://reviews.llvm.org/D18369#422367, @yaxunl wrote:
>
> > In http://reviews.llvm.org/D18369#421963, @pxli168 wrote:
> >
> > > If we want to save some space, could we use some macro to expand the
> > > gentyp
pxli168 added a comment.
In http://reviews.llvm.org/D18369#422367, @yaxunl wrote:
> In http://reviews.llvm.org/D18369#421963, @pxli168 wrote:
>
> > If we want to save some space, could we use some macro to expand the
> > gentype or some script to expand the gentype into each types when the clang
yaxunl added inline comments.
Comment at: lib/Headers/opencl-c.h:14057
@@ +14056,3 @@
+event_t __attribute__((overloadable)) async_work_group_copy(__local float2
*dst, const __global float2 *src, size_t num_elements, event_t event);
+event_t __attribute__((overloadable)) async_wo
yaxunl added a comment.
In http://reviews.llvm.org/D18369#421963, @pxli168 wrote:
> If we want to save some space, could we use some macro to expand the gentype
> or some script to expand the gentype into each types when the clang is build?
We need to balance between space and readability. Whe
pxli168 added a comment.
If we want to save some space, could we use some macro to expand the gentype or
some script to expand the gentype into each types when the clang is build?
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-comm
yaxunl added a comment.
typo. saved 300KB space.
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
yaxunl added inline comments.
Comment at: lib/Headers/opencl-c.h:4872
@@ +4871,3 @@
+
+#ifdef cl_khr_fp64
+char __const_func __attribute__((overloadable)) convert_char(double);
Anastasia wrote:
> Interesting, macro has the same name as an extension?
The spec requi
Anastasia added inline comments.
Comment at: lib/Headers/opencl-c.h:14
@@ +13,3 @@
+#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#define _CL20_AND_ABOVE 1
+#endif
Is there any reason to define this extra macro?
Could we not just check __OPENCL_C_VERSION__ >= CL_VE
yaxunl marked 12 inline comments as done.
yaxunl added a comment.
Changed file name to opencl-c.h and updated comments.
Addressed Anastasia's early comments.
I am thinking we may need several more patches to address remaining issues to
keep the changes manageable.
Comment at:
bader added a comment.
In http://reviews.llvm.org/D18369#409011, @yaxunl wrote:
> In http://reviews.llvm.org/D18369#408773, @bader wrote:
>
> > BTW, there is a comment on GitHub that opencl.h might be a bad name for
> > that header, since Khronos group provides the header with the same name,
>
yaxunl added a comment.
In http://reviews.llvm.org/D18369#408773, @bader wrote:
> BTW, there is a comment on GitHub that opencl.h might be a bad name for that
> header, since Khronos group provides the header with the same name, but it
> defines host API. So if some developer is using clang to
bader added inline comments.
Comment at: lib/Headers/opencl.h:2
@@ +1,3 @@
+//
+// SPIR Tools
+//
This comment should be updated.
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-c
bader added a comment.
BTW, there is a comment on GitHub that opencl.h might be a bad name for that
header, since Khronos group provides the header with the same name, but it
defines host API. So if some developer is using clang to compile OpenCL
application it might accidentally include opencl
bader added inline comments.
Comment at: lib/Headers/opencl.h:4870
@@ +4869,3 @@
+
+#ifdef cl_khr_fp64
+char __const_func __attribute__((overloadable)) convert_char(double);
Sam, could you confirm that this macro id implicitly defined for OpenCL
versions 1.2+?
bader accepted this revision.
bader added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks!
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo
bader added inline comments.
Comment at: lib/Headers/opencl.h:14476-14479
@@ +14475,6 @@
+
+#ifdef _HAS_READ_WRITE_IMAGE
+float4 __attribute__((overloadable)) read_imagef(read_write image2d_t image,
sampler_t sampler, int2 coord);
+float4 __attribute__((overloadable)) read_imagef
bader added inline comments.
Comment at: lib/Headers/opencl.h:14473-14474
@@ +14472,4 @@
+
+float4 __const_func __attribute__((overloadable)) read_imagef(read_only
image2d_t image, sampler_t sampler, int2 coord);
+float4 __const_func __attribute__((overloadable)) read_imagef(read
bader added a comment.
Sam, could you also add declarations of samplerless read_image and write_image
built-in functions with read_write qualified images, please?
http://reviews.llvm.org/D18369
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
bader added a comment.
To clarify my last comment: I don't think we should define Image Query built-in
functions only for 'read_only' access qualifier in OpenCL 1.x.
It's just bug in OpenCL 1.x specifications. I think the intention was to
provide these built-in functions for both 'read_only' and
bader added inline comments.
Comment at: lib/Headers/opencl.h:15372-15373
@@ +15371,4 @@
+ */
+int __const_func __attribute__((overloadable)) get_image_width(image1d_t
image);
+int __const_func __attribute__((overloadable))
get_image_width(image1d_buffer_t image);
+int __const_f
bader added inline comments.
Comment at: lib/Headers/opencl.h:14892-14898
@@ +14891,9 @@
+
+/**
+* Sampler-less Image Access
+*/
+
+float4 __const_func __attribute__((overloadable)) read_imagef(read_only
image1d_t image, int coord);
+int4 __const_func __attribute__((overloadable)
35 matches
Mail list logo
