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

Open
wants to merge 9 commits into
base: sycl
Choose a base branch
from

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

@@ -13,16 +13,16 @@
// RUN: %clangxx -fsycl -fsycl-targets=spir64 %s -o %t.out
// RUN: %{run} %t.out

// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend "-device gen9" %s -o %t.out
// RUN: %clangxx -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend "-device gen12lp" %s -o %t.out
// RUN: %{run} %t.out
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to me like this is GPU AOT, so it should be limited to only run on GPU.

Suggested change
// RUN: %{run} %t.out
// RUN: %if gpu %{ %{run} %t.out %}

@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.

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