[SYCL] Add DPC++ RT support for SYCL 2020 spec constants (part 1) #3382

dm-vodopyanov · 2021-03-19T18:05:45Z

This patch adds partial implementation of specialization constants in DPC++ RT:

Implementation of specialization_id class
Implementation of kernel_handler class
Support for user's device lambdas which take kernel_handler argument

This patch adds partial implementation of specialization constants in DPC++ RT: 1. Implementation of `specialization_id` class 2. Implementation of `kernel_handler` class 3. Support for user's device lambdas which take `kernel_handler` argument

dm-vodopyanov · 2021-03-19T18:07:04Z

This patch depends on FE part #3345 and tools part #3353

dm-vodopyanov · 2021-03-19T18:09:27Z

sycl/include/CL/sycl/kernel_handler.hpp

+  T getSpecializationConstantOnDevice() {
+    const char *SymbolicID = __builtin_unique_stable_name(
+        detail::specialization_id_name_generator<S>);
+    return __sycl_getScalar2020SpecConstantValue<T>(


Declaration is here: https://github.com/intel/llvm/pull/3353/files#diff-622b2db16138a17764c8376940c1bb4f5d748a395849f21f7d9635ab78752e6eR36

dm-vodopyanov · 2021-03-19T18:09:45Z

sycl/include/CL/sycl/kernel_handler.hpp

+  T getSpecializationConstantOnDevice() {
+    const char *SymbolicID = __builtin_unique_stable_name(
+        detail::specialization_id_name_generator<S>);
+    return __sycl_getComposite2020SpecConstantValue<T>(


Declaration is here: https://github.com/intel/llvm/pull/3353/files#diff-622b2db16138a17764c8376940c1bb4f5d748a395849f21f7d9635ab78752e6eR41

sycl/include/CL/sycl/handler.hpp

sycl/include/CL/sycl/kernel_handler.hpp

dm-vodopyanov · 2021-03-23T23:53:38Z

sycl/include/CL/sycl/handler.hpp

+
+      // TODO: replace detail::RangeRoundedLambda with
+      // "constexpr if" when DPC++ RT switched to C++17
+      detail::RangeRoundedLambda<TransformedArgType, KernelType, Dims> Wrapper{


@rdeodhar, please review from range rounding point-of-view

The function containing the range rounding code, "void parallel_for_lambda_impl(range NumWorkItems, KernelType KernelFunc) {...}" is called with a KernelFunc.

KernelFunc may have a kernel_handler parameter. If it does, the wrapped kernel should be declared with that additional parameter, (say, KH), and the body of the wrapper should pass it to the original kernel, as KernelFunc(Arg, KH);

I'm not sure how you'll do it.

sycl/test/regression/sycl-include-gnu17.cpp

dm-vodopyanov · 2021-03-24T00:20:04Z

Ran post-commit locally on Linux with #3345 and #3353 patches: no failures during build and check-sycl:

[36/39] Running device-agnostic SYCL regression tests for SPIR-V
-- Testing: 208 tests, 24 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 85.77s
  Unsupported:   2
  Passed     : 206
[37/39] Running the SYCL regression tests for OpenCL
-- Testing: 9 tests, 9 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 9.85s
  Unsupported: 4
  Passed     : 5
[38/39] Running the SYCL regression tests for Level Zero
-- Testing: 9 tests, 9 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 9.24s
  Unsupported: 5
  Passed     : 4

dm-vodopyanov · 2021-03-24T13:35:35Z

Ran post-commit locally on Windows with #3345 and #3353 patches: no failures during build, and one unrelated to this patch failure in SYCL-on-device :: back_to_back_collectives.cpp (machine environment issue):

[57/60] Running device-agnostic SYCL regression tests for SPIR-V
-- Testing: 208 tests, 12 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 118.07s
  Unsupported:  20
  Passed     : 188
[58/60] Running the SYCL regression tests for Level Zero
-- Testing: 9 tests, 9 workers --
Testing:  0.. 10.. 20.. 30
FAIL: SYCL-on-device :: back_to_back_collectives.cpp (4 of 9)
******************** TEST 'SYCL-on-device :: back_to_back_collectives.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   d:\dvodopya\dv-llvm\build\bin\clang.exe --driver-mode=g++ -fsycl -fsycl-targets=spir64-unknown-unknown-sycldevice D:\dvodopya\dv-llvm\sycl\test\on-device\back_to_back_collectives.cpp -o D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out
: 'RUN: at line 2';   env SYCL_DEVICE_FILTER=host  D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out
: 'RUN: at line 3';   true D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out
: 'RUN: at line 4';    env SYCL_DEVICE_FILTER=level_zero:gpu  D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out
: 'RUN: at line 5';   true D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out
--
Exit Code: 127

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "d:\dvodopya\dv-llvm\build\bin\clang.exe" "--driver-mode=g++" "-fsycl" "-fsycl-targets=spir64-unknown-unknown-sycldevice" "D:\dvodopya\dv-llvm\sycl\test\on-device\back_to_back_collectives.cpp" "-o" "D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out"
$ ":" "RUN: at line 2"
$ "env" "SYCL_DEVICE_FILTER=host" "D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out"
# command output:
Skipping test


$ ":" "RUN: at line 3"
$ "true" "D:\dvodopya\dv-llvm\build\tools\sycl\test\Output\back_to_back_collectives.cpp.tmp.out"
# command stderr:
'true': command not found
error: command failed with exit status: 127

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (1):
  SYCL-on-device :: back_to_back_collectives.cpp


Testing Time: 13.45s
  Unsupported: 1
  Passed     : 7
  Failed     : 1

dm-vodopyanov · 2021-03-24T13:37:33Z

Ping @ everyone

dm-vodopyanov · 2021-03-24T14:24:23Z

sycl/include/CL/sycl/detail/cg_types.hpp

+};
+
+template <typename F, typename RetT, typename... Args>
+struct check_fn_signature<F, RetT(Args...)> {


Moved from handler.hpp as is, except check_kernel_lambda_takes_args - this is a new type trait

dm-vodopyanov · 2021-03-24T14:34:56Z

sycl/include/CL/sycl/detail/cg_types.hpp

-                                    }
-                                    MKernel(ID);
-                                  });
+    detail::NDLoop<Dims>::iterate(


clang-format, the only change - runKernelWithArg... line

sycl/include/CL/sycl/detail/cg_types.hpp

sycl/include/CL/sycl/handler.hpp

romanovvlad · 2021-03-24T15:17:20Z

sycl/include/CL/sycl/kernel_handler.hpp

+
+#ifdef __SYCL_DEVICE_ONLY__
+  template <auto &S, typename T = std::remove_reference_t<decltype(S)>,
+            std::enable_if_t<std::is_scalar_v<T>>>


Does it make sense to use is_fundamental here?
@AlexeySachkov

Made is_fundamental. @AlexeySachkov, can you please review if is_fundamental and is_compaund are correct type traits here?

sycl/test/on-device/basic_tests/specialization_constants/kernel_handler.cpp

sycl/include/CL/sycl/handler.hpp

kbobrovs · 2021-03-25T23:45:11Z

sycl/include/CL/sycl/handler.hpp

+  // additional kernel_handler argument.
+
+  // NOTE: to support kernel_handler argument in kernel lambdas, only
+  // kernel_***_wrapper functions must be called in this code


This condition - only kernel_***_wrapper functions must be called in this code - does not hold. kernel_handler constructor is also called. Can it be a problem if the constructor becomes non-trivial?

This condition - only kernel_***_wrapper functions must be called in this code - does not hold

Ca you please elaborate on this a bit? What I meant by this is that we shouldn't call, e.g., kernel_parallel_for in our headers anywhere because we need some logic above it to decide what version of kernel_parallel_for to run - with or w/o kernel_handler support. And we need some place to initialize kernel_handler obj on our side. In these cases kernel_parallel_for_wrapper acts as this place.

Can it be a problem if the constructor becomes non-trivial?

It can if something will be changed in spec, in this case initialization of kernel_handler object should be re-written here. Right now we have separate init function __init_specialization_constants_buffer which is called in FE.

OK, I see. This is not quite clear from the comments - I'd suggest to reword for better clarity.
My guess was that this was related more to parallel_for_work_group, where variables local to a kernel become WG-shared (address space 3 global in LLVM IR), and there is code filtering for the WG leader work item.

kbobrovs · 2021-03-25T23:50:09Z

sycl/include/CL/sycl/kernel_handler.hpp

+// This guard is needed because the libsycl.so can compiled with C++ <=14
+// while the code requires C++17. This code is not supposed to be used by the
+// libsycl.so so it should not be a problem.
+#if __cplusplus > 201402L


NIT: spec const 2020 design is not settled yet, so this code may need to be changed/removed in future. + @AlexeySachkov, @gmlueck for possible comments.

sycl/include/CL/sycl/kernel_handler.hpp

sycl/include/CL/sycl/specialization_id.hpp

...est/on-device/basic_tests/specialization_constants/kernel_lambda_with_kernel_handler_arg.cpp

sycl/include/CL/sycl/specialization_id.hpp

sycl/include/CL/sycl/detail/kernel_desc.hpp

sycl/source/detail/scheduler/commands.cpp

sycl/source/handler.cpp

dm-vodopyanov · 2021-03-26T18:01:48Z

Jenkins/Precommit failure: tests compile with c++14, removed the flag and created PR: intel/llvm-test-suite#202

dm-vodopyanov added 2 commits March 19, 2021 20:49

Code style fixes

9fd5474

dm-vodopyanov requested a review from a team as a code owner March 19, 2021 18:05

dm-vodopyanov requested a review from vladimirlaz March 19, 2021 18:05

dm-vodopyanov requested review from romanovvlad, AlexeySachkov and kbobrovs March 19, 2021 18:07

dm-vodopyanov commented Mar 19, 2021

View reviewed changes

romanovvlad reviewed Mar 21, 2021

View reviewed changes

sycl/include/CL/sycl/handler.hpp Outdated Show resolved Hide resolved

sycl/include/CL/sycl/kernel_handler.hpp Show resolved Hide resolved

Fix pre-commit

da5af9e

dm-vodopyanov commented Mar 23, 2021

View reviewed changes

sycl/test/regression/sycl-include-gnu17.cpp Show resolved Hide resolved

dm-vodopyanov requested a review from rdeodhar March 23, 2021 23:55

Minor changes

4f2a03c

Fix build failure on Windows

90dbbe3

dm-vodopyanov commented Mar 24, 2021

View reviewed changes

romanovvlad reviewed Mar 24, 2021

View reviewed changes

dm-vodopyanov added 2 commits March 25, 2021 18:54

Fix CR commnets

fb62c8b

Remove unnecessary changes

c6cf04f

dm-vodopyanov requested a review from romanovvlad March 25, 2021 16:02

Fix compilation error with msvc

f92ccfe

kbobrovs reviewed Mar 26, 2021

View reviewed changes

dm-vodopyanov added 2 commits March 26, 2021 12:57

Apply CR comments

9c60d80

Add TODO for support spec consts on host device

558f24b

dm-vodopyanov requested a review from kbobrovs March 26, 2021 12:05

gmlueck reviewed Mar 26, 2021

View reviewed changes

sycl/include/CL/sycl/specialization_id.hpp Outdated Show resolved Hide resolved

romanovvlad previously approved these changes Mar 26, 2021

View reviewed changes

Add kind_specialization_constants_buffer to kernel_desc.hpp

39c4df8

dm-vodopyanov dismissed romanovvlad’s stale review via 39c4df8 March 26, 2021 13:57

erichkeane reviewed Mar 26, 2021

View reviewed changes

sycl/include/CL/sycl/detail/kernel_desc.hpp Outdated Show resolved Hide resolved

Fix subsequent build failure

9b167ab

kbobrovs suggested changes Mar 26, 2021

View reviewed changes

sycl/source/detail/scheduler/commands.cpp Outdated Show resolved Hide resolved

sycl/source/handler.cpp Outdated Show resolved Hide resolved

Apply CR comments

dd4d260

kbobrovs previously approved these changes Mar 27, 2021

View reviewed changes

Temporarily disabling spec const test for CUDA

1a0b200

dm-vodopyanov dismissed kbobrovs’s stale review via 1a0b200 March 29, 2021 15:25

dm-vodopyanov requested a review from romanovvlad March 30, 2021 10:05

romanovvlad previously approved these changes Mar 30, 2021

View reviewed changes

Merge branch 'sycl' into private/dvodopya/spec_consts_dpcpp_rt_part_1

73f1d19

dm-vodopyanov dismissed romanovvlad’s stale review via 73f1d19 March 31, 2021 15:20

dm-vodopyanov requested a review from romanovvlad March 31, 2021 15:55

romanovvlad approved these changes Mar 31, 2021

View reviewed changes

kbobrovs approved these changes Mar 31, 2021

View reviewed changes

dm-vodopyanov mentioned this pull request Mar 31, 2021

Disable some SYCL/Reduction tests on Windows OpenCL intel/llvm-test-suite#211

Merged

bader merged commit 4524a6b into intel:sycl Apr 1, 2021

[SYCL] Add DPC++ RT support for SYCL 2020 spec constants (part 1) #3382

[SYCL] Add DPC++ RT support for SYCL 2020 spec constants (part 1) #3382

Uh oh!

Conversation

dm-vodopyanov commented Mar 19, 2021

Uh oh!

dm-vodopyanov commented Mar 19, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dm-vodopyanov commented Mar 24, 2021

Uh oh!

dm-vodopyanov commented Mar 24, 2021

Uh oh!

dm-vodopyanov commented Mar 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dm-vodopyanov Mar 24, 2021 • 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!

dm-vodopyanov Mar 24, 2021 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dm-vodopyanov commented Mar 26, 2021

Uh oh!

Uh oh!

dm-vodopyanov commented Mar 24, 2021 •

edited

Loading

dm-vodopyanov Mar 24, 2021 •

edited

Loading

dm-vodopyanov Mar 24, 2021 •

edited

Loading