Skip to content

[SYCL] Refactor builtins implementation #11956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 25 commits into from
Jan 17, 2024
Merged

[SYCL] Refactor builtins implementation #11956

merged 25 commits into from
Jan 17, 2024

Conversation

aelovikov-intel
Copy link
Contributor

@aelovikov-intel aelovikov-intel commented Nov 20, 2023 •
edited
Loading

See builtins_preview.hpp for the outline of the new design. This PR
changes the implementation under -fpreview-breaking-changes and
removes reliance on a python builtins generator script.

Suggested reading/review order: builtins_preview.hpp, helper_macros.hpp,
host_helper_macros.hpp, then headers implementing user-visible side with
the library implementation sycl/source/builtins/*_functions.cpp last.

@aelovikov-intel aelovikov-intel requested a review from a team as a code owner November 20, 2023 23:27
@aelovikov-intel aelovikov-intel force-pushed the builtins-pr branch 2 times, most recently from 77dffe7 to cf277dc Compare December 7, 2023 00:11
@aelovikov-intel aelovikov-intel force-pushed the builtins-pr branch 2 times, most recently from 74457c9 to 7ca0f9c Compare December 8, 2023 01:14
@aelovikov-intel aelovikov-intel force-pushed the builtins-pr branch 2 times, most recently from 80adc97 to 862165f Compare December 8, 2023 23:37
@aelovikov-intel aelovikov-intel changed the title [WIP] Refactor SYCL builtins implementation Refactor SYCL builtins implementation Dec 11, 2023
@aelovikov-intel
Copy link
Contributor Author

@AlexeySachkov , @sergey-semenov , ping x3.

sergey-semenov
sergey-semenov reviewed Jan 12, 2024
View reviewed changes
Copy link
Contributor

@sergey-semenov sergey-semenov left a comment

Choose a reason for hiding this comment

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

Sorry about the late response. I don't have any objections on the overall approach, just some very minor comments from me.

@aelovikov-intel
Copy link
Contributor Author

Jenkins/RHEL build failure is caused by the usage of outdated RHEL7.8 image + gcc 7.5.0 toolchain where std::isinf(double) returns int instead of bool. The issue does not exist using RHEL8.8 + same gcc 7.5.0 toolchain and we will be switching the CI task to that configuration.

@aelovikov-intel aelovikov-intel merged commit 7e9819d into intel:sycl Jan 17, 2024
@aelovikov-intel aelovikov-intel deleted the builtins-pr branch January 17, 2024 18:29
@aelovikov-intel aelovikov-intel mentioned this pull request Jan 18, 2024
Merged
aelovikov-intel added a commit that referenced this pull request Jan 19, 2024
@aelovikov-intel
[SYCL] Try to fix MacOS build after #11956 (#12444)
36e82f2 
MacOS' STL is not standard-conformant, try to workaround for that in our
code.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jan 19, 2024
@aelovikov-intel
[SYCL] Remove builtins_generator.py
1930d18 
Not used after intel#11956.
@aelovikov-intel aelovikov-intel mentioned this pull request Jan 19, 2024
Merged
aelovikov-intel added a commit that referenced this pull request Jan 24, 2024
@aelovikov-intel
[SYCL] Remove builtins_generator.py (#12451)
48ec4dd 
Not used after #11956.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Feb 2, 2024
@aelovikov-intel
[SYCL] Fix bitselect builtin for integer types
dc6c737 
This regressed after intel#11956 as return type wasn't correctly converted
from SPIR-V intrinsic back to SYCL types. This PR fixes that.

In addition, I'm also adding tests for `sycl::select` builtin that was
left unaffected only because we couldn't use SPIR-V intrinsic for its
implementation.
@aelovikov-intel aelovikov-intel mentioned this pull request Feb 2, 2024
Merged
aelovikov-intel added a commit that referenced this pull request Feb 5, 2024
@aelovikov-intel
[SYCL] Fix bitselect builtin for integer types (#12598)
a2e1669 
This regressed after #11956 as return type wasn't correctly converted
from SPIR-V intrinsic back to SYCL types. This PR fixes that.

In addition, I'm also adding tests for `sycl::select` builtin that was
left unaffected only because we couldn't use SPIR-V intrinsic for its
implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergey-semenov sergey-semenov sergey-semenov approved these changes

@steffenlarsen steffenlarsen steffenlarsen left review comments

@againull againull Awaiting requested review from againull againull is a code owner automatically assigned from intel/llvm-reviewers-runtime

@AlexeySachkov AlexeySachkov Awaiting requested review from AlexeySachkov

@cperkinsintel cperkinsintel Awaiting requested review from cperkinsintel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aelovikov-intel @sergey-semenov @steffenlarsen