Skip to content

[ESIMD]Move addc and subb functions out of experimental namespace #10589

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 3 commits into from
Jul 28, 2023
Merged

[ESIMD]Move addc and subb functions out of experimental namespace #10589

merged 3 commits into from
Jul 28, 2023

Conversation

fineg74
Copy link
Contributor

@fineg74 fineg74 commented Jul 26, 2023

No description provided.

@fineg74 fineg74 requested a review from a team as a code owner July 26, 2023 23:23
@fineg74 fineg74 temporarily deployed to aws July 26, 2023 23:38 — with GitHub Actions Inactive
v-klochkov
v-klochkov reviewed Jul 26, 2023
View reviewed changes
sycl/include/sycl/ext/intel/experimental/esimd/math.hpp Outdated

carry = Result.first;
return Result.second;
__SYCL_DEPRECATED("the function moved out of experimental namespace")
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
__SYCL_DEPRECATED("the function moved out of experimental namespace")
__SYCL_DEPRECATED("Please use sycl::ext::intel::esimd::addc(carry, src0, src1);")

v-klochkov
v-klochkov reviewed Jul 26, 2023
View reviewed changes
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.

Looks good. I have only 1 comment regarding the deprecation message.

@fineg74 fineg74 temporarily deployed to aws July 27, 2023 00:20 — with GitHub Actions Inactive
turinevgeny
turinevgeny reviewed Jul 27, 2023
View reviewed changes
Copy link
Contributor

@turinevgeny turinevgeny left a comment

Choose a reason for hiding this comment

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

Do you think we need tests for both experimental and non-experimental versions? What is the rule of thumb?

@fineg74
Copy link
Contributor Author

fineg74 commented Jul 27, 2023

Do you think we need tests for both experimental and non-experimental versions? What is the rule of thumb?

I updated the tests only for non-experimental version only to avoid producing deprecation messages while building the tests and save some work later when the experimental API will be actually deleted

@fineg74 fineg74 temporarily deployed to aws July 27, 2023 02:34 — with GitHub Actions Inactive
@fineg74 fineg74 temporarily deployed to aws July 27, 2023 03:12 — with GitHub Actions Inactive
@turinevgeny
Copy link
Contributor

Do you think we need tests for both experimental and non-experimental versions? What is the rule of thumb?

I updated the tests only for non-experimental version only to avoid producing deprecation messages while building the tests and save some work later when the experimental API will be actually deleted

Ok, thanks.

@v-klochkov v-klochkov merged commit 9e3852b into intel:sycl Jul 28, 2023
@fineg74 fineg74 deleted the addcsubbMove branch August 1, 2023 16:20
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
@fineg74 @mdtoguchi
[ESIMD] Move addc() and subb() functions out of experimental namespace (
5859546 
intel#10589)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turinevgeny turinevgeny turinevgeny approved these changes

@v-klochkov v-klochkov v-klochkov approved these changes

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
@fineg74 @turinevgeny @v-klochkov