Skip to content

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
Ewan Crawford 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 <ben.tracy@codeplay.com>
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.

Co-authored-by: Ben Tracy <ben.tracy@codeplay.com>
@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.

4 participants