[TRANSFORMATIONS] Destroy constants after replacement in constant folding#33961
[TRANSFORMATIONS] Destroy constants after replacement in constant folding#33961mryzhov wants to merge 13 commits intoopenvinotoolkit:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modifies the constant folding pass to use a deque-based iteration approach instead of a range-based for loop, allowing for dynamic modification of the node processing queue. The change also adds an early exit condition in the can_constant_fold method to check if a node has an evaluate method before attempting constant folding.
Changes:
- Modified the main loop in
ConstantFolding::run_on_modelto use a deque instead of iterating directly over ordered operations - Added
has_evaluate()check tocan_constant_foldto prevent folding attempts on nodes without evaluation capability
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/core/src/pass/constant_folding.cpp | Replaced range-based for loop with deque-based iteration pattern, adding explicit pop_front() calls |
| src/core/src/node.cpp | Added early return condition in can_constant_fold to check for evaluate method existence |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Details:
model->get_ordered_ops()to an index-based for loop, usingstd::moveto transfer ownership of each node from thenodesvector. That would allow us to destroy unused constants immediately and minimize the peak memory allocation.Tickets: