[Mesa-dev] [PATCH 2/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D'

[Francisco Jerez]

Edward O'Callaghan <funfunctor at folklore1984.net> writes:

> Hi Francisco, thanks for the review..
>
> On 11/25/2016 03:39 PM, Francisco Jerez wrote:
>> Edward O'Callaghan <funfunctor at folklore1984.net> writes:
>> 
>>> Signed-off-by: Edward O'Callaghan <funfunctor at folklore1984.net>
>>> ---
>>>  src/gallium/state_trackers/clover/api/memory.cpp  |  9 ++++++++-
>>>  src/gallium/state_trackers/clover/core/memory.cpp | 13 +++++++++++++
>>>  src/gallium/state_trackers/clover/core/memory.hpp | 10 ++++++++++
>>>  3 files changed, 31 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/state_trackers/clover/api/memory.cpp b/src/gallium/state_trackers/clover/api/memory.cpp
>>> index 58f56d1..41e344e 100644
>>> --- a/src/gallium/state_trackers/clover/api/memory.cpp
>>> +++ b/src/gallium/state_trackers/clover/api/memory.cpp
>>> @@ -166,6 +166,14 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
>>>     ret_error(r_errcode, CL_SUCCESS);
>>>  
>>>     switch (desc->image_type) {
>>> +   case CL_MEM_OBJECT_IMAGE1D:
>>> +      if (!desc->image_width)
>>> +         throw error(CL_INVALID_IMAGE_SIZE);
>>> +
>> 
>> We probably need to check that the specified image width is within the
>> device limits -- There's no device::max_image_levels_1d() query but
>> max_image_levels_2d() should work as well.
>> 
>>> +      return new image1d(ctx, flags, format,
>>> +                         desc->image_width,
>>> +                         desc->image_row_pitch, host_ptr);
>>> +
>>>     case CL_MEM_OBJECT_IMAGE2D:
>>>        if (!desc->image_width || !desc->image_height)
>>>           throw error(CL_INVALID_IMAGE_SIZE);
>>> @@ -214,7 +222,6 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
>>>                           desc->image_depth, desc->image_row_pitch,
>>>                           desc->image_slice_pitch, host_ptr);
>>>  
>>> -   case CL_MEM_OBJECT_IMAGE1D:
>>>     case CL_MEM_OBJECT_IMAGE1D_ARRAY:
>>>     case CL_MEM_OBJECT_IMAGE1D_BUFFER:
>>>        // XXX - Not implemented.
>>> diff --git a/src/gallium/state_trackers/clover/core/memory.cpp b/src/gallium/state_trackers/clover/core/memory.cpp
>>> index de1862b..246bd2d 100644
>>> --- a/src/gallium/state_trackers/clover/core/memory.cpp
>>> +++ b/src/gallium/state_trackers/clover/core/memory.cpp
>>> @@ -185,6 +185,19 @@ image::slice_pitch() const {
>>>     return _slice_pitch;
>>>  }
>>>  
>>> +image1d::image1d(clover::context &ctx, cl_mem_flags flags,
>>> +                 const cl_image_format *format,
>>> +                 size_t width, size_t row_pitch,
>>> +                 void *host_ptr) :
>>> +   image(ctx, flags, format, width, 0, 1,
>> 
>> All surface dimension fields (width, height, depth) of a clover::image
>> object are considered valid regardless of the image type, so we should
>> set unused dimensions to 1 in order to avoid unexpected results while
>> doing arithmetic or various error checking with them.
>> 
>>> +         row_pitch, 0, row_pitch, host_ptr) {
>> 
>> I don't think you can rely on the row pitch to be meaningful for 1D
>> images, it's probably necessary to pass it as argument to the
>> constructor, we're probably better off calculating the size by hand.
> Why not and what do you propose here exactly?
>

Because technically 1D images are a single row, so the row pitch is kind
of meaningless...  How about
'util_format_get_blocksize(translate_format(format)) * width'?

>> 
>>> +}
>>> +
>>> +cl_mem_object_type
>>> +image1d::type() const {
>>> +   return CL_MEM_OBJECT_IMAGE1D;
>>> +}
>>> +
>>>  image2d::image2d(clover::context &ctx, cl_mem_flags flags,
>>>                   const cl_image_format *format, size_t width,
>>>                   size_t height, size_t row_pitch,
>>> diff --git a/src/gallium/state_trackers/clover/core/memory.hpp b/src/gallium/state_trackers/clover/core/memory.hpp
>>> index 1a3e8c9..ad9052b 100644
>>> --- a/src/gallium/state_trackers/clover/core/memory.hpp
>>> +++ b/src/gallium/state_trackers/clover/core/memory.hpp
>>> @@ -134,6 +134,16 @@ namespace clover {
>>>                 std::unique_ptr<root_resource>> resources;
>>>     };
>>>  
>>> +   class image1d : public image {
>>> +   public:
>>> +      image1d(clover::context &ctx, cl_mem_flags flags,
>>> +              const cl_image_format *format,
>>> +              size_t width, size_t row_pitch,
>>> +              void *host_ptr);
>>> +
>>> +      virtual cl_mem_object_type type() const;
>>> +   };
>>> +
>>>     class image2d : public image {
>>>     public:
>>>        image2d(clover::context &ctx, cl_mem_flags flags,
>>> -- 
>>> 2.7.4
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161211/ac836c88/attachment.sig>