[SYCL] Align `range` with SYCL 2020 by Robertkq · Pull Request #22889 · intel/llvm

Robertkq · 2026-08-06T12:40:24Z

This PR introduces an explicit line for range destructor (Rule of 5 or 0 is mandatory) and a missing overload for operator op, alongside multiple missing noexcept keywords for functions

Robertkq · 2026-08-06T13:03:03Z

This PR introduces an explicit line for range destructor (Rule of 5 or 0 is mandatory) and a missing overload for operator op, alongside multiple missing noexcept keywords for functions

Robertkq · 2026-08-06T13:05:31Z

This overload is not in SYCL 2020 latest documentation, not adding noexcept

I believe the idea is that all operators for range class should be noexcept. So I vote for adding this specificator if we keep these operators.
could you please dig into commit history and check if there are any clues why these operators were introduced?

I believe it is best to just remove these, they were introduced initially in #4538, seems like there was / is some implicit conversion of id & range, but even with these, the documented, existing functions covers all cases, especially: sycl/test/basic_tests/id.cpp passes without any problems, where this exact thing is tested.

In latest commit, I've removed them

Robertkq · 2026-08-06T13:06:05Z

This overload is not in SYCL 2020 latest documentation, not adding noexcept. Also it is not guarded by __SYCL_DISABLE_ID_TO_INT_CONV__ like the previous ones

I guess that's why you've also skipped some operators on lines 112 & 120 (upd. oh sorry you've already mentioned this above)? I'm not sure why we need these, I need to check the spec.

I believe this should also have noexcept. It's an #else branch of #ifndef __SYCL_DISABLE_ID_TO_INT_CONV__, although I don't know yet why we need this.
Anyways, std::is_integral_v<size_t> equals to true.
UPD looks like previously in SYCL 2020 were only operators with size_t args, but DPC++ supported all integer types. Seems like then it became part of the standard, see #4538 (comment)

@KseniyaTikhomirova do you know what is the purpose of __SYCL_DISABLE_ID_TO_INT_CONV__? At least here it looks like it does nothing as the code in both branches of this macro is same.

function body is same but function signature is different. Depending on this macro id<> to size_t convertion is enabled and operator can accept more types. Although I don't really know the reason why it is worth disabling and it is not a default option, I assume it may be done to enable some compiler optimization. @sergey-semenov do you know anything about it?

@sergey-semenov should we just get rid of this and unify the code?

As it stands, this is an undocumented feature that we support, but at least we provide a way to turn it off. I think this should be formalized as a proper extension (assuming we want to keep supporting this) and then we can get rid of the macro.

@sergey-semenov why is it not a part of spec?

https://github.com/intel/llvm/blob/sycl/sycl/include/sycl/id.hpp#L106

class id declaration in SYCl2020 contains:

// only available if Dimensions == 1 operator std::size_t() const noexcept;

Whoops, I somehow overlooked that. Then yeah, we should just get rid of the macro.

@Robertkq feel free to remove this or just skip this so we'll remove this later.

Robertkq · 2026-08-06T13:07:34Z

-range(size_t, size_t, size_t)->range<3>;
+range(size_t) -> range<1>;
+range(size_t, size_t) -> range<2>;
+range(size_t, size_t, size_t) -> range<3>;


automatic change from my clang-format, I think it adheres to coding style rules of the repository

That's ok, let's update this. In general try to use git clang-format HEAD~ after you've created a commit. It'll only format your changes then.

Yeah this is my first time running into this kind of issue, maybe I don't know exactly how to fix it but I ran: python3 clang/tools/clang-format/git-clang-format HEAD~ which also with use of AI, I think should format correctly, but it left the files unmodified. Can you please tell me if with your clang-format version it really doesnt do this change?

I have this:

clang-format version 22.1.8 (Fedora 22.1.8-4.fc44)

Anyways, I can always just save without formatting and remove this, but I'd like to confirm / learn something new from this if possible :D

See https://github.com/intel/llvm/blob/sycl/clang/tools/clang-format/git-clang-format
You need to put it to your PATH. Then call git clang-format *commit* where *commit* is the commit to compare current changes with, in general it's the previous commit.

CI uses clang-format-20 Ubuntu package.

From the log:

Setting up clang-format-20 (1:20.1.8~++20250804090239+87f0227cb601-1~~exp1~~20250804210352.139) ...

please just revert this code change

I think I correctly setup my local environment to use clang-format 20.1.8 and I think this code style change is expected. This format issues we're a mess on my side, if it is possible to merge it like this as (I strongly believ) it adheres to project code style, for next PRs I will try to run git clang-format HEAD~ frequently to avoid changing lines I didn't touch, but my commit history on this is a bit messy with the changes. If needed, I can manually remove these, let me know

Robertkq · 2026-08-06T13:08:36Z

@KornevNikita @KseniyaTikhomirova can you take a look over the PR? Thanks!

Robertkq · 2026-08-06T13:34:49Z

Also, I wasn't able to find an existing test file to add a case for the new overload. If it exists and it can be provided to me, I'll happily add tests, otherwise I think the PR can continue?

KornevNikita · 2026-08-06T13:43:52Z

This PR introduces an explicit line for range destructor (Rule of 5 or 0 is mandatory) and a missing overload for operator op, alongside multiple missing noexcept keywords for functions

could you please add this to the description

KornevNikita · 2026-08-06T15:15:04Z


  range(const range<Dimensions> &rhs) = default;
-  range(range<Dimensions> &&rhs) = default;
+  range(range<Dimensions> &&rhs) noexcept = default;


IIUC SYCL 2020 doesn't require these functions (for by-value semantics) to be noexcept.

That's right.. I thought that I should apply good practice here to add noexcept for move functions so they could benefit from moves. standard library usually moves types only if they have noexcept move constructor / operator, but now I properly understand that this type is designed to be passed by value and as such this noexcept should bring little to no improvement, so I will most likely remove it from here

KornevNikita · 2026-08-06T15:22:39Z

I guess that's why you've also skipped some operators on lines 112 & 120 (upd. oh sorry you've already mentioned this above)? I'm not sure why we need these, I need to check the spec.

KornevNikita · 2026-08-06T15:24:10Z

-range(size_t, size_t, size_t)->range<3>;
+range(size_t) -> range<1>;
+range(size_t, size_t) -> range<2>;
+range(size_t, size_t, size_t) -> range<3>;


That's ok, let's update this. In general try to use git clang-format HEAD~ after you've created a commit. It'll only format your changes then.

Robertkq · 2026-08-12T20:41:38Z

@KornevNikita Hello, sorry for the delay. I was not able to figure out the clang-format at all and from the test failiure, it is not only the last part of the file that does not adhere to the rules. Locally, everything I've tried indicates that the files are formatted correctly. I'm unsure what the next steps would be if the test fails again.

Anyways -- please take a look over the PR. I removed the undocumented and unguarded feature for size_t but I didn't remove the one that is guarded by the macro, can you please confirm if that should be removed or not? Thanks!

KornevNikita

can you please confirm if that should be removed or not? Thanks!

According to Sergey's comment above we can remove the #else branch of the __SYCL_DISABLE_ID_TO_INT_CONV__ macro and keep the code under the macro as default.

UPD. probably let's not do it for now, I need to investigate a bit.

KornevNikita · 2026-08-13T11:14:54Z

@@ -68,10 +69,12 @@ template <int Dimensions = 1> class range : public detail::array<Dimensions> {
  range<Dimensions> &operator=(range<Dimensions> &&rhs) = default;
  range() = default;


Suggested change

range() = default;

range() noexcept = default;

…dded RFC

Robertkq · 2026-08-16T10:11:05Z

@KornevNikita Is there any chance you can checkout my branch and apply the necessary formatting chances and push a commit? I can't get it to work at all..

Also, on a sidenote, is there a certain reason why CI would use clang-format version 20.x.x? It's quite a bit old and I'm pretty sure we compile clang-format or could compile format from LLVM?

Anyways -- would really appreciate if you or anyone else with some free time could apply the formatting changes to me and push to this branch, thanks!

KornevNikita · 2026-08-17T12:11:21Z

@KornevNikita Is there any chance you can checkout my branch and apply the necessary formatting chances and push a commit? I can't get it to work at all..

Done. Actually you can just check the workflow output and apply it manually.
Anyway, all I did was - put https://github.com/intel/llvm/blob/sycl/clang/tools/clang-format/git-clang-format to PATH (I'm using ubuntu, so it might be different for win). Ran:

git clang-format d87afb86a0c6ba414e4f6c994df93249504831ce

it's the commit in your branch before your first commit.

Also, on a sidenote, is there a certain reason why CI would use clang-format version 20.x.x? It's quite a bit old and I'm pretty sure we compile clang-format or could compile format from LLVM?

IIUC https://github.com/aminya/setup-cpp is used to install clang-format. I see that https://github.com/llvm/llvm-project/blob/main/.github/workflows/pr-code-format.yml is different, likely we don't sync our workflow with llvm-project, I'll check that.

Robertkq · 2026-08-17T17:47:50Z

@KornevNikita Thank you for the commit with format-clang changes, I will keep trying to solve the issue on my side as well.

Can I get workflow approval and reviews on the PR, I believe it is close if not in merge-able state.
@KseniyaTikhomirova kind ping for review as code owner

Also, related to noexpects, I've opened this issue for SYCL-Docs

Thanks!

Robertkq · 2026-08-17T19:10:08Z

2 failiures in:
SYCL Pre Commit on Windows / run_prebuilt_e2e_tests (Intel Arc Graphics with Level Zero, ["Windows","arc"]) / Intel Arc Graphics with Level Zero (pull_request)

Basic/alloc_pinned_host_memory.cpp:
#22287 -- Issue is opened for BMG, it passes on BMG but fails on arc, could it be that arc also is affected by sporadic fails on this?

Basic/fill_accessor.cpp:
#19586 -- Same, opened for BMG.

Worth nothing, issues are opened for BMG while jobs fail on arc, also tests make use of range which is modified, but I doubt it's not sporadic fail

If it's suspected that it's not sporadic, can I get a re-run of the jobs? I don't have such hardware or setup to run myself

KornevNikita

LGTM. @KseniyaTikhomirova could you please take a look? thanks.

KseniyaTikhomirova · 2026-08-18T11:11:00Z

  }

-  size_t get(int dimension) const {
+  size_t get(int dimension) const noexcept {


check_dimension throws exception https://github.com/Robertkq/llvm/blob/fce97a96106612800cf50c2d50db736ab964d259/sycl/include/sycl/detail/array.hpp#L111
that means that it is incorrect just to mark functions calling it as noexcept with the current impl of check_dimension.

I've swapped the throwsin favor of __SYCL_ASSERT, similar to how sycl_span checks. There are no tests that depend on catching that exception

KseniyaTikhomirova · 2026-08-18T11:12:28Z

    return result;                                                             \
  }
 #else
+// RFC: remove these as well?


looks like comment which should be removed

Robertkq changed the title ~~Add missing functions & noexcept keyword~~ [SYCL] Add missing functions & noexcept keyword Aug 6, 2026

Robertkq changed the title ~~[SYCL] Add missing functions & noexcept keyword~~ [SYCL] Align range to SYCL 2020 specs Aug 6, 2026

Robertkq commented Aug 6, 2026

View reviewed changes

Comment thread sycl/include/sycl/range.hpp Outdated

Robertkq commented Aug 6, 2026

View reviewed changes

Robertkq marked this pull request as ready for review August 6, 2026 13:08

Robertkq requested a review from a team as a code owner August 6, 2026 13:08

Robertkq requested a review from KseniyaTikhomirova August 6, 2026 13:08

KornevNikita reviewed Aug 6, 2026

View reviewed changes

Robertkq force-pushed the Robertkq/22736 branch from e267437 to c7f8336 Compare August 12, 2026 20:37

KornevNikita reviewed Aug 13, 2026

View reviewed changes

Robertkq added 5 commits August 16, 2026 12:49

Add missing functions & noexcept keyword

3539930

remove comments

0d7e1cb

add noexcept to detail::array

336282f

remove undocumented unguarded feature & some unnecessary noexcepts, a…

250d2b4

…dded RFC

noexcept for default constructor

7c8569f

Robertkq force-pushed the Robertkq/22736 branch from c7f8336 to 7c8569f Compare August 16, 2026 10:07

emdevv mentioned this pull request Aug 16, 2026

[SYCL] Add missing noexcept to sycl::id functions #22898

Merged

format

fce97a9

KornevNikita approved these changes Aug 18, 2026

View reviewed changes

KornevNikita changed the title ~~[SYCL] Align range to SYCL 2020 specs~~ [SYCL] Align range with SYCL 2020 Aug 18, 2026

KseniyaTikhomirova reviewed Aug 18, 2026

View reviewed changes

Switch throw in favor of assert & remove old code

67d6953

Robertkq requested a review from KseniyaTikhomirova August 22, 2026 18:13

		@@ -68,10 +69,12 @@ template <int Dimensions = 1> class range : public detail::array<Dimensions> {
		range<Dimensions> &operator=(range<Dimensions> &&rhs) = default;
		range() = default;

Uh oh!

Conversation

Robertkq commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Robertkq commented Aug 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KornevNikita Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KornevNikita Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KseniyaTikhomirova Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robertkq commented Aug 6, 2026

Uh oh!

Robertkq commented Aug 6, 2026

Uh oh!

KornevNikita commented Aug 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KornevNikita Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robertkq commented Aug 12, 2026

Uh oh!

KornevNikita left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Robertkq commented Aug 6, 2026 •

edited

Loading

KornevNikita Aug 6, 2026 •

edited

Loading

KornevNikita Aug 6, 2026 •

edited

Loading

KseniyaTikhomirova Aug 7, 2026 •

edited

Loading

KornevNikita Aug 6, 2026 •

edited

Loading

KornevNikita left a comment •

edited

Loading

KornevNikita commented Aug 17, 2026 •

edited

Loading