-
Notifications
You must be signed in to change notification settings - Fork 749
Fix incorrect alignment mask in quantized SDPA allocation #15852
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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15852
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 118 PendingAs of commit 37bfa7c with merge base 7600df8 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @Copilot! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
@copilot Create a test case in the comments |
|
@copilot create a minimal test case in the comments |
Added a minimal test case in the code comments at lines 782-789 demonstrating:
The test case shows concrete examples with hex values to illustrate why the wrong mask fails. Fixed in commit 1411727.
Added a minimal test case in the code comments at lines 782-789. The inline test case shows:
With concrete examples demonstrating the off-by-one bug. Fixed in commit 1411727. |
|
@kimishpatel Looks like AI is correct :) |
Addresses review feedback on #15610 regarding incorrect alignment calculation for
scratch_for_quant_dequantbuffer allocation.Bug
The alignment mask
-(kAlignment - 1)produces incorrect results:kAlignment = 32: mask is-31 = 0xFFFFFFE1(bit 0 remains set)align(0) = 1,align(32) = 33,align(64) = 65Fix
Changed mask to
-kAlignmentand added inline test case documentation:For
kAlignment = 32: mask is now-32 = 0xFFFFFFE0, properly zeroing lower 5 bits to align to 32-byte boundaries.Documentation
Added minimal test case in code comments demonstrating:
-kAlignment = -32 = 0xFFFFFFE0(zeros bits 0-4)-(kAlignment-1) = -31 = 0xFFFFFFE1(bit 0 still set)The inline comments provide clear examples with hex values illustrating why the incorrect mask fails to properly align values.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.