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

[Xiuli PAN via cfe-commits]

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 __const_func has overloadable attribute.

> #define __const_func __attribute__((const, overloadable))


================
Comment at: lib/Headers/opencl-c.h:13534
@@ +13533,3 @@
+/**
+ * For n = 1, 2, 4, 8 and 16 read sizeof (halfn)
+ * bytes of data from address (p + (offset * n)).
----------------
I have seen a lot of comments with such few words in one line. It seems very strange here.

================
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);
----------------
Anastasia wrote:
> btw, perhaps it's a good idea to have a macro for __attribute__((overloadable)). This should be relatively simple to replace everywhere.
Agree, it seems a lot of functions need overloable attribute.

================
Comment at: lib/Headers/opencl-c.h:14766
@@ +14765,3 @@
+
+ATOMIC_INIT_PROTOTYPE(int)
+ATOMIC_INIT_PROTOTYPE(uint)
----------------
Anastasia wrote:
> 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).
So maybe we could use see script to generate the header when build? It will save space and avoid macro break readability and  diagnostics reporting. The clang builtin functions are defined in some .def files, or maybe we could try something like that?
This can be done after we have a stable version of what is need for header, we may just decide what should be in this header and don't focus on the performance. 


http://reviews.llvm.org/D18369