subject:"\[Mesa\-dev\] \[PATCH 2\/4\] clover\: Implement 'CL_MEM_OBJECT

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

2016-12-11 Thread Francisco Jerez

Edward O'Callaghan  writes:

> Hi Francisco, thanks for the review..
>
> On 11/25/2016 03:39 PM, Francisco Jerez wrote:
>> Edward O'Callaghan  writes:
>> 
>>> Signed-off-by: Edward O'Callaghan 
>>> ---
>>>  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 , 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 , 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> resources;
>>> };
>>>  
>>> +   class image1d : public image {
>>> +   public:
>>> +  image1d(clover::context , 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 , cl_mem_flags flags,
>>> -- 
>>> 2.7.4
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-25 Thread Edward O'Callaghan

Hi Francisco, thanks for the review..

On 11/25/2016 03:39 PM, Francisco Jerez wrote:
> Edward O'Callaghan  writes:
> 
>> Signed-off-by: Edward O'Callaghan 
>> ---
>>  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 , 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?

> 
>> +}
>> +
>> +cl_mem_object_type
>> +image1d::type() const {
>> +   return CL_MEM_OBJECT_IMAGE1D;
>> +}
>> +
>>  image2d::image2d(clover::context , 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> resources;
>> };
>>  
>> +   class image1d : public image {
>> +   public:
>> +  image1d(clover::context , 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 , cl_mem_flags flags,
>> -- 
>> 2.7.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-24 Thread Francisco Jerez

Edward O'Callaghan  writes:

> Signed-off-by: Edward O'Callaghan 
> ---
>  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 , 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.

> +}
> +
> +cl_mem_object_type
> +image1d::type() const {
> +   return CL_MEM_OBJECT_IMAGE1D;
> +}
> +
>  image2d::image2d(clover::context , 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> resources;
> };
>  
> +   class image1d : public image {
> +   public:
> +  image1d(clover::context , 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 , cl_mem_flags flags,
> -- 
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-21 Thread Edward O'Callaghan

Signed-off-by: Edward O'Callaghan 
---
 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);
+
+  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 , 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,
+ row_pitch, 0, row_pitch, host_ptr) {
+}
+
+cl_mem_object_type
+image1d::type() const {
+   return CL_MEM_OBJECT_IMAGE1D;
+}
+
 image2d::image2d(clover::context , 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> resources;
};
 
+   class image1d : public image {
+   public:
+  image1d(clover::context , 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 , cl_mem_flags flags,
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information