Skip to content
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

[SYCL] Make SYCL math functions overloads instead of templates #9753

Merged
merged 19 commits into from
Jun 29, 2023
Merged

[SYCL] Make SYCL math functions overloads instead of templates #9753

merged 19 commits into from
Jun 29, 2023

Conversation

KornevNikita
Copy link
Contributor

@KornevNikita KornevNikita commented Jun 6, 2023 •
edited
Loading

It was decided to change all built-in functions from templates to overloads. This patch changes non-marray part of "4.17.5. Math functions".

Spec: KhronosGroup/SYCL-Docs#428

@KornevNikita @steffenlarsen
[SYCL] Make SYCL math functions overloads instead of templates
0c64b20 
It was decided to change all built-in functions from templates to
overloads. This patch changes non-marray part of "4.17.5. Math
functions".

Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
@KornevNikita KornevNikita requested a review from a team as a code owner June 6, 2023 12:06
dm-vodopyanov
dm-vodopyanov reviewed Jun 6, 2023
View reviewed changes
sycl/include/sycl/builtins.hpp Outdated Show resolved Hide resolved
sycl/include/sycl/builtins.hpp Show resolved Hide resolved
@KornevNikita KornevNikita temporarily deployed to aws June 6, 2023 13:23 — with GitHub Actions Inactive
@bader
Copy link
Contributor

bader commented Jun 6, 2023

It was decided to change all built-in functions from templates to overloads.

Why?

@dm-vodopyanov
Copy link
Contributor

It was decided to change all built-in functions from templates to overloads.

Why?

Replied internally.

@KornevNikita
Copy link
Contributor Author

It was decided to change all built-in functions from templates to overloads.

Why?

Replied internally.

Thanks. I'll post a link here, when there is a spec-change PR, describing these changes.

@KornevNikita KornevNikita requested a review from a team as a code owner June 9, 2023 16:28
@KornevNikita KornevNikita temporarily deployed to aws June 9, 2023 18:46 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 9, 2023 19:58 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 13, 2023 15:52 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 13, 2023 17:49 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 14, 2023 10:01 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 14, 2023 10:54 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 14, 2023 15:27 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 14, 2023 17:18 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 19, 2023 12:10 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 19, 2023 13:18 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 20, 2023 15:50 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 22, 2023 09:28 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 22, 2023 10:20 — with GitHub Actions Inactive
@KornevNikita
Copy link
Contributor Author

@steffenlarsen @dm-vodopyanov this patch finally passes CI, but I'm going to reduce it a bit by outlining the defines - #10026.

@KornevNikita KornevNikita changed the title [SYCL] Make SYCL math functions overloads instead of templates [DO NOT MERGE][SYCL] Make SYCL math functions overloads instead of templates Jun 22, 2023
@KornevNikita KornevNikita changed the title [DO NOT MERGE][SYCL] Make SYCL math functions overloads instead of templates [SYCL] Make SYCL math functions overloads instead of templates Jun 22, 2023
@KornevNikita KornevNikita temporarily deployed to aws June 22, 2023 14:24 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 22, 2023 15:38 — with GitHub Actions Inactive
KornevNikita
KornevNikita commented Jun 23, 2023
View reviewed changes
sycl/include/sycl/builtins.hpp
__SYCL_DEF_BUILTIN_GENFLOAT
#undef __SYCL_BUILTIN_DEF

// svgenfloat fract (svgenfloat x, genfloatptr iptr)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed there are no some built-ins that accept arguments with gen*ptr:

genfloat fract(genfloat x, genfloatptr iptr)
genfloat frexp(genfloat x, genintptr exp)
genfloat lgamma_r(genfloat x, genintptr signp)
genfloat modf(genfloat x, genfloatptr iptr)
genfloat remquo(genfloat x, genfloat y, genintptr quo)
genfloat sincos(genfloat x, genfloatptr cosval)

I think it will be better to create a separate patch for them as this patch is already too big for review.

@KornevNikita KornevNikita temporarily deployed to aws June 23, 2023 16:17 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 23, 2023 17:27 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 26, 2023 08:25 — with GitHub Actions Inactive
@KornevNikita KornevNikita temporarily deployed to aws June 26, 2023 09:19 — with GitHub Actions Inactive
@KornevNikita
Copy link
Contributor Author

@intel/dpcpp-esimd-reviewers could you please take a look at esimd-related part?

@dm-vodopyanov
Copy link
Contributor

@intel/dpcpp-esimd-reviewers, friendly ping.

sarnex
sarnex approved these changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

esimd changes look good to me, slava may have more feedback when he is back from vacation but i don't see anything insane

@dm-vodopyanov dm-vodopyanov merged commit 826d868 into intel:sycl Jun 29, 2023
Chenyang-L pushed a commit that referenced this pull request Jun 30, 2023
@KornevNikita @steffenlarsen @Chenyang-L
[SYCL] Make SYCL math functions overloads instead of templates (#9753)
9944033 
It was decided to change all built-in functions from templates to
overloads. This patch changes non-marray part of "4.17.5. Math
functions".

Spec: KhronosGroup/SYCL-Docs#428

---------

Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
steffenlarsen added a commit to steffenlarsen/llvm that referenced this pull request Jul 3, 2023
@steffenlarsen
Revert "[SYCL] Make SYCL math functions overloads instead of templates (
0c9aa18 
intel#9753)"

This reverts commit 826d868.
@steffenlarsen steffenlarsen mentioned this pull request Jul 3, 2023
Merged
steffenlarsen added a commit that referenced this pull request Jul 3, 2023
@steffenlarsen
[SYCL] Temporarily revert recent SYCL builtin changes (#10173)
f109448 
This partially reverts the following patches:

- #9753
- #10050
- #10048
- #10014
- #10026

General fixes have been kept.

---------

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@Chenyang-L Chenyang-L mentioned this pull request Jul 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dm-vodopyanov dm-vodopyanov dm-vodopyanov approved these changes

@sarnex sarnex sarnex approved these changes

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

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

Successfully merging this pull request may close these issues.
5 participants
@KornevNikita @bader @dm-vodopyanov @sarnex @steffenlarsen