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] Remove CGF reuse in graph nodes #87

Merged
merged 9 commits into from
Mar 23, 2023
Merged

Conversation

Bensuo
Copy link
Collaborator

@Bensuo Bensuo commented Mar 6, 2023

  • Removes use of the CGF when submitting graph nodes.
  • Handler info is extracted and copied into nodes.
  • Node execution is now done manually rather than going through the handler.
  • Adding nodes in record and replay moved to handler::finalize.
  • Workarounds for reduction wg sizes added.

Note reductions are broken by this commit due to missing accessor support which will be addressed in a future PR.

@Bensuo Bensuo added the Graph Implementation Related to DPC++ implementation and testing label Mar 6, 2023
@Bensuo Bensuo requested review from EwanC, julianmi and reble March 6, 2023 14:18
@EwanC
Copy link
Collaborator

EwanC commented Mar 7, 2023

Would be good to have a test case with this PR to show if #49 was fixed.

@EwanC EwanC force-pushed the ben/cgf-refactor branch from f32f745 to c0d9bd4 Compare March 13, 2023 17:08
@EwanC
Copy link
Collaborator

EwanC commented Mar 13, 2023

@reble I've rebased this after your naming convention changes and renamed the variables to use the uppercase LLVM convention

Bensuo and others added 5 commits March 17, 2023 10:01
- Removes use of the CGF when submitting graph nodes.
- Handler info is extracted and copied into nodes
- Adding nodes in record and replay moved to finalize.
- Workarounds for reduction wg sizes added.
- Note reductions are broken by this commit due to missing accessor support
Introduce a test case which fails before this commit
and passes afterwards. Based on #49
* Comment where hardcoded defaults came from
* Use `static_cast` rather than c-style cast
* clang-format new test
@EwanC EwanC force-pushed the ben/cgf-refactor branch from c0d9bd4 to 20bb1fb Compare March 17, 2023 10:02
EwanC added 2 commits March 21, 2023 09:26
Instead of USM arguments, it is buffer accessors that should be used for
edge detection. Fixes `graph-explicit-node-ordering.cpp` test ordering which is currently
creating incorrect extra edges

Also added a test for explicit API with accessor edges, we can use to see if this
logic works once accessors are better supported.

* Defined macro ``TEST_GRAPH_REDUCTIONS` for use in tests with reductions
  to enable that codepath, otherwise it is undefined.
Make test consistent with other tests by using asserts
rather than printing to std out.
@EwanC EwanC self-assigned this Mar 21, 2023
This change adds a new handler constructor which takes
a graph, rather than creating a default temporary queue object
to pass to the existing constructor.

Co-authored-by: Ben Tracy <[email protected]>
Copy link
Collaborator Author

@Bensuo Bensuo left a comment

Choose a reason for hiding this comment

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

LGTM just a minor typo suggestion.

@EwanC EwanC deleted the ben/cgf-refactor branch June 28, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Implementation Related to DPC++ implementation and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants