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

Add test for value_type alias in vector_api #879

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Add test for value_type alias in vector_api #879

merged 2 commits into from
Mar 21, 2024

Conversation

lbushi25
Copy link
Contributor

This PR adds testing logic to verify that the type described by the vec::value_type alias is the same as the underlying type of the vector.

@lbushi25 lbushi25 requested a review from a team as a code owner March 11, 2024 19:22
@CLAassistant
Copy link

CLAassistant commented Mar 11, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@lbushi25 lbushi25 closed this Mar 12, 2024
@lbushi25 lbushi25 reopened this Mar 12, 2024
keryell
keryell approved these changes Mar 21, 2024
View reviewed changes
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Good catch!

.github/workflows/cts_ci.yml Outdated
@@ -56,7 +56,7 @@ jobs:
matrix:
include:
- sycl-impl: dpcpp
version: 1dbee22f9c8a3a825deb871bab76937e04fa26fc
version: 33e5b101fadc4259135231f99ab62d077b284810
Copy link
Member

Choose a reason for hiding this comment

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

Is this related to the issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

@keryell, DPC++ version uplift is required to pass updated test. The version we use today doesn't pass as you can see in pre-commit results.

@lbushi25, DPC++ version uplift can be validated only for the local branches. I'll open a new PR using a local branch to validate the uplift status.

Copy link
Contributor

Choose a reason for hiding this comment

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

#880 should carry the required changes to intel/llvm DPCPP.

@lbushi25 lbushi25 closed this Mar 21, 2024
@lbushi25 lbushi25 reopened this Mar 21, 2024
@bader
Copy link
Contributor

bader commented Mar 21, 2024

@lbushi25, please, update your local branch to make sure that #880 is there.

@lbushi25
Copy link
Contributor Author

@lbushi25, please, update your local branch to make sure that #880 is there.

Thanks for the heads up, I went ahead and updated it!

@lbushi25
Copy link
Contributor Author

@bader ping for merge!

@bader bader merged commit 7b5332e into KhronosGroup:SYCL-2020 Mar 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader left review comments

@steffenlarsen steffenlarsen steffenlarsen left review comments

@keryell keryell keryell 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.
5 participants
@lbushi25 @CLAassistant @bader @keryell @steffenlarsen