[SYCL] Deprecate sycl::atomics in SYCL 2020 mode #5440

IgorKharchikov · 2022-01-31T23:32:04Z

Explicitly marked class cl::sycl::atomic as deprecated to meet an ad hoc requirement of new SYCL standard.

Redefinition of default constructor image_accessor() (previously used "= default") to make it possible to use const references in parallel_for.

Reduced ad hoc test file.

Fixed typo

Rewrite of RUN part.

Created new test for fixed issue with mismatched namespace for ctz function.

[SYCL]Created regression test for ctz fix

…ed_tests Revert "[SYCL]Created regression test for ctz fix"

Reduced test file so it contains only func() function, edited commented line for test running.

Edited commented line for testing, corrected typos in code.

Removed SYCL_EXTERNAL attribute due to it's unnecessity in this test (parallel_for is used here).

Co-authored-by: Alexey Bader <[email protected]>

cl::sycl::atomic was marked as deprecated to meet new SYCL standart.

sycl/test/regression/atomic_deprecated.cpp

bader · 2022-02-02T09:37:02Z

Just out of curiosity. What are following commands doing in the test:

// RUN: %clangxx %fsycl-host-only -fsyntax-only -sycl-std=2017 -Xclang -verify -Xclang -verify-ignore-unexpected=note %s -o %t.out
// RUN: %clangxx %fsycl-host-only -fsyntax-only -sycl-std=1.2.1 -Xclang -verify -Xclang -verify-ignore-unexpected=note %s -o %t.out

I would expect that they are added to check that the compiler doesn't emit any errors or warnings in SYCL 1.2.1 mode, but I don't see any checks for that. @vladimirlaz, could you clarify, please?

BTW, I think one of them must be removed. There is a driver check that -sycl-std=1.2.1 and -sycl-std=2017 work the same, so using both is unnecessary.

IgorKharchikov · 2022-02-02T10:12:11Z

@bader, as far as I am concerned, -sycl-std args are to explicitly mark the version of the compiler, with which the syntax shall be checked, additionally ignoring notes, if any (due to -verify-ignore-unexpected=note). Is that right? In addition, I understand that this may be redundant here, as deprecations from this list must appear only in 2020, but I am still not sure.

bader · 2022-02-02T10:26:02Z

-sycl-std args are to explicitly mark the version of the compiler

It's not the version of the compiler, but the version of the SYCL standard the compiler should comply with.

The test does four compilations and two of the are obviously redundant.
One of two (that are supposed to be useful) doesn't seem to work as intended, but I'm not sure about that, so I'd like @vladimirlaz to confirm if my concern is valid.

bader · 2022-02-03T06:22:37Z

@vladimirlaz, ping.
@alexbatashev, do you know how the test is supposed to work?

alexbatashev · 2022-02-03T06:27:51Z

@bader this isn't how this test looked like when it was created. Originally, it was using -Werror to check, that there're no warnings for SYCL 1.2.1. That was changed with this commit 8445dc1

@AlexeySachkov

vladimirlaz · 2022-02-03T07:09:24Z

I would expect that they are added to check that the compiler doesn't emit any errors or warnings in SYCL 1.2.1 mode, but I don't see any checks for that. @vladimirlaz, could you clarify, please?

I have the same expectations.

bader · 2022-02-03T07:10:55Z

I would expect that they are added to check that the compiler doesn't emit any errors or warnings in SYCL 1.2.1 mode, but I don't see any checks for that. @vladimirlaz, could you clarify, please?

I have the same expectations.

Cool. Do you agree that test does exactly the opposite and checks that warnings are emitted for SYCL-1.2.1?

vladimirlaz · 2022-02-03T07:16:24Z

yes.
It is reasonable to update the test to make sure that no warning is emitted when -sycl-std=2020 is specified and is not emitted when if is not specific.
But it is out of the scope of the PR. It can be done separately.

bader · 2022-02-03T07:24:52Z

Okay. I would prefer to remove lines 2-4 within this PR and add a check for "no warnings" in a separate PR. Does it sound okay to everyone?

IgorKharchikov · 2022-02-03T08:00:06Z

@bader, ok, I will do it right now, but just to put it clear: only the first //RUN string shall remain, right?

bader

only the first //RUN string shall remain, right?

Right. Thanks.

bader · 2022-02-03T10:36:47Z

yes. It is reasonable to update the test to make sure that no warning is emitted when -sycl-std=2020 is specified and is not emitted when if is not specific. But it is out of the scope of the PR. It can be done separately.

It's very surprising.

// RUN: %clangxx %fsycl-host-only -fsyntax-only -sycl-std=2017 -Xclang -verify -Xclang -verify-ignore-unexpected=note %s -o %t.out
// RUN: %clangxx %fsycl-host-only -fsyntax-only -sycl-std=1.2.1 -Xclang -verify -Xclang -verify-ignore-unexpected=note %s -o %t.out

These two checks are successfully passing i.e. the compiler emits warnings in SYCL 1.2.1 mode.

