Skip to content

[NFC][SYCL][Graph] Update some maps to use raw node_impl * #19334

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 3 commits into from
Jul 8, 2025

Conversation

aelovikov-intel
Copy link
Contributor

Continuation of the refactoring in

#19295
#19332

Part of refactoring to get rid of most (all?) `std::weak_ptr<node_impl>`
and some of `std::shared_ptr<node_impl>` started in
intel#19295. Use `nodes_range` from that PR
to implement `successors`/`predecessors` views and update read-only
accesses to the successors/predecessors to go through them.

I'm not changing the data members `MSuccessors`/`MPredecessors` yet
because it would affect unittests. I'd prefer to refactor most of the
code in future PRs before making that change and updating unittests in
one go.

I'm updating some APIs to accept `node_impl &` instead of
`std::shared_ptr` where the change is mostly localized to the callers
iterating over successors/predecessors and doesn't spoil into other code
too much. For those that weren't updated here we (temporarily) use
`shared_from_this()` but I expect to eliminate those unnecessary copies
when those interfaces will be updated in the subsequent PRs.
@aelovikov-intel
Copy link
Contributor Author

Please ignore first commit during review, this is based on top of #19332 that hasn't been merged in yet.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
@aelovikov-intel aelovikov-intel merged commit 0bba746 into intel:sycl Jul 8, 2025
37 of 38 checks passed
@aelovikov-intel aelovikov-intel deleted the maps branch July 8, 2025 17:36
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
... and update the code surrounding their uses in the same spirit.

Continuation of
intel#19295
intel#19332
intel#19334
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.
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 8, 2025
aelovikov-intel added a commit that referenced this pull request Jul 9, 2025
)

... 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
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jul 9, 2025
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.

3 participants