Skip to content

Avoid DDE in narrow with unbacked start#166361

Closed
laithsakka wants to merge 19 commits into
gh/laithsakka/316/basefrom
gh/laithsakka/316/head
Closed

Avoid DDE in narrow with unbacked start#166361
laithsakka wants to merge 19 commits into
gh/laithsakka/316/basefrom
gh/laithsakka/316/head

Conversation

@laithsakka
Copy link
Copy Markdown
Contributor

@laithsakka laithsakka commented Oct 28, 2025

Stack from ghstack (oldest at bottom):

Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate,
for that case we shall pass dim_size instead of start+length

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @jerryzh168 @aditew01 @voznesenskym @penguinwu @EikanWang @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Oct 28, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/166361

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit da51cb7 with merge base 08ef852 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

laithsakka added a commit that referenced this pull request Oct 28, 2025
ghstack-source-id: 2e19823
Pull Request resolved: #166361
@laithsakka laithsakka marked this pull request as draft October 28, 2025 06:13
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice
but we have to write the check carefully.  

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Oct 28, 2025
ghstack-source-id: 8720bf2
Pull Request resolved: #166361
@laithsakka laithsakka marked this pull request as ready for review October 28, 2025 06:29
@laithsakka laithsakka requested a review from aditvenk October 28, 2025 06:43
@laithsakka laithsakka added the topic: not user facing topic category label Oct 28, 2025
@laithsakka laithsakka marked this pull request as draft October 28, 2025 16:36
@laithsakka laithsakka closed this Oct 28, 2025
@laithsakka laithsakka reopened this Oct 28, 2025
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice
but we have to write the check carefully.  

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Oct 29, 2025
ghstack-source-id: 9c71f93
Pull Request resolved: #166361
@pytorch-bot pytorch-bot Bot added ciflow/inductor module: cpu CPU specific problem (e.g., perf, algorithm) module: inductor labels Oct 29, 2025
@laithsakka laithsakka requested a review from aditvenk October 29, 2025 00:48
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice
but we have to write the check carefully.  

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Oct 29, 2025
ghstack-source-id: 099dd50
Pull Request resolved: #166361
@laithsakka laithsakka marked this pull request as ready for review October 29, 2025 01:07
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice
but we have to write the check carefully.  

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
pytorch-bot Bot pushed a commit that referenced this pull request Nov 4, 2025
This reverts commit c761999.

Reverted #166361 on behalf of https://github.com/pytorch-auto-revert due to Reverted automatically by pytorch's autorevert, to avoid this behaviour add the tag autorevert: disable ([comment](#166361 (comment)))
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate, 
for that case we shall pass dim_size instead of start+length 

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 aditew01 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Nov 4, 2025
ghstack-source-id: df15da6
Pull Request resolved: #166361
@laithsakka
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Nov 4, 2025
@malfet
Copy link
Copy Markdown
Contributor

malfet commented Nov 5, 2025

@pytorchbot revert -m "Looks like it broke test_torchfuzz subtests, see https://hud.pytorch.org/hud/pytorch/pytorch/01e6e35c7faf913c3a85c7a64d2939cfa768358a/1?per_page=50&name_filter=trunk&mergeEphemeralLF=true" -c nosignal

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Reverting PR 166361 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit ed45c5f38df6aa419c67d139d932c2c94404223a returned non-zero exit code 1

Auto-merging aten/src/ATen/native/TensorShape.cpp
Auto-merging test/test_dynamic_shapes.py
CONFLICT (content): Merge conflict in test/test_dynamic_shapes.py
error: could not revert ed45c5f38df... Avoid DDE in narrow with unbacked start (#166361)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Details for Dev Infra team Raised by workflow job

@malfet
Copy link
Copy Markdown
Contributor

malfet commented Nov 5, 2025

@pytorchbot revert -m "Looks like it broke test_torchfuzz subtests, see https://hud.pytorch.org/hud/pytorch/pytorch/01e6e35c7faf913c3a85c7a64d2939cfa768358a/1?per_page=50&name_filter=trunk&mergeEphemeralLF=true" -c nosignal

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@laithsakka your PR has been successfully reverted.

Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate, 
for that case we shall pass dim_size instead of start+length 

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 aditew01 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Nov 5, 2025
ghstack-source-id: 6a48fb4
Pull Request resolved: #166361
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate, 
for that case we shall pass dim_size instead of start+length 

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 aditew01 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
@laithsakka
Copy link
Copy Markdown
Contributor Author

rebase on main

laithsakka added a commit that referenced this pull request Nov 5, 2025
ghstack-source-id: 6313e4d
Pull Request resolved: #166361
Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate, 
for that case we shall pass dim_size instead of start+length 

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 aditew01 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
laithsakka added a commit that referenced this pull request Nov 5, 2025
ghstack-source-id: eae5112
Pull Request resolved: #166361
@laithsakka
Copy link
Copy Markdown
Contributor Author

ensure internals has no issues https://www.internalfb.com/diff/D86270654

Slice knows how to handle unbacked start, we do not need to offset start before calling slice, we can leave it for slice.
The only edge case is when start<0 and start+length ==0 in that case slice and narrow would deviate, 
for that case we shall pass dim_size instead of start+length 

cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 jerryzh168 aditew01 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben

[ghstack-poisoned]
@laithsakka
Copy link
Copy Markdown
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td Do not run TD on this PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: inductor Reverted topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants