[SYCL] Implement annotated_arg headers #6894

wangdi4 · 2022-09-28T16:55:42Z

Add annotated_arg headers based on sycl_ext_oneapi_annotated_arg.asciidoc in PR #5755

steffenlarsen

In general I think this looks good, but we may want to hold back on merging it until the extension has been finalized and merged.

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp

…annotation

dm-vodopyanov · 2022-11-03T10:58:31Z

@wangdi4 friendly ping.

AlexeySachkov · 2022-12-09T13:40:47Z

sycl/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp

+#ifdef __SYCL_DEVICE_ONLY__
+  using global_pointer_t = typename sycl::detail::DecoratedType<
+      T, access::address_space::global_space>::type *;
+#else
+  using global_pointer_t = T *;
+#endif


I wonder, can we use decorated_global_ptr<T>::pointer here? See 4.7.7.2. Explicit pointer aliases and 4.7.7.1. Multi-pointer class

Changed to using global_pointer_t =typename decorated_global_ptr<T>::pointer;
Have confirmed the compile-time properties are correctly propagated to the kernel argument attribute in output IR.

sycl/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp

steffenlarsen

Just a few more comments but otherwise I think it's looking good.

sycl/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp

steffenlarsen

I have no more concerns. Note that the extension is in review still.

againull · 2023-01-04T23:25:16Z

It looks like extension is merged: #5755

wangdi4 Could you please resolve merge conflicts?

againull · 2023-01-06T21:52:32Z

@wangdi4 Could you please fix formatting issue.

bader · 2023-01-13T00:38:21Z

@wangdi4, this PR introduced several issues to post-commit testing (see below). Please, fix ASAP or revert the patch if takes significant time to fix.

Compilation errors:

/home/runner/work/llvm/llvm/build/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp:97:40: error: unused parameter 'PropList' [-Werror,-Wunused-parameter]
                const property_list_t &PropList = properties{}) noexcept
                                       ^
/home/runner/work/llvm/llvm/build/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp:141:47: error: unused parameter 'proplist' [-Werror,-Wunused-parameter]
                         const PropertyListV &proplist) noexcept
                                              ^
/home/runner/work/llvm/llvm/build/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp:198:40: error: unused parameter 'PropList' [-Werror,-Wunused-parameter]
                const property_list_t &PropList = properties{}) noexcept
                                       ^
/home/runner/work/llvm/llvm/build/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp:241:47: error: unused parameter 'proplist' [-Werror,-Wunused-parameter]
                         const PropertyListV &proplist) noexcept
                                              ^
4 errors generated.

Link to the logs - https://github.com/intel/llvm/actions/runs/3906152648/jobs/6673997977

SYCL-CTS regression.
I'm not sure if this is related but this is the first commit where this issue appeared. This might be caused by some recent SYCL-CTS changes.
Link to logs - https://github.com/intel/llvm/actions/runs/3904102563/jobs/6670410622.

group_api test fails on validation. @steffenlarsen, FYI.

romanovvlad · 2023-01-13T10:42:47Z

Patch which silences unused variable warning #8002

steffenlarsen · 2023-01-13T14:42:50Z

SYCL-CTS regression. I'm not sure if this is related but this is the first commit where this issue appeared. This might be caused by some recent SYCL-CTS changes. Link to logs - https://github.com/intel/llvm/actions/runs/3904102563/jobs/6670410622.

group_api test fails on validation.

I suspect KhronosGroup/SYCL-CTS#457 is the reason. Do we know if this is only a problem for the CUDA backend?

bader · 2023-01-13T18:21:41Z

SYCL-CTS regression. I'm not sure if this is related but this is the first commit where this issue appeared. This might be caused by some recent SYCL-CTS changes. Link to logs - https://github.com/intel/llvm/actions/runs/3904102563/jobs/6670410622.
group_api test fails on validation.

I suspect KhronosGroup/SYCL-CTS#457 is the reason. Do we know if this is only a problem for the CUDA backend?

Unfortunately, we don't run CTS on other platforms. Khronos SYCL-CTS CI builds for SPIR-V devices, but the problem hits the execution.

wangdi4 requested a review from a team as a code owner September 28, 2022 16:55

wangdi4 requested a review from steffenlarsen September 28, 2022 16:55

steffenlarsen requested changes Sep 29, 2022

View reviewed changes

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp Outdated Show resolved Hide resolved

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp Outdated Show resolved Hide resolved

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp Outdated Show resolved Hide resolved

Chen, Brox and others added 7 commits September 29, 2022 11:52

added sycl kernel arg annotation support

1c4379c

run clang-format

21d5629

Merge branch 'sycl' of https://github.com/intel/llvm into mmhost_ptr_…

75a72a0

…annotation

resolve merge error

7a50687

run clang-format

9fe0a3d

combine changes on spirv-header & spirv-translator

f53534c

add annotated_arg

27a8714

wangdi4 added 7 commits November 21, 2022 06:42

implement based on annotated_arg spec

a657d70

add ctors for case when T is a pointer type

addd1c2

add deduction guide

9360f9f

finish pointer's implementation of annnotated_arg

8299d1a

finish non-pointer's implementation of annnotated_arg

5ab61cf

small fixes

33a2557

add operator[]

acc0bee

wangdi4 force-pushed the annotated_arg branch from cba0c8c to acc0bee Compare December 1, 2022 20:24

wangdi4 requested review from a team as code owners December 1, 2022 20:24

wangdi4 added 8 commits December 1, 2022 14:30

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

1086a42

revert SPIRV changes

55f300e

add extension tests

d7c82c9

reformat

863b355

code refactor

78f0fd1

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

e8fb0bc

fix warning

8155e5b

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

98799c1

wangdi4 added 2 commits December 6, 2022 08:16

resolve comments

b0a8119

code cleanup

0895a73

steffenlarsen requested a review from AlexeySachkov December 7, 2022 15:35

AlexeySachkov reviewed Dec 9, 2022

View reviewed changes

sort properties in deduction guide

c5b90ce

AlexeySachkov removed the request for review from a team December 12, 2022 15:39

wangdi4 added 4 commits December 13, 2022 06:21

resolve comments

4558641

use multi_pointer for decorated global pointer

856221d

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

d10aad2

fix LIT error

41fe464

wangdi4 requested review from AlexeySachkov and steffenlarsen and removed request for AlexeySachkov December 13, 2022 16:15

steffenlarsen reviewed Dec 13, 2022

View reviewed changes

sycl/include/sycl/ext/oneapi/annotated_arg/annotated_arg.hpp Outdated Show resolved Hide resolved

sycl/include/sycl/ext/oneapi/annotated_arg/properties.hpp Outdated Show resolved Hide resolved

check is_device_copyable<T>

b3e4fd9

steffenlarsen approved these changes Dec 14, 2022

View reviewed changes

wangdi4 requested a review from AlexeySachkov December 14, 2022 13:14

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

dc28208

wangdi4 added 2 commits January 10, 2023 13:04

Merge branch 'sycl' of https://github.com/intel/llvm into annotated_arg

183046a

run clang-format

5567daa

wangdi4 temporarily deployed to aws January 11, 2023 17:43 — with GitHub Actions Inactive

wangdi4 temporarily deployed to aws January 12, 2023 07:21 — with GitHub Actions Inactive

againull merged commit 1649fe4 into intel:sycl Jan 12, 2023

bader mentioned this pull request Apr 3, 2023

Doxygen documentation build is broken. #8932

Closed

[SYCL] Implement annotated_arg headers #6894

[SYCL] Implement annotated_arg headers #6894

Uh oh!

Conversation

wangdi4 commented Sep 28, 2022

Uh oh!

steffenlarsen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dm-vodopyanov commented Nov 3, 2022

Uh oh!

AlexeySachkov Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

wangdi4 Dec 13, 2022 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steffenlarsen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

steffenlarsen left a comment

Choose a reason for hiding this comment

Uh oh!

againull commented Jan 4, 2023

Uh oh!

againull commented Jan 6, 2023

Uh oh!

bader commented Jan 13, 2023

Uh oh!

romanovvlad commented Jan 13, 2023

Uh oh!

steffenlarsen commented Jan 13, 2023

Uh oh!

bader commented Jan 13, 2023

Uh oh!

Uh oh!

wangdi4 Dec 13, 2022 •

edited

Loading