Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL] Test reduction for complex numbers. #1609

Merged
merged 11 commits into from
Mar 22, 2023
Merged

[SYCL] Test reduction for complex numbers. #1609

merged 11 commits into from
Mar 22, 2023

Conversation

uditagarwal97
Copy link

This PR adds test case for testing identityless reduction for complex numbers.

Corresponding intel/llvm PR: intel/llvm#8425

@uditagarwal97 uditagarwal97 requested a review from a team as a code owner February 22, 2023 16:17
@uditagarwal97 uditagarwal97 mentioned this pull request Feb 22, 2023
Merged
@uditagarwal97
Copy link
Author

/Verify with intel/llvm#8425

aelovikov-intel
aelovikov-intel reviewed Feb 22, 2023
View reviewed changes
@uditagarwal97
Copy link
Author

/verify with intel/llvm#8425

steffenlarsen pushed a commit to intel/llvm that referenced this pull request Mar 2, 2023
@uditagarwal97
[SYCL] Augment known_identity for std::complex and std::plus. (#8425)
2e73312 
This PR proposes to augment sycl::has_known_identity to return true for
std::complex and std::plus operator. This will have two benefits:

1. It enables support for complex numbers in sycl::reduction without the
user having to explicitly pass identity.
2. sycl::known_identity can now be used to simplify the implementation
of group algorithms (See PR #5394).

Also, this PR addresses the Github issue #5477.
Test Case PR: intel/llvm-test-suite#1609
@uditagarwal97
Copy link
Author

@steffenlarsen The PR is ready, please merge it. The failures in Jenkins CI are unrelated.

@steffenlarsen
Copy link

@steffenlarsen The PR is ready, please merge it. The failures in Jenkins CI are unrelated.

Test seems to be failing on Windows. I believe you need #include <algorithm>.

steffenlarsen
steffenlarsen reviewed Mar 14, 2023
View reviewed changes
@uditagarwal97
Copy link
Author

@steffenlarsen the test case is passing now for both Windows and Linux. There was a bug in the test case before:

The problem was in the test case, precisely in the following statement:std::generate(a.begin(), a.end(),                   [&n] { return std::complex<T>(n, ++n + 1); });

I was using this statement to initialize the buffer. However, the order of evaluation of the expression <n, ++n + 1> is undefined and that's why for different compilers on Linux and Windows, it was initializing the buffer differently.

steffenlarsen
steffenlarsen approved these changes Mar 22, 2023
View reviewed changes
Copy link

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Good find! Looks good. 🚀

@steffenlarsen steffenlarsen merged commit 8d77241 into intel:intel Mar 22, 2023
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
@uditagarwal97
[SYCL] Test reduction for complex numbers. (intel/llvm-test-suite#1609)
870c8c4 
This PR adds test case for testing identityless reduction for complex numbers.

Corresponding intel/llvm PR: intel#8425
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aelovikov-intel aelovikov-intel aelovikov-intel approved these changes

@steffenlarsen steffenlarsen steffenlarsen 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
@uditagarwal97 @steffenlarsen @aelovikov-intel