[PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL header file.

Anastasia Stulova via cfe-commits cfe-commits at lists.llvm.org
Tue May 24 10:09:39 PDT 2016


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 standard libraries though and would be very helpful to have it upstream for all of us. Any suggestion on testing would be useful too as it's a bit simplified for now due to its size.


================
Comment at: lib/Headers/opencl-c.h:19
@@ +18,3 @@
+
+#define _ovld __attribute__((overloadable))
+
----------------
Could we change to ovld and cnfn with double underscores please?

================
Comment at: lib/Headers/opencl-c.h:143
@@ +142,3 @@
+#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#define NULL                              ((void*)0)
+#endif
----------------
indentation seems wrong!

================
Comment at: lib/Headers/opencl-c.h:175
@@ +174,3 @@
+ */
+#define NAN as_float(INT_MAX)
+
----------------
Is this a part of spec? I don't see it in s6.13.2.1.

================
Comment at: lib/Headers/opencl-c.h:7953
@@ +7952,3 @@
+/**
+ * Compute the base e exponential of x.
+ */
----------------
Ping!

================
Comment at: lib/Headers/opencl-c.h:8307
@@ +8306,3 @@
+#else
+float _ovld fract(float x, __global float *iptr);
+float2 _ovld fract(float2 x, __global float2 *iptr);
----------------
Any problem with doing this unconditionally?

================
Comment at: lib/Headers/opencl-c.h:8458
@@ +8457,3 @@
+/**
+ * Compute the value of the square root of x^2+ y^2
+ * without undue overflow or underflow.
----------------
Ping!

================
Comment at: lib/Headers/opencl-c.h:12646
@@ +12645,3 @@
+/**
+ * Return sizeof (gentypen) bytes of data read
+ * from address (p + (offset * n)). The address
----------------
This description is missing explaining what gentype is! The same happens in other places too.

================
Comment at: lib/Headers/opencl-c.h:12773
@@ +12772,3 @@
+#ifdef cl_khr_fp16
+half _ovld vload(size_t offset, const half *p);
+half2 _ovld vload2(size_t offset, const half *p);
----------------
Ping!

================
Comment at: lib/Headers/opencl-c.h:14004
@@ +14003,3 @@
+
+// Builtin functions to_global, to_local, and to_private need to be declared as Clang builtin functions
+// and checked in Sema since they should be declared as
----------------
Formatting is wrong!


http://reviews.llvm.org/D18369





More information about the cfe-commits mailing list