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.

Sign up for GitHub

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

Jump to bottom

AllocatorT template parameter is not used in constructors of sampled_image class (unlike unsampled_image class) #119

Open
tizmajlo opened this issue Mar 11, 2021 · 3 comments · May be fixed by #722
Open

AllocatorT template parameter is not used in constructors of sampled_image class (unlike unsampled_image class) #119

tizmajlo opened this issue Mar 11, 2021 · 3 comments · May be fixed by #722
Labels
bug Something isn't working

Comments

@tizmajlo
Copy link

Compare the current interfaces of sampled_image and unsampled_image classes in SYCL 2020 rev 3 specification:

SYCL-Docs/adoc/headers/sampledImage.h

Lines 19 to 40 in 3aa000f

template <int dimensions = 1, typename AllocatorT = sycl::image_allocator>
class sampled_image {
public:
sampled_image(const void *hostPointer, image_format format,
image_sampler sampler, const range<dimensions> &rangeRef,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
sampled_image(const void *hostPointer, image_format format,
image_sampler sampler, const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch,
const property_list &propList = {});
sampled_image(std::shared_ptr<const void> &hostPointer, image_format format,
image_sampler sampler, const range<dimensions> &rangeRef,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
sampled_image(std::shared_ptr<const void> &hostPointer, image_format format,
image_sampler sampler, const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch,
const property_list &propList = {});

SYCL-Docs/adoc/headers/unsampledImage.h

Lines 19 to 76 in 3aa000f

template <int dimensions = 1, typename AllocatorT = sycl::image_allocator>
class unsampled_image {
public:
unsampled_image(image_format format, const range<dimensions> &rangeRef,
const property_list &propList = {});
unsampled_image(image_format format, const range<dimensions> &rangeRef,
AllocatorT allocator, const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(image_format format, const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(image_format format, const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch, AllocatorT allocator,
const property_list &propList = {});
unsampled_image(void *hostPointer, image_format format,
const range<dimensions> &rangeRef,
const property_list &propList = {});
unsampled_image(void *hostPointer, image_format format,
const range<dimensions> &rangeRef, AllocatorT allocator,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(void *hostPointer, image_format format,
const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(void *hostPointer, image_format format,
const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch, AllocatorT allocator,
const property_list &propList = {});
unsampled_image(std::shared_ptr<void> &hostPointer, image_format format,
const range<dimensions> &rangeRef,
const property_list &propList = {});
unsampled_image(std::shared_ptr<void> &hostPointer, image_format format,
const range<dimensions> &rangeRef, AllocatorT allocator,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(std::shared_ptr<void> &hostPointer, image_format format,
const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch,
const property_list &propList = {});
/* Available only when: dimensions > 1 */
unsampled_image(std::shared_ptr<void> &hostPointer, image_format format,
const range<dimensions> &rangeRef,
const range<dimensions -1> &pitch, AllocatorT allocator,
const property_list &propList = {});

Both of them have AllocatorT template type parameter that is a type of an allocator used by the corresponding class. But no allocators of the given type can be passed to the constructors of an object of sampled_image class (unlike unsampled_image class). It looks like a bug in the specification.
@gmlueck
Copy link
Contributor

gmlueck commented Mar 18, 2021

Comparing unsampled_image to sampled_image, I also see that sampled_image is missing the get_allocator() member function. Looking at the git history, it seems like these omissions can be traced back to the original MR that added the sampled and unsampled image classes (https://gitlab.khronos.org/sycl/Specification/-/merge_requests/398).

I presume it was our intent to allow sampled_image to have a custom allocator. Does anyone involved with the original sampled / unsampled image design know of a reason why we would not want that?

@ProGTX
Copy link
Contributor

ProGTX commented Apr 7, 2021

The intent was to still allow a custom allocator, yes, this seems to simply have been overlooked.

@fraggamuffin
Copy link

yes, missing parts should be added.

@Pennycook Pennycook linked a pull request Feb 13, 2025 that will close this issue
Open
@Pennycook Pennycook added the bug Something isn't working label Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add allocator functions to sampled_image Pennycook/SYCL-Docs
5 participants
@Pennycook @gmlueck @ProGTX @fraggamuffin @tizmajlo