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][Reduction] Hide reducer non-standard members and add identity #8215

Merged
merged 5 commits into from
Feb 10, 2023
Merged

[SYCL][Reduction] Hide reducer non-standard members and add identity #8215

merged 5 commits into from
Feb 10, 2023

Conversation

steffenlarsen
Copy link
Contributor

This commit hides the members in reducer that are not mentioned in the SYCL 2020 specification and introduces the identity member function.

@steffenlarsen
[SYCL] Add reducer class member aliases and constexpr value
033aaac 
This commit adds the value_type and binary_operation member aliases and
the dimensions value to the reducer class.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
[SYCL][Reduction] Hide reducer non-standard members and add identity
b032eda 
This commit hides the members in reducer that are not mentioned in the
SYCL 2020 specification and introduces the identity member function.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
Copy link
Contributor Author

Currently in draft as it builds on #8211.

@steffenlarsen steffenlarsen temporarily deployed to aws February 6, 2023 18:24 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen temporarily deployed to aws February 6, 2023 19:54 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen marked this pull request as ready for review February 7, 2023 13:26
@steffenlarsen steffenlarsen requested a review from a team as a code owner February 7, 2023 13:26
@steffenlarsen
Copy link
Contributor Author

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

@steffenlarsen steffenlarsen mentioned this pull request Feb 7, 2023
Merged
@steffenlarsen
Fix formatting
77bf7af 
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen temporarily deployed to aws February 7, 2023 14:14 — with GitHub Actions Inactive
@steffenlarsen
Copy link
Contributor Author

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

@steffenlarsen steffenlarsen temporarily deployed to aws February 7, 2023 18:42 — with GitHub Actions Inactive
@steffenlarsen
Address MSVC error
81afc45 
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
Copy link
Contributor Author

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

@steffenlarsen
Copy link
Contributor Author

/testwin

@steffenlarsen steffenlarsen temporarily deployed to aws February 8, 2023 10:33 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen temporarily deployed to aws February 9, 2023 03:38 — with GitHub Actions Inactive
@steffenlarsen steffenlarsen merged commit 505aa7d into intel:sycl Feb 10, 2023
whitneywhtsang added a commit to whitneywhtsang/llvm that referenced this pull request Feb 17, 2023
@whitneywhtsang
Revert "[SYCL][Reduction] Hide reducer non-standard members and add i…
55c95b6 
…dentity (intel#8215)"

This reverts commit 505aa7d.
@whitneywhtsang
Copy link
Contributor

@steffenlarsen This change fails to compile with clang++ with errors:

include/sycl/reduction.hpp:286:60: error: member reference base type 'ReducerAccess' is not a structure or union
      Functor(std::move(AtomicRef), ReducerAccess{*reducer}.getElement(E));
include/sycl/reduction.hpp:959:47: error: member reference base type 'ReducerAccess' is not a structure or union
          GroupSum[E] = ReducerAccess(Reducer).getIdentity();
                        ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
include/sycl/reduction.hpp:972:35: error: member reference base type 'ReducerAccess' is not a structure or union
            ReducerAccess{Reducer}.getElement(E) = GroupSum[E];
            ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
include/sycl/reduction.hpp:1022:49: error: member reference base type 'ReducerAccess' is not a structure or union
          auto &RedElem = ReducerAccess{Reducer}.getElement(E);
include/sycl/reduction.hpp:1033:41: error: member reference base type 'ReducerAccess' is not a structure or union
                  ReducerAccess{Reducer}.getElement(E);
                  ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
include/sycl/reduction.hpp:1056:51: error: member reference base type 'ReducerAccess' is not a structure or union
            auto LocalSum = ReducerAccess{Reducer}.getIdentity();
include/sycl/reduction.hpp:1146:48: error: member reference base type 'ReducerAccess' is not a structure or union
        LocalReds[LID] = ReducerAccess{Reducer}.getElement(E);
include/sycl/reduction.hpp:1221:33: error: member reference base type 'ReducerAccess' is not a structure or union
          ReducerAccess{Reducer}.getElement(E) = reduce_over_group(
          ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
include/sycl/reduction.hpp:1222:55: error: member reference base type 'ReducerAccess' is not a structure or union
              NDIt.get_group(), ReducerAccess{Reducer}.getElement(E), BOp);
                                ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
...

@steffenlarsen
Copy link
Contributor Author

@whitneywhtsang - Seems like it could be a problem with the deduction guides. Which version of clang is having this problem?

@whitneywhtsang
Copy link
Contributor

@whitneywhtsang - Seems like it could be a problem with the deduction guides. Which version of clang is having this problem?

$ /usr/bin/clang++ --version
clang version 11.0.0 (Red Hat 11.0.0-1.module+el8.4.0+8598+a071fcd5)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

@steffenlarsen
Copy link
Contributor Author

Fixed in #8411.

whitneywhtsang added a commit to whitneywhtsang/llvm that referenced this pull request Feb 22, 2023
@whitneywhtsang
Revert "Revert "[SYCL][Reduction] Hide reducer non-standard members a…
e945d42 
…nd add identity (intel#8215)""

This reverts commit 55c95b6.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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