Skip to content

[NFC][SYCL][Graph] Use raw node_impl * in MRoots/MSchedule #19350

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

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from

Conversation

aelovikov-intel
Copy link
Contributor

... and update the code surrounding their uses in the same spirit.

Also adds roots and schedule views (and renames old schedule()->updateSchedule()) similarly to what's been done for successors()/predecessors earlier.

Continuation of
#19295
#19332
#19334

... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner July 8, 2025 17:56
@aelovikov-intel aelovikov-intel requested a review from reble July 8, 2025 17:56
sycl::detail::device_impl &DeviceImpl,
ur_exp_command_buffer_handle_t CommandBuffer,
std::shared_ptr<node_impl> Node) {
ur_exp_command_buffer_sync_point_t exec_graph_impl::enqueueNodeDirect(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and enqueue are called at around line 870, with shared_ptrs removed from that area of the code. I could have used shared_from_this() and kept these two the same, but they don't seem do be called anywhere else and it made sense to me to update them as part of this PR.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
Part of the refactoring to eliminate `std::weak_ptr<node_impl>` and
reduce usage of `std::shared_ptr<node_impl>` by preferring raw ptr/ref.
Previous PRs in the series:
intel#19295
intel#19332
intel#19334
intel#19350

* Accept `Deps` as `nodes_range` in `graph_impl::add`
* Return `node_impl &` from `graph_impl::add`
* Add `node` support in `nodes_range` and use that together with
  modified `graph_impl::add` when created new `node_impl`s based on
  `std::vector<node> Deps` to avoid creation of temporary `DepImpls`
  storage.
* Also updated `registerSuccessor/registerPredecessor` and
  `addEventForNode/addDepsToNode` to accept raw `node_impl &` as the
  changes above resulted in having raw reference at the call sites.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
Part of the refactoring to eliminate `std::weak_ptr<node_impl>` and
reduce usage of `std::shared_ptr<node_impl>` by preferring raw ptr/ref.
Previous PRs in the series:
intel#19295
intel#19332
intel#19334
intel#19350

* Accept `Deps` as `nodes_range` in `graph_impl::add`
* Return `node_impl &` from `graph_impl::add`
* Add `node` support in `nodes_range` and use that together with
  modified `graph_impl::add` when created new `node_impl`s based on
  `std::vector<node> Deps` to avoid creation of temporary `DepImpls`
  storage.
* Also updated `registerSuccessor/registerPredecessor` and
  `addEventForNode/addDepsToNode` to accept raw `node_impl &` as the
  changes above resulted in having raw reference at the call sites.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
Part of the refactoring to eliminate `std::weak_ptr<node_impl>` and
reduce usage of `std::shared_ptr<node_impl>` by preferring raw ptr/ref.
Previous PRs in the series:
intel#19295
intel#19332
intel#19334
intel#19350

* Accept `Deps` as `nodes_range` in `graph_impl::add`
* Return `node_impl &` from `graph_impl::add`
* Add `node` support in `nodes_range` and use that together with
  modified `graph_impl::add` when created new `node_impl`s based on
  `std::vector<node> Deps` to avoid creation of temporary `DepImpls`
  storage.
* Also updated `registerSuccessor/registerPredecessor` and
  `addEventForNode/addDepsToNode` to accept raw `node_impl &` as the
  changes above resulted in having raw reference at the call sites.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
Part of the refactoring to eliminate `std::weak_ptr<node_impl>` and
reduce usage of `std::shared_ptr<node_impl>` by preferring raw ptr/ref.
Previous PRs in the series:
intel#19295
intel#19332
intel#19334
intel#19350

* Accept `Deps` as `nodes_range` in `graph_impl::add`
* Return `node_impl &` from `graph_impl::add`
* Add `node` support in `nodes_range` and use that together with
  modified `graph_impl::add` when created new `node_impl`s based on
  `std::vector<node> Deps` to avoid creation of temporary `DepImpls`
  storage.
* Also updated `registerSuccessor/registerPredecessor` and
  `addEventForNode/addDepsToNode` to accept raw `node_impl &` as the
  changes above resulted in having raw reference at the call sites.
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.

1 participant