[PATCH] Add image attribute getter builtins

Added get_image_* OpenCL builtins to the headers along with a dummy
implementation. The builtins need to be replaced by the compiler.

I’m not keeping track of the back-end, but has work been done there to support this?

Added get_image_* OpenCL builtins to the headers along with a dummy
implementation. The builtins need to be replaced by the compiler.
---
generic/include/clc/clc.h | 4 +++
generic/include/clc/image/image.h | 16 ++++++++++++
generic/lib/SOURCES | 1 +
generic/lib/image/image.cl | 54 +++++++++++++++++++++++++++++++++++++++
4 files changed, 75 insertions(+)
create mode 100644 generic/include/clc/image/image.h
create mode 100644 generic/lib/image/image.cl

diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index cab751d..5848a98 100644
--- a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -180,6 +180,10 @@
#include <clc/atomic/atomic_xchg.h>
#include <clc/atomic/atomic_xor.h>

+/* 6.11.13 Image Read and Write Functions */
+
+#include <clc/image/image.h>
+

I think this must be moved slightly. It now breaks the inclusion of the atomic headers into two parts.

/* cl_khr_global_int32_base_atomics Extension Functions */
#include <clc/cl_khr_global_int32_base_atomics/atom_add.h>
#include <clc/cl_khr_global_int32_base_atomics/atom_cmpxchg.h>
diff --git a/generic/include/clc/image/image.h b/generic/include/clc/image/image.h
new file mode 100644
index 0000000..23d2367
--- /dev/null
+++ b/generic/include/clc/image/image.h
@@ -0,0 +1,16 @@
+_CLC_OVERLOAD _CLC_DEF int get_image_width (image2d_t image);
+_CLC_OVERLOAD _CLC_DEF int get_image_width (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DEF int get_image_height (image2d_t image);
+_CLC_OVERLOAD _CLC_DEF int get_image_height (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DEF int get_image_depth (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DEF int get_image_channel_data_type (image2d_t image);
+_CLC_OVERLOAD _CLC_DEF int get_image_channel_data_type (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DEF int get_image_channel_order (image2d_t image);
+_CLC_OVERLOAD _CLC_DEF int get_image_channel_order (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DEF int2 get_image_dim (image2d_t image);
+_CLC_OVERLOAD _CLC_DEF int4 get_image_dim (image3d_t image);
diff --git a/generic/lib/SOURCES b/generic/lib/SOURCES
index 1e63994..a284747 100644
--- a/generic/lib/SOURCES
+++ b/generic/lib/SOURCES
@@ -127,3 +127,4 @@ shared/vload.cl
shared/vstore.cl
workitem/get_global_id.cl
workitem/get_global_size.cl
+image/image.cl
diff --git a/generic/lib/image/image.cl b/generic/lib/image/image.cl
new file mode 100644
index 0000000..0e89423
--- /dev/null
+++ b/generic/lib/image/image.cl
@@ -0,0 +1,54 @@
+#include <clc/clc.h>
+
+// An llvm pass will substitute these to the correct llvm register.
+// Use intrinsics to mark the places for substitution.
+int __clc_get_image_width_2d(image2d_t) __asm("llvm.opencl.image.get.width.2d");
+int __clc_get_image_width_3d(image3d_t) __asm("llvm.opencl.image.get.width.3d");
+int __clc_get_image_height_2d(image2d_t) __asm("llvm.opencl.image.get.height.2d");
+int __clc_get_image_height_3d(image3d_t) __asm("llvm.opencl.image.get.height.3d");
+int __clc_get_image_depth_3d(image3d_t) __asm("llvm.opencl.image.get.depth.3d");
+int __clc_get_image_channel_data_type_2d(image2d_t) __asm("llvm.opencl.image.get.channel_data_type.2d");
+int __clc_get_image_channel_data_type_3d(image3d_t) __asm("llvm.opencl.image.get.channel_data_type.3d");
+int __clc_get_image_channel_order_2d(image2d_t) __asm("llvm.opencl.image.get.channel_order.2d");
+int __clc_get_image_channel_order_3d(image3d_t) __asm("llvm.opencl.image.get.channel_order.3d");
+
+
+_CLC_OVERLOAD _CLC_DEF int get_image_width(image2d_t image) {
+ return __clc_get_image_width_2d(image);
+}

Why is the indirection needed in this case and many of the following cases? Why not simply define

int get_image_width(image2d_t image) __asm("llvm.opencl.image.get.width.2d”);

in clc/image/image.h? That would follow what was initially done for some of the math builtins.

Jeroen

I’m not keeping track of the back-end, but has work been done there to support this?

Yes, but it is a work in progress at the moment.

Why is the indirection needed in this case and many of the following cases? Why not simply define
int get_image_width(image2d_t image) __asm("llvm.opencl.image.get.width.2d”);
in clc/image/image.h?

Good point, I’ll move those defs to the header then.

Zoltan

What does this work look like? I don’t think adding a pseudo intrinsic that the backend handles is a good idea. It would be preferable to have target specific intrinsics implementing these functions in the library.

-Matt

What does this work look like? I don’t think adding a pseudo intrinsic that the backend handles is a good idea. It would be preferable to have target specific intrinsics implementing these functions in the library.

The intrinsic is lowered in a target-specific way (currently working on the r600 target), so you are right. I’ll fix this.

Added get_image_* OpenCL builtins to the headers along with a dummy
implementation to the R600 libs.
The builtins need to be replaced by the compiler.

Backend support for this has been submitted to llvm.

Added get_image_* OpenCL builtins to the headers along with a dummy
implementation to the R600 libs.
The builtins need to be replaced by the compiler.

Added get_image_* OpenCL builtins to the headers.
Added implementation to the r600 target.

Added get_image_* OpenCL builtins to the headers.
Added implementation to the r600 target.
---
generic/include/clc/clc.h | 4 ++++
generic/include/clc/image/image.h | 16 ++++++++++++++++
generic/lib/SOURCES | 1 +
generic/lib/image/get_image_dim.cl | 9 +++++++++
r600/lib/SOURCES | 5 +++++
r600/lib/image/attributes.h | 11 +++++++++++
r600/lib/image/get_image_channel_data_type.cl | 13 +++++++++++++
r600/lib/image/get_image_channel_order.cl | 13 +++++++++++++
r600/lib/image/get_image_depth.cl | 8 ++++++++
r600/lib/image/get_image_height.cl | 13 +++++++++++++
r600/lib/image/get_image_width.cl | 13 +++++++++++++
11 files changed, 106 insertions(+)
create mode 100644 generic/include/clc/image/image.h
create mode 100644 generic/lib/image/get_image_dim.cl
create mode 100644 r600/lib/image/attributes.h
create mode 100644 r600/lib/image/get_image_channel_data_type.cl
create mode 100644 r600/lib/image/get_image_channel_order.cl
create mode 100644 r600/lib/image/get_image_depth.cl
create mode 100644 r600/lib/image/get_image_height.cl
create mode 100644 r600/lib/image/get_image_width.cl

diff --git a/generic/include/clc/clc.h b/generic/include/clc/clc.h
index cab751d..4199842 100644
--- a/generic/include/clc/clc.h
+++ b/generic/include/clc/clc.h
@@ -210,6 +210,10 @@
#include <clc/cl_khr_local_int32_extended_atomics/atom_or.h>
#include <clc/cl_khr_local_int32_extended_atomics/atom_xor.h>

+/* 6.11.13 Image Read and Write Functions */
+
+#include <clc/image/image.h>
+
/* libclc internal defintions */
#ifdef __CLC_INTERNAL
#include <math/clc_nextafter.h>
diff --git a/generic/include/clc/image/image.h b/generic/include/clc/image/image.h
new file mode 100644
index 0000000..9c97563
--- /dev/null
+++ b/generic/include/clc/image/image.h
@@ -0,0 +1,16 @@
+_CLC_OVERLOAD _CLC_DECL int get_image_width (image2d_t image);
+_CLC_OVERLOAD _CLC_DECL int get_image_width (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DECL int get_image_height (image2d_t image);
+_CLC_OVERLOAD _CLC_DECL int get_image_height (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DECL int get_image_depth (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DECL int get_image_channel_data_type (image2d_t image);
+_CLC_OVERLOAD _CLC_DECL int get_image_channel_data_type (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DECL int get_image_channel_order (image2d_t image);
+_CLC_OVERLOAD _CLC_DECL int get_image_channel_order (image3d_t image);
+
+_CLC_OVERLOAD _CLC_DECL int2 get_image_dim (image2d_t image);
+_CLC_OVERLOAD _CLC_DECL int4 get_image_dim (image3d_t image);
diff --git a/generic/lib/SOURCES b/generic/lib/SOURCES
index 565404b..d39c19b 100644
--- a/generic/lib/SOURCES
+++ b/generic/lib/SOURCES
@@ -129,3 +129,4 @@ shared/vload.cl
shared/vstore.cl
workitem/get_global_id.cl
workitem/get_global_size.cl
+image/get_image_dim.cl
diff --git a/generic/lib/image/get_image_dim.cl b/generic/lib/image/get_image_dim.cl
new file mode 100644
index 0000000..26dbd00
--- /dev/null
+++ b/generic/lib/image/get_image_dim.cl
@@ -0,0 +1,9 @@
+#include <clc/clc.h>
+
+_CLC_OVERLOAD _CLC_DEF int2 get_image_dim (image2d_t image) {
+ return (int2)(get_image_width(image), get_image_height(image));
+}
+_CLC_OVERLOAD _CLC_DEF int4 get_image_dim (image3d_t image) {
+ return (int4)(get_image_width(image), get_image_height(image),
+ get_image_depth(image), 0);
+}
diff --git a/r600/lib/SOURCES b/r600/lib/SOURCES
index 5cdb14a..afb7c07 100644
--- a/r600/lib/SOURCES
+++ b/r600/lib/SOURCES
@@ -10,3 +10,8 @@ workitem/get_global_size.ll
workitem/get_work_dim.ll
synchronization/barrier.cl
synchronization/barrier_impl.ll
+image/get_image_width.cl
+image/get_image_height.cl
+image/get_image_depth.cl
+image/get_image_channel_data_type.cl
+image/get_image_channel_order.cl
diff --git a/r600/lib/image/attributes.h b/r600/lib/image/attributes.h
new file mode 100644
index 0000000..3f34f77
--- /dev/null
+++ b/r600/lib/image/attributes.h
@@ -0,0 +1,11 @@
+enum {
+ IMAGE_WIDTH = 0,
+ IMAGE_HEIGHT = 1,
+ IMAGE_DEPTH = 2,
+ IMAGE_CHANNEL_DATA_TYPE = 3,
+ IMAGE_CHANNEL_ORDER = 4,
+};
+
+int __clc_read_image_attribute(int img_id, int attr_id) __asm("llvm.r600.read.image.attribute");
+int __clc_get_image_id_2d(image2d_t) __asm("llvm.AMDGPU.get.image.id.2d");
+int __clc_get_image_id_3d(image3d_t) __asm("llvm.AMDGPU.get.image.id.3d");

You should replace these with calls to __builtin_amdgpu_* calls.

Added get_image_* OpenCL builtins to the headers.
Added implementation to the r600 target.

Ping.