[PATCH] D18369: [OpenCL] Upstreaming khronos OpenCL 1.2/2.0 header files.

Yaxun Liu via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 14 12:41:20 PDT 2016


yaxunl marked 4 inline comments as done.

================
Comment at: lib/Headers/opencl.h:13721-13726
@@ +13720,8 @@
+
+/**
+ * Queue a memory fence to ensure correct ordering of memory
+ * operations between work-items of a work-group to
+ * image memory.
+ */
+#define CLK_IMAGE_MEM_FENCE  0x04
+
----------------
pxli168 wrote:
> Move this to the barrier part with worg_group_barrier maybe better.
Will do.

================
Comment at: lib/Headers/opencl.h:15636-15637
@@ +15635,4 @@
+#if defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 200
+#define PIPE_RESERVE_ID_VALID_BIT (1U << 30)
+#define CLK_NULL_RESERVE_ID (__builtin_astype(((void*)(~PIPE_RESERVE_ID_VALID_BIT)), reserve_id_t))
+bool __attribute__((overloadable)) is_valid_reserve_id(reserve_id_t reserve_id);
----------------
pxli168 wrote:
> Is this macro needed in this header?
> And what happens to spir32 and spir64 difference?
The spec requires to define this macro.

I agree this definition seems arbitrary since the spec does not define PIPE_RESERVE_ID_VALID_BIT.

How about

  // Define an internally used macro for the maximum value of size_t.
  #if defined(__SPIR32__)
  #define _SIZET_MAX UINT_MAX
  #elif defined(__SPIR64__ )
  #define _SIZET_MAX ULONG_MAX
  #endif

and use it for defining CLK_NULL_RESERVE_ID.

================
Comment at: lib/Headers/opencl.h:15661
@@ +15660,3 @@
+#define CLK_NULL_QUEUE                              0
+#define CLK_NULL_EVENT (__builtin_astype(((void*)(UINT32MAX)), clk_event_t))
+
----------------
pxli168 wrote:
> What is this UINT32MAX?
> And what about spir64?
How about replacing UINT32MAX with _SIZET_MAX defined above?


================
Comment at: lib/Headers/opencl.h:15675
@@ +15674,3 @@
+#define MAX_WORK_DIM        3
+#if defined(__clang__) && (__clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 9))
+typedef struct {
----------------
pxli168 wrote:
> Is this part necessary for up-streaming to the llvm3.9? It will never be hint.
I will remove it.


http://reviews.llvm.org/D18369





More information about the cfe-commits mailing list