Skip to content

[SYCL][Graph] E2E tests for SYCL Graphs (4/4) #10216

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

Merged
merged 23 commits into from
Jul 28, 2023

Conversation

Bensuo
Copy link
Contributor

@Bensuo Bensuo commented Jul 5, 2023

E2E Tests for SYCL Graphs

This is the fourth patch of a series that adds support for an experimental command graph extension

A snapshot of the complete work can be seen in draft PR #9375 which has support all the specification defined ways of
adding nodes and edges to the graph, including both Explicit and Record & Replay graph construction. The two types of nodes currently implemented are kernel execution and memcpy commands.

See https://github.com/reble/llvm#implementation-status for the status of our total work.

Scope

This fourth patch focuses on adding E2E tests for SYCL Graphs, covering the following:

  • Record and Replay API based tests.
  • Explicit API based tests.
  • Thread safety tests.
  • A small amount of miscellaneous tests.

Following Split PRs

Future follow-up PRs with the remainder of our work on the extension will include:

  • NFC changes - Design doc.

Authors

Co-authored-by: Pablo Reble pablo.reble@intel.com
Co-authored-by: Julian Miller julian.miller@intel.com
Co-authored-by: Ben Tracy ben.tracy@codeplay.com
Co-authored-by: Ewan Crawford ewan@codeplay.com
Co-authored-by: Maxime France-Pillois maxime.francepillois@codeplay.com

- Adds E2E tests for SYCL Graphs
@Bensuo Bensuo temporarily deployed to aws July 5, 2023 14:49 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 5, 2023 15:22 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 5, 2023 16:25 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 5, 2023 16:58 — with GitHub Actions Inactive
@reble reble temporarily deployed to aws July 12, 2023 20:30 — with GitHub Actions Inactive
@reble reble temporarily deployed to aws July 12, 2023 21:24 — with GitHub Actions Inactive
Bensuo added 2 commits July 14, 2023 10:59
Matching implementation change to specification PR
#255
- Fix an issue in the inconsistent device test where if a CUDA device was selected the test would terminate due to the CUDA backend stubs
- Adjusted to always use the level zero device for making the command buffer
@Bensuo Bensuo temporarily deployed to aws July 14, 2023 10:06 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 14, 2023 11:06 — with GitHub Actions Inactive
@EwanC EwanC temporarily deployed to aws July 17, 2023 09:20 — with GitHub Actions Inactive
@EwanC EwanC temporarily deployed to aws July 17, 2023 09:59 — with GitHub Actions Inactive
CI has discovered some failing Graphs E2E tests on Windows.
Either mark them XFAIL or skip them until we can investigate
and fix.
@EwanC EwanC temporarily deployed to aws July 17, 2023 15:08 — with GitHub Actions Inactive
@EwanC EwanC marked this pull request as ready for review July 17, 2023 15:35
@EwanC EwanC requested a review from a team as a code owner July 17, 2023 15:35
@EwanC EwanC requested a review from aelovikov-intel July 17, 2023 15:35
@aelovikov-intel
Copy link
Contributor

I believe @steffenlarsen has been reviewing graph PRs, so it would make sense for him to look at this one as well.

@EwanC EwanC temporarily deployed to aws July 17, 2023 16:43 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 26, 2023 15:05 — with GitHub Actions Inactive
- Unify tests where possible.
- Test code moved to Inputs folder and actual lit tests define an API and include the input file
- Added missing tests for some APIs which have a counterpart in the other and are not API specific
- New common functions which are API agnostic for adding diamond dependency nodes
- New API agnostic function for adding single nodes
- API selected by defining GRAPH_E2E_<EXPLICIT/RECORD_REPLAY> before including test input

Some tests have been removed as being unnecessary due to testing almost identical functionality to other tests. Also removed the reduction regression test since reductions are pretty well covered in the E2E tests already and they were redundant.

---------

Co-authored-by: Pablo Reble <pablo.reble@intel.com>
@reble reble temporarily deployed to aws July 26, 2023 21:31 — with GitHub Actions Inactive
@reble reble temporarily deployed to aws July 26, 2023 22:12 — with GitHub Actions Inactive
@reble reble temporarily deployed to aws July 27, 2023 03:50 — with GitHub Actions Inactive
@reble reble temporarily deployed to aws July 27, 2023 04:29 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 13:57 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 14:25 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 15:06 — with GitHub Actions Inactive
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Only one small open, but otherwise it looks good!

@Bensuo Bensuo temporarily deployed to aws July 28, 2023 16:11 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 16:51 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 17:01 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 28, 2023 17:02 — with GitHub Actions Inactive
@Bensuo
Copy link
Contributor Author

Bensuo commented Jul 28, 2023

The pre-commit workflow failure on HIP appears to be a CI wide issue affecting multiple PRs and unrelated to this PR specifically. I've reported the issue in #10615

@steffenlarsen
Copy link
Contributor

Since none of the tests are intended to run on HIP, I believe this is good to merge.

@steffenlarsen steffenlarsen merged commit f5bbf33 into intel:sycl Jul 28, 2023
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
# E2E Tests for SYCL Graphs
This is the fourth patch of a series that adds support for an
[experimental command graph
extension](intel#5626)

A snapshot of the complete work can be seen in draft PR intel#9375 which has
support all the specification defined ways of
adding nodes and edges to the graph, including both Explicit and Record
& Replay graph construction. The two types of nodes currently
implemented are kernel execution and memcpy commands.

See https://github.com/reble/llvm#implementation-status for the status
of our total work.

## Scope
This fourth patch focuses on adding E2E tests for SYCL Graphs, covering
the following:

* Record and Replay API based tests.
* Explicit API based tests.
* Thread safety tests.
* A small amount of miscellaneous tests.

## Following Split PRs
Future follow-up PRs with the remainder of our work on the extension
will include:
* NFC changes - Design doc.

## Authors
Co-authored-by: Pablo Reble <pablo.reble@intel.com>
Co-authored-by: Julian Miller <julian.miller@intel.com>
Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>
Co-authored-by: Ewan Crawford <ewan@codeplay.com>
Co-authored-by: Maxime France-Pillois
<maxime.francepillois@codeplay.com>
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.

6 participants