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] Add some trivial util functions for half type. #6691

Merged
merged 12 commits into from
Nov 1, 2022
Merged

[SYCL] Add some trivial util functions for half type. #6691

merged 12 commits into from
Nov 1, 2022

Conversation

jinge90
Copy link
Contributor

@jinge90 jinge90 commented Sep 2, 2022 •
edited
Loading

The usage of sycl::half and CUDA half is different, sycl::half overrides arithmetic and comparison operators(+, -, *, /, >, <,==.!=...) while CUDA math provides those arithmetic and comparison functionalities via a bunch of util functions. This PR provides some sycl::half util functions aligning with CUDA math, users who are porting half related CUDA code to SYCL will spend less effort with those util functions' help.
Signed-off-by: jinge90 ge.jin@intel.com

@jinge90 jinge90 requested a review from a team as a code owner September 2, 2022 02:17
@jinge90 jinge90 marked this pull request as draft September 2, 2022 02:17
@jinge90 jinge90 mentioned this pull request Sep 11, 2022
Merged
@jinge90 jinge90 marked this pull request as ready for review September 16, 2022 07:13
@jinge90 jinge90 requested review from akolesov-intel, zettai-reido and xtian-github and removed request for akolesov-intel September 16, 2022 07:14
@jinge90
Copy link
Contributor Author

jinge90 commented Sep 19, 2022

/verify with intel/llvm-test-suite#1250

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 3, 2022

/verify with intel/llvm-test-suite#1250

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 3, 2022

Hi, @xtian-github , @zettai-reido and @intel/llvm-reviewers-runtime
Could you take a look at this PR?
Thanks very much.

xtian-github
xtian-github approved these changes Oct 3, 2022
View reviewed changes
Copy link

@xtian-github xtian-github left a comment

Choose a reason for hiding this comment

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

LGTM

zettai-reido
zettai-reido approved these changes Oct 3, 2022
View reviewed changes
Copy link

@zettai-reido zettai-reido left a comment

Choose a reason for hiding this comment

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

LGTM

@romanovvlad
Copy link
Contributor

romanovvlad commented Oct 4, 2022 •
edited
Loading

Hi,
@jinge90
Since the functions are in public namespace could you please clarify if we have an extension document covering them? If not, shouldn't we add/update existing one?
Could you please add more details to the PR summary?

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 11, 2022

/verify with intel/llvm-test-suite#1250

xtian-github
xtian-github approved these changes Oct 12, 2022
View reviewed changes
Copy link

@xtian-github xtian-github left a comment

Choose a reason for hiding this comment

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

LGTM

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 17, 2022

Hi, @pvchupin
Could you take a look at this PR?

Thanks very much.

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 20, 2022

Hi, @pvchupin
Kind ping.

@pvchupin
Copy link
Contributor

@jinge90, please address @romanovvlad comments. And please get @intel/llvm-reviewers-runtime approval.

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 21, 2022

Hi, @romanovvlad
As we discussed, we can proceed with this PR and I will submit a new PR to update the doc, is it OK?

@intel/llvm-reviewers-runtime
Could you help review this PR?

Thanks very much.

@romanovvlad
Copy link
Contributor

Hi, @romanovvlad As we discussed, we can proceed with this PR and I will submit a new PR to update the doc, is it OK?

@intel/llvm-reviewers-runtime Could you help review this PR?

Thanks very much.

This is OK.

@pvchupin
Copy link
Contributor

Could you please add more details to the PR summary?

@jinge90, please do this.

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 24, 2022

Could you please add more details to the PR summary?

@jinge90, please do this.

Done.

Thanks very much.

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 25, 2022 •
edited
Loading

Hi, @intel/llvm-reviewers-runtime and @sergey-semenov
Could you take a look at this PR?

Thanks very much.

@jinge90
Copy link
Contributor Author

jinge90 commented Oct 27, 2022

Hi, @intel/llvm-reviewers-runtime and @sergey-semenov
Kind ping~

cperkinsintel
cperkinsintel approved these changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

LGTM

@pvchupin pvchupin merged commit b4ce7c0 into intel:sycl Nov 1, 2022
@jinge90 jinge90 deleted the imf_half_trivial_hpp branch December 8, 2022 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zettai-reido zettai-reido zettai-reido approved these changes

@xtian-github xtian-github xtian-github approved these changes

@akolesov-intel akolesov-intel akolesov-intel approved these changes

@cperkinsintel cperkinsintel cperkinsintel approved these changes

@sergey-semenov sergey-semenov Awaiting requested review from sergey-semenov sergey-semenov 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.
7 participants
@jinge90 @romanovvlad @pvchupin @cperkinsintel @akolesov-intel @xtian-github @zettai-reido