-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[dynamo, nested graph breaks] fix RETURN_VALUE tx skipping in nested graph breaks #165808
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
Conversation
…graph breaks [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/165808
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ No FailuresAs of commit 042b331 with merge base a988510 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
… in nested graph breaks" Previously, we would completely skip building and calling any resume function if the leaf frame's resume instruction was RETURN_VALUE/RETURN_CONST. Now, we only skip building/calling resume functions for frames that are resuming on RETURN_VALUE. [ghstack-poisoned]
Lucaskabela
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comments they are helpful in trying to parse out what all these stack operations and instruction pushes do.
As a minor nit, they seem a bit shorthanded still - could we transform them to long hand so new users can onboard to this logic easier? i.e
do the same with cells
becomes
Filter out the cells of skipped tx'es from codegen
… in nested graph breaks" Previously, we would completely skip building and calling any resume function if the leaf frame's resume instruction was RETURN_VALUE/RETURN_CONST. Now, we only skip building/calling resume functions for frames that are resuming on RETURN_VALUE. [ghstack-poisoned]
… in nested graph breaks" Previously, we would completely skip building and calling any resume function if the leaf frame's resume instruction was RETURN_VALUE/RETURN_CONST. Now, we only skip building/calling resume functions for frames that are resuming on RETURN_VALUE. [ghstack-poisoned]
… in nested graph breaks" Previously, we would completely skip building and calling any resume function if the leaf frame's resume instruction was RETURN_VALUE/RETURN_CONST. Now, we only skip building/calling resume functions for frames that are resuming on RETURN_VALUE. [ghstack-poisoned]
|
Starting merge as part of PR stack under #166016 |
…rs; enable nested_graph_breaks in test_ctx_manager.py and test_generator.py (#165809) Generators should not support nested graph breaks. Pull Request resolved: #165809 Approved by: https://github.com/Lucaskabela, https://github.com/guilhermeleobas ghstack dependencies: #166013, #166015, #165808
…graph breaks ghstack-source-id: 0f4e9e1 Pull Request resolved: pytorch/pytorch#165808
…graph breaks ghstack-source-id: 6cf7c57 Pull Request resolved: pytorch/pytorch#165808
Stack from ghstack (oldest at bottom):
Previously, we would completely skip building and calling any resume function if the leaf frame's resume instruction was RETURN_VALUE/RETURN_CONST. Now, we only skip building/calling resume functions for frames that are resuming on RETURN_VALUE.
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames @Lucaskabela