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 @@ +/** + * Round to integral value using the round to +ve + * infinity rounding mode. ---------------- What does +ve mean? ================ Comment at: lib/Headers/opencl-c.h:7795 @@ +7794,3 @@ + * Returns x with its sign changed to match the sign of + * y. + */ ---------------- Could y be merged with the previous line? ================ Comment at: lib/Headers/opencl-c.h:7952 @@ +7951,3 @@ +/** + * Compute the base- e exponential of x. + */ ---------------- Can't read this comment. Perhaps: "Compute base e exponent"? ================ Comment at: lib/Headers/opencl-c.h:8306 @@ +8305,3 @@ +#else +float __attribute__((overloadable)) fract(float x, __global float *iptr); +float2 __attribute__((overloadable)) fract(float2 x, __global float2 *iptr); ---------------- Does this mean that all non-generic versions are not available in CL2.0 forcing the dynamic AS conversion for all BIFs with a pointer type? Wondering if adding those unconditionally would be better thing to do... ================ Comment at: lib/Headers/opencl-c.h:8457 @@ +8456,3 @@ +/** + * Compute the value of the square root of x^2+ y^2 + * without undue overflow or underflow. ---------------- Could you add space please: x^2 + y^2 ================ 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); ---------------- Overloadable here and in other places too? ================ Comment at: lib/Headers/opencl-c.h:12772 @@ +12771,3 @@ +#ifdef cl_khr_fp16 +half __attribute__((overloadable)) vload(size_t offset, const half *p); +half2 __attribute__((overloadable)) vload2(size_t offset, const half *p); ---------------- Should it be vload_half instead? ================ Comment at: lib/Headers/opencl-c.h:14484 @@ +14483,3 @@ +long __attribute__((overloadable)) atom_sub(volatile __global long *p, long val); +unsigned long __attribute__((overloadable)) atom_sub(volatile __global unsigned long *p, unsigned long val); +long __attribute__((overloadable)) atom_sub(volatile __local long *p, long val); ---------------- btw, perhaps it's a good idea to have a macro for __attribute__((overloadable)). This should be relatively simple to replace everywhere. ================ Comment at: lib/Headers/opencl-c.h:14766 @@ +14765,3 @@ + +ATOMIC_INIT_PROTOTYPE(int) +ATOMIC_INIT_PROTOTYPE(uint) ---------------- Let's not have macros for function declarations. As commented earlier they can break proper diagnostics reporting (ie. diagnostics will display macro instead of actual function). ================ Comment at: lib/Headers/opencl-c.h:15674 @@ +15673,3 @@ +/** + * Use the coordinate (coord.z) to index into the + * 2D image array object and (coord.x, coord.y) to do an ---------------- I feel that some bits of this description are being repeated multiple times. Could we have a common description at the beginning instead that would cover all the different cases. ================ Comment at: lib/Headers/opencl-c.h:16220 @@ +16219,3 @@ +/** + * Write color value to location specified by coordinate + * (coord.x) in the 1D image object specified by index ---------------- The same here. Could we unify the description for all write_image functions somehow? ================ Comment at: lib/Headers/opencl-c.h:16960 @@ +16959,3 @@ + +#define WG_BROADCAST_1D_DECL(type) \ +type __attribute__((overloadable)) work_group_broadcast(type a, size_t local_id); ---------------- Let's remove macros from here too. ================ Comment at: lib/Headers/opencl-c.h:17051 @@ +17050,3 @@ +#define CLK_SUCCESS 0 +#define CLK_ENQUEUE_FAILURE -101 +#define CLK_INVALID_QUEUE -102 ---------------- Are those arbitrary taken values I am guessing? ================ Comment at: lib/Headers/opencl-c.h:17099 @@ +17098,3 @@ + +// ToDo: Add enqueue_kernel as a Clang builtin because it requires custom check of type of variadic +// arguments as well as block arguments. ---------------- Could we remove the enqueue_kernel as it's being added as Clang builtin? It's prototype is incorrect here anyways. Btw, I have started the review for it: http://reviews.llvm.org/D20249 ================ Comment at: lib/Headers/opencl-c.h:17252 @@ +17251,3 @@ +/** + * Use the coordinate (x) to do an element lookup in + * the mip-level specified by the Level-of-Detail (lod) ---------------- I think this should be better grouped with other image functions above. http://reviews.llvm.org/D18369 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
