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.

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

[SYCL][NFC] Drop Gen9 detection from E2E tests #14452

Merged

Conversation

AlexeySachkov
Copy link
Contributor

Gen9 HW is not officially supported anymore by our product, we don't have such machines in our CI and therefore it doesn't make sense to keep those legacy LIT features and their usage.

Gen9 HW is not officially supported anymore by our product, we don't
have such machines in our CI and therefore it doesn't make sense to keep
those legacy LIT features and their usage.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm submitting this as a draft for now to wait for #14451 to reduce amount of conflicts. However, I think that it worth to do an early review of removed files.

@intel/dpcpp-esimd-reviewers, could you please take a look at them? I simply removed them because they require gen9 and nothing else, but I'm not that familiar with ESIMD to say for sure that it was a correct decision and we shouldn't rewrite those tests instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

im gonna add new tests for gen12 soon, this test is dead so its okay to remove it

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

esimd lgtm

@AlexeySachkov AlexeySachkov marked this pull request as ready for review July 10, 2024 15:35
@AlexeySachkov AlexeySachkov requested review from a team as code owners July 10, 2024 15:35
Copy link
Contributor

Choose a reason for hiding this comment

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

im gonna add new tests for gen12 soon, this test is dead so its okay to remove it

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@AlexeySachkov
Copy link
Contributor Author

I cannot approve my own PR, but my comments about recent changes were addressed, so I'm fine with merging this.

@steffenlarsen
Copy link
Contributor

Primary changes since approvals were that the bfloat16_example has been split into separate files to avoid simplify the cpu/gpu conditional runs/builds.

@sarnex | @kbenzie | @againull - Would you like to re-review?

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

esimd lgtm

@sarnex sarnex requested a review from a team September 23, 2024 14:05
@steffenlarsen
Copy link
Contributor

Thank you, @sarnex ! Since there were no other reactions, we will proceed with the merge. Please feel free to do a post-commit review and we will address it in a separate patch.

@steffenlarsen steffenlarsen merged commit 004bfb1 into intel:sycl Sep 24, 2024
12 checks passed
@AlexeySachkov
Copy link
Contributor Author

This PR broke post-commit on AMD, the fix is in #15533

@AlexeySachkov AlexeySachkov deleted the private/asachkov/drop-gen9-e2e-tests branch October 9, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants