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

[SYCL] Add identityless reduction test cases #1640

Merged

Conversation

steffenlarsen
Copy link

This commit adds test cases to existing tests for testing reductions where the identity is unspecified and unknown.

This commit adds test cases to existing tests for testing reductions
where the identity is unspecified and unknown.

Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen requested a review from a team as a code owner March 7, 2023 14:47
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen
Copy link
Author

/verify with intel/llvm#8562

Comment on lines 15 to 18
template <typename Name, typename T, class BinaryOperation>
void tests(queue &Q, T Identity, T Init, BinaryOperation BOp, range<1> Range) {
NumErrors += test<Name>(Q, Identity, Init, BOp, Range);
}

Choose a reason for hiding this comment

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

nit, if possible:

template <typename Name, typename... ArgTys>
void tests(ArgTys &&Args) {
  NumErrors += test<Name>(std::forward<ArgTys>(Args)...);
}

Same in other places.

@@ -48,6 +53,22 @@ int main() {

tests<class C1>(Q, CustomVec<long long>(0), CustomVec<long long>(99),
CustomVecPlus<long long>{}, range<2>{33, MaxWGSize});
tests<class C2>(Q, CustomVec<long long>(99), CustomVecPlus<long long>{},

Choose a reason for hiding this comment

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

Do we really need to keep the one with the identity param? Especially given the new test cases below to test that path.

Copy link
Author

Choose a reason for hiding this comment

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

I think it makes sense to test for custom types to make sure that runtime identities also work correctly for custom types.

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

/verify with intel/llvm#8562

1 similar comment
@steffenlarsen
Copy link
Author

/verify with intel/llvm#8562

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

Some test cases are disabled temporarily due to a known bug in reductions. A patch for this will follow after intel/llvm#8562.

@steffenlarsen
Copy link
Author

Verified with intel/llvm#8562.

@steffenlarsen steffenlarsen merged commit 89ae917 into intel:intel Mar 14, 2023
myler added a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
Auto pulldown and update tc files for xmain-cand branch on 20230313
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
This commit adds test cases to existing tests for testing reductions
where the identity is unspecified and unknown.

---------

Signed-off-by: Larsen, Steffen <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
)

This commit adds test cases to existing tests for testing reductions
where the identity is unspecified and unknown.

---------

Signed-off-by: Larsen, Steffen <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants