Re: [Mesa-dev] [PATCH 2/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D'
Edward O'Callaghanwrites: > 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'
Hi Francisco, thanks for the review.. On 11/25/2016 03:39 PM, Francisco Jerez wrote: > Edward O'Callaghanwrites: > >> 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'
Edward O'Callaghanwrites: > 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'
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