Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate cl_ext_image_from_buffer into unified specification #1299

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kpet
Copy link
Contributor

@kpet kpet commented Jan 15, 2025

Also add version notes for all CL_IMAGE_REQUIREMENTS_* enums.

Change-Id: I7b7b093034121a9215786beff7318b18e7d0c24a

Also add version notes for all CL_IMAGE_REQUIREMENTS_* enums.

Signed-off-by: Kevin Petit <[email protected]>
Change-Id: I7b7b093034121a9215786beff7318b18e7d0c24a
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few comments.

I'd be OK merging this as-is, but we really ought to fix the formatting issue first.

buffer.
* {CL_INVALID_IMAGE_SIZE} if the {cl_ext_image_from_buffer_EXT} extension is
supported and an image is created from a buffer and the buffer passed in
_image_desc->_mem_object_ is too small to be used as a data store for the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting nitpick: I think this should be:

Suggested change
_image_desc->_mem_object_ is too small to be used as a data store for the
__image_desc__->__mem_object__ is too small to be used as a data store for the

@@ -53,7 +53,7 @@ Note that reading and writing 2D image arrays from a kernel with `image_array_si
]

:fn-image-from-buffer: pass:n[ \
To create a 2D image from a buffer object that share the data store between the image and buffer object. \
To create an image from a buffer object that share the data store between the image and buffer object. \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional / editorial: this wasn't changed in this PR, but I don't this sentence is grammatically correct. How about:

Suggested change
To create an image from a buffer object that share the data store between the image and buffer object. \
To create an image from a buffer object that shares the data store between the image and buffer object. \

Comment on lines +40 to +46
. TODO Test access via read/write/map commands?

. TODO Test copy to/from buffer?

. TODO Test fill?

. TODO Test copy to/from another image?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recording a comment from the January 21st teleconference: do we want to do anything with these TODOs, or are they ok as-is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants