Skip to content

Conversation

kbobrovs
Copy link
Contributor

@kbobrovs kbobrovs commented Mar 24, 2022

@kbobrovs kbobrovs requested review from a team as code owners March 24, 2022 06:39
s-kanaev
s-kanaev previously approved these changes Mar 24, 2022
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kbobrovs
Copy link
Contributor Author

@rolandschulz, please also review

@kbobrovs
Copy link
Contributor Author

Had to rebase to import some recent fixes in other places.

@rolandschulz, @Pennycook, @s-kanaev - please review

@s-kanaev s-kanaev requested a review from Pennycook March 31, 2022 11:08
s-kanaev
s-kanaev previously approved these changes Mar 31, 2022
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Pennycook
Pennycook previously approved these changes Mar 31, 2022
@kbobrovs kbobrovs dismissed stale reviews from Pennycook and s-kanaev via 9d1e5c3 April 1, 2022 02:07
@kbobrovs kbobrovs marked this pull request as draft April 1, 2022 18:44
kbobrovs added 9 commits April 4, 2022 08:41
- invoke_simd:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_invoke_simd.asciidoc
  Current limitations:
  * function pointers and lambdas not supported
  * simd width fixed to 16, must be auto-deduced in future
  * bool parameter type not supported

- uniform:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_uniform.asciidoc

Signed-off-by: Konstantin S Bobrovsky <[email protected]>
- Improvements based on @rolandschulz's POC:
  * auto-deduce the subgroup size
  * support lambdas and functors
- Added more compilation test cases
- Split __builtin_invoke_simd into two overloads
- Added TODO about uniform handling in the compiler
- fix compilation errors of other tests related to incorrect
   sub_group namespace in forward declaration in uniform.hpp
- fix test to add SYCL_EXTERNAL to undefined functor's '()' operators

Signed-off-by: Konstantin S Bobrovsky <[email protected]>
Signed-off-by: Konstantin S Bobrovsky <[email protected]>
@kbobrovs
Copy link
Contributor Author

kbobrovs commented Apr 5, 2022

The only failure is unrelated - clang FE crash on Printf/long.cpp:

[2022-04-05T02:26:01.584Z] ******************** TEST 'SYCL :: Printf/long.cpp' FAILED ********************
...
[2022-04-05T02:26:01.584Z] $ "w:/jenkins-dir/workspace/SYCL_CI/intel/Win/Test_Suite/llvm.obj/bin/clang-cl.exe" "/EHsc" "-fsycl" "-fsycl-targets=spir64" "W:\jenkins-dir\workspace\SYCL_CI\intel\Win\Test_Suite\llvm-test-suite\SYCL\Printf\long.cpp" "-o" "W:\jenkins-dir\workspace\SYCL_CI\intel\Win\Test_Suite\build\SYCL\Printf\Output\long.cpp.tmp.out"
[2022-04-05T02:26:01.584Z] # command stderr:
[2022-04-05T02:26:01.584Z] PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
[2022-04-05T02:26:01.584Z] Stack dump:
[2022-04-05T02:26:01.585Z] 0.	Program arguments: w:\\jenkins-dir\\workspace\\sycl_ci\\intel\\win\\test_suite\\llvm.obj\\bin\\clang-cl.exe -cc1 ...
...
[2022-04-05T02:26:01.585Z] clang-cl: error: clang frontend command failed due to signal (use -v to see invocation)

@kbobrovs kbobrovs marked this pull request as ready for review April 5, 2022 21:26
@kbobrovs
Copy link
Contributor Author

kbobrovs commented Apr 5, 2022

@rolandschulz, @Pennycook, @s-kanaev, please take one more look. I had to fight some of Linux-only failures (bug in glibcxx, different assert_is_func instantiations mapping to the same mangling in the test), sub_group namespace, as well as had to rebase to include a fix for incorrect sycl::detail::boost::mp11 deploy directory. I think the patch will be ready to merge, as testing passed except one unrelated failure.

#else
{
// __builtin_invoke_simd is not supported on the host device yet
throw sycl::feature_not_supported();
Copy link
Contributor

@v-klochkov v-klochkov Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not use the deprecated API. I recently made corresponding change to fix warnings: #5861
(There are few more similar places below)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw sycl::feature_not_supported();
__ESIMD_UNSUPPORTED_ON_HOST;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, will address in a separate PR.

@kbobrovs kbobrovs requested a review from Pennycook April 6, 2022 15:44
Copy link
Contributor

@Pennycook Pennycook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving because I don't want to block this. But I suspect that we will have to revisit the sub_group question I raised in #5871 (comment).

Copy link
Contributor

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the usage of deprecated feature_not_supported() in follow-up PR.

@kbobrovs
Copy link
Contributor Author

kbobrovs commented Apr 6, 2022

Approving because I don't want to block this. But I suspect that we will have to revisit the sub_group question I raised in #5871 (comment).

John, does this address the concern - #5871 (comment) ?
I'm merging since you and others approved, will follow-up on this later if needed.

@kbobrovs kbobrovs merged commit a37ca84 into intel:sycl Apr 6, 2022
@kbobrovs kbobrovs deleted the invoke_simd_api branch April 6, 2022 19:59
gmlueck added a commit to gmlueck/llvm that referenced this pull request Sep 21, 2023
The uniform extension was implemented in intel#5871.  Move the extension
specification document to the "experimental" directory.
bader pushed a commit that referenced this pull request Sep 27, 2023
The uniform extension was implemented in #5871.  Move the extension
specification document to the "experimental" directory, and update to
the
latest specification template.

Also fix a small bug in the implementation.  The `sycl::id` type should
not be disallowed for use in `uniform`.  Although, it is invalid to
pass the work-item's ID to `uniform` (because that value is not uniform
across all work-items), the user could construct their own `id` that
does have a uniform value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants