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

Questions about type aliases #335

Open
AlexeySachkov opened this issue Dec 29, 2022 · 3 comments
Open

Questions about type aliases #335

AlexeySachkov opened this issue Dec 29, 2022 · 3 comments
Assignees
Labels
Agenda To be discussed during a SYCL committee meeting

Comments

@AlexeySachkov
Copy link
Contributor

Hi folks,

I've been looking into intel/llvm SYCL implementation details after vector_aliases SYCL-CTS update and whilst looking into the SYCL spec I got a few questions about whether we want to add a few more type aliases into it or about how of the existing aliases are defined.

No more aliases for OpenCL vector types?

In SYCL 1.2.1 we have aliases like cl_int2 -> cl::sycl::vec<cl::sycl::cl_int, 2>. However, in SYCL 2020 we only have sycl::opencl::cl_int, but not sycl::opencl::cl_int2. I was wondering: is there a particular reason for not including them?

Potentially confusing alias long{n}

4.14.2.2. Aliases defines aliases long{n} and ulong{n}, which correspond to sycl::vec<int64_t, n> and sycl::vec<uint64_t, n>. However, long data type in C++ is not guaranteed to be 64 bits.

To me, this looks a bit weird to have sycl::vec<long, 2> and sycl::long2 which are potentially different types. Someone who is not familiar with exact details of vector aliases may think that they are the same, but that may not always be true. I suppose that the idea could be to better match OpenCL, but isn't it better to align with C++ instead and add OpenCL-related aliases specific to the corresponding backend?

Missing longlong{n} alias?

Also, there is no alias for long long type. For someone, who comes from C++ background, where only long long is guaranteed to be at least 64 bits, I think it maybe useful to have such an alias. Note that we had such an alias in SYCL 1.2.1 spec, but it didn't make it to SYCL 2020 by some reason.

AlexeySachkov added a commit to AlexeySachkov/llvm that referenced this issue Jan 5, 2023
SYCL-CTS are still using some aliases which were removed from SYCL 2020
specification, see KhronosGroup/SYCL-CTS#446. Those tests are part of
our post-commit and to make it green, this commit temporary adds those
aliases back.

Note that it is not entierly clear if those aliases should have been
removed in the first place, see KhronosGroup/SYCL-Docs#335.
bader pushed a commit to intel/llvm that referenced this issue Jan 6, 2023
SYCL-CTS are still using some aliases which were removed from SYCL 2020
specification, see KhronosGroup/SYCL-CTS#446. Those tests are part of
our post-commit and to make it green, this commit temporary adds those
aliases back.

Note that it is not entierly clear if those aliases should have been
removed in the first place, see KhronosGroup/SYCL-Docs#335.
@gmlueck
Copy link
Contributor

gmlueck commented Jan 13, 2023

To me, this looks a bit weird to have sycl::vec<long, 2> and sycl::long2 which are potentially different types.

I definitely agree that this is a problem, and I think this should be fixed in the SYCL 2020 spec. The problem exists also for the marray aliases, and it exists for other alias variants like sycl::short.

there is no alias for long long type

This seems like an oversight to me.

@fraggamuffin
Copy link

same as #353

@keryell
Copy link
Member

keryell commented Sep 25, 2024

We need to review the whole story again for the CTS.

@keryell keryell added the Agenda To be discussed during a SYCL committee meeting label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agenda To be discussed during a SYCL committee meeting
Projects
None yet
Development

No branches or pull requests

4 participants