* upstream/sycl: (3571 commits) [ESIMD] Doxygen update part III - core APIs. (intel#5472) [SYCL][DOC] Move proposed FPGA extensions (intel#5453) [SYCL] Add -fsycl-fp32-prec-sqrt flag (intel#5309) [SYCL] Emit program build logs for warning levels >= 2 (intel#5319) [SYCL] Add clang support for code_location in KernelInfo (intel#5335) [SYCL][Doc] Move FPGA extensions (intel#5470) [ESIMD] Fix public simd and simd_view APIs. (intel#5465) [SYCL] Deprecate sycl::atomics in SYCL 2020 mode (intel#5440) [SYCL] Add unit test for PR 5414 (intel#5450) [XPTI] Allow arbitrary data types in metadata (intel#4998) [SYCL][DOC] Move discard queue events to supported (intel#5452) [Driver][SYCL] Initial support for allowing fat static -lname processing (intel#5413) [SYCL] Fix dead pointer usage if leaf buffer overflows (intel#5417) [SYCL][L0] Fix memory leak in USM prefetch (intel#5461) [SYCL][Doc] Add new free function queries proposal (intel#5106) [SYCL][ESIMD] Update vc-intrinsics deps to the top of the trunk (intel#5460) [SYCL][DOC] Move old spec constant extension spec (intel#5456) [SYCL][DOC] Move deprecated extensions (intel#5458) [SYCL][DOC] Fix links to old SubGroupMask doc (intel#5459) [ESIMD] Doxygen update part II - memory APIs. (intel#5443) ...

IgorKharchikov and others added 21 commits January 14, 2022 21:20

Updated image_accessor constructor

225c6c0

Redefinition of default constructor image_accessor() (previously used "= default") to make it possible to use const references in parallel_for.

Added test for image_accessor const ref fix

667ce93

Update const_ref_image_accessor.cpp

0645ee6

Reduced ad hoc test file.

Update const_ref_image_accessor.cpp

a6eefc8

Fixed typo

Update const_ref_image_accessor.cpp

f5641da

Rewrite of RUN part.

[SYCL]Created regression test for ctz fix

b8f7509

Created new test for fixed issue with mismatched namespace for ctz function.

Merge pull request #1 from IgorKharchikov/IgorKharchikov-added_tests

d433b34

[SYCL]Created regression test for ctz fix

Revert "[SYCL]Created regression test for ctz fix"

0f54611

Merge pull request #2 from IgorKharchikov/revert-1-IgorKharchikov-add…

42ef2fa

…ed_tests Revert "[SYCL]Created regression test for ctz fix"

Update const_ref_image_accessor.cpp

7465d36

Reduced test file so it contains only func() function, edited commented line for test running.

Update const_ref_image_accessor.cpp

9b4be53

Edited commented line for testing, corrected typos in code.

Merge branch 'intel:sycl' into sycl

bcf96d3

Merge branch 'intel:sycl' into sycl

f30e0b4

Update const_ref_image_accessor.cpp

333ea45

Removed SYCL_EXTERNAL attribute due to it's unnecessity in this test (parallel_for is used here).

Update const_ref_image_accessor.cpp

e650676

Update sycl/test/check_device_code/const_ref_image_accessor.cpp

92a181c

Co-authored-by: Alexey Bader <[email protected]>

Merge branch 'intel:sycl' into sycl

96e6c67

Merge branch 'intel:sycl' into sycl

473c784

Merge branch 'intel:sycl' into sycl

05d3e9f

Update atomic.hpp

a8c1492

cl::sycl::atomic was marked as deprecated to meet new SYCL standart.

Create atomic_deprecated.cpp

b064743

IgorKharchikov requested a review from a team as a code owner January 31, 2022 23:32

IgorKharchikov requested a review from alexbatashev January 31, 2022 23:32

vladimirlaz suggested changes Feb 1, 2022

View reviewed changes

sycl/test/regression/atomic_deprecated.cpp Outdated Show resolved Hide resolved

IgorKharchikov added 4 commits February 1, 2022 11:43

Update sycl_2020_deprecations.cpp

2d73f27

Delete atomic_deprecated.cpp

5aeef52

Update sycl_2020_deprecations.cpp

74acc39

Update sycl_2020_deprecations.cpp

af9ba65

vladimirlaz previously approved these changes Feb 1, 2022

View reviewed changes

bader changed the title ~~[SYCL] Marked sycl::atomics as deprecated~~ [SYCL] Deprecate sycl::atomics in SYCL 2020 mode Feb 2, 2022

Update sycl_2020_deprecations.cpp

d0a07a6

IgorKharchikov dismissed vladimirlaz’s stale review via d0a07a6 February 3, 2022 08:01

alexbatashev approved these changes Feb 3, 2022

View reviewed changes

bader approved these changes Feb 3, 2022

View reviewed changes

bader merged commit 52fad5a into intel:sycl Feb 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Deprecate sycl::atomics in SYCL 2020 mode #5440

[SYCL] Deprecate sycl::atomics in SYCL 2020 mode #5440

Uh oh!

IgorKharchikov commented Jan 31, 2022

Uh oh!

Uh oh!

bader commented Feb 2, 2022

Uh oh!

IgorKharchikov commented Feb 2, 2022

Uh oh!

bader commented Feb 2, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

alexbatashev commented Feb 3, 2022

Uh oh!

vladimirlaz commented Feb 3, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

vladimirlaz commented Feb 3, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

IgorKharchikov commented Feb 3, 2022

Uh oh!

bader left a comment

Uh oh!

bader commented Feb 3, 2022

Uh oh!

Uh oh!

[SYCL] Deprecate sycl::atomics in SYCL 2020 mode #5440

[SYCL] Deprecate sycl::atomics in SYCL 2020 mode #5440

Uh oh!

Conversation

IgorKharchikov commented Jan 31, 2022

Uh oh!

Uh oh!

bader commented Feb 2, 2022

Uh oh!

IgorKharchikov commented Feb 2, 2022

Uh oh!

bader commented Feb 2, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

alexbatashev commented Feb 3, 2022

Uh oh!

vladimirlaz commented Feb 3, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

vladimirlaz commented Feb 3, 2022

Uh oh!

bader commented Feb 3, 2022

Uh oh!

IgorKharchikov commented Feb 3, 2022

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

bader commented Feb 3, 2022

Uh oh!

Uh oh!