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][Graph] Enable Graphs with L0 immediate command-list #12279

Merged
merged 13 commits into from
Feb 7, 2024

Conversation

mfrancepillois
Copy link
Contributor

Updates e2e tests to test L0 immediate command-lists.
Improves filecheck usage in graph E2E tests
Removes test that checks immediate command-list exception.
Updates the spec.

Addresses Issue: #10467

Updates e2e tests to test L0 immediate command-lists.
Improves filecheck usage in graph E2E tests
Removes test that checks immediate command-list exception
Updates the spec.

Addresses Issue: intel#10467
@EwanC
Copy link
Contributor

EwanC commented Jan 9, 2024

Noting that if either of these two PRs merge before this:

Then this PR will need updated so that the new E2E tests introduced use the same method of setting immediate command lists

@EwanC EwanC requested a review from a team as a code owner January 17, 2024 10:49
@EwanC EwanC requested a review from bso-intel January 17, 2024 10:49
Comment on lines 59 to 60
set(UNIFIED_RUNTIME_REPO "https://github.com/bensuo/unified-runtime.git")
set(UNIFIED_RUNTIME_TAG maxime/imm-cmd-list-support)
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to update target UR!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this will be updated as soon as the UR PR is merged to the main branch.

@aarongreig
Copy link
Contributor

oneapi-src/unified-runtime#1218 has merged, please update the tag @mfrancepillois

@mfrancepillois
Copy link
Contributor Author

@intel/llvm-gatekeepers could you please merge this PR?

@sommerlukas sommerlukas merged commit eead989 into intel:sycl Feb 7, 2024
12 checks passed
@sarnex
Copy link
Contributor

sarnex commented Feb 7, 2024

@mfrancepillois Seems we have a lot of post-commit fails in Graph tests on Arc. Can you take a look? If they cannot be fixed quickly I recommend reverting this PR

Failed Tests (16):
  SYCL :: Graph/Explicit/basic_usm.cpp
  SYCL :: Graph/Explicit/basic_usm_host.cpp
  SYCL :: Graph/Explicit/basic_usm_mixed.cpp
  SYCL :: Graph/Explicit/basic_usm_shared.cpp
  SYCL :: Graph/Explicit/queue_constructor_usm.cpp
  SYCL :: Graph/Explicit/sub_graph.cpp
  SYCL :: Graph/RecordReplay/after_use.cpp
  SYCL :: Graph/RecordReplay/barrier_with_work.cpp
  SYCL :: Graph/RecordReplay/basic_usm.cpp
  SYCL :: Graph/RecordReplay/basic_usm_host.cpp
  SYCL :: Graph/RecordReplay/basic_usm_mixed.cpp
  SYCL :: Graph/RecordReplay/basic_usm_shared.cpp
  SYCL :: Graph/RecordReplay/host_task_in_order.cpp
  SYCL :: Graph/RecordReplay/queue_constructor_usm.cpp
  SYCL :: Graph/RecordReplay/sub_graph.cpp
  SYCL :: Graph/RecordReplay/usm_copy.cpp

@sommerlukas
Copy link
Contributor

sommerlukas commented Feb 7, 2024

@mfrancepillois Seems we have a lot of post-commit fails in Graph tests on Arc. Can you take a look? If they cannot be fixed quickly I recommend reverting this PR

Failed Tests (16):
 ...

Hi @sarnex, I'm already in touch with @mfrancepillois, we will follow up with a PR marking those tests unsupported on specific hardware, while the reason for the failure is investigated.

@sarnex
Copy link
Contributor

sarnex commented Feb 7, 2024

@sommerlukas Great thank you!!!

@sommerlukas
Copy link
Contributor

The PR to disable the tests temporarily is here: #12648

ldrumm pushed a commit that referenced this pull request Feb 8, 2024
After PR #12279, the USM tests fail in
post-commit CIs
(https://github.com/intel/llvm/actions/runs/7814201804/job/21315560479).
We temporarily disable these tests during the bug investigation.
@mfrancepillois
Copy link
Contributor Author

The PR #12677 (linked to the UR PR oneapi-src/unified-runtime#1328 ) fixes the bug.

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.

8 participants