[MPSHooks] Release pending command encoder #164093
Conversation
Before returning a comand buffer, as subsequent calle are very likely to allocate their own encoder, which results in the following runtime error ``` tryCoalescingPreviousComputeCommandEncoderWithConfig:nextEncoderClass:]:1090: failed assertion `A command encoder is already encoding to this command buffer' ``` Fixes #163721 [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/164093
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 4331f79 with merge base 6ba83e0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Before returning a comand buffer, as subsequent calle are very likely to allocate their own encoder, which results in the following runtime error ``` tryCoalescingPreviousComputeCommandEncoderWithConfig:nextEncoderClass:]:1090: failed assertion `A command encoder is already encoding to this command buffer' ``` Fixes #163721 ghstack-source-id: b214852 Pull Request resolved: #164093
To avoid regression on MacOS 26 Fixes #164093 [ghstack-poisoned]
|
|
||
| void mps_add_one_new_encoder(const at::Tensor& input) { | ||
| using namespace at::native::mps; | ||
| TORCH_CHECK(input.is_mps()); |
There was a problem hiding this comment.
Use TORCH_CHECK_VALUE for these two checks
There was a problem hiding this comment.
It does not matter, as this is an internal test, I'm unsure why they contain those checks to begin with, as they are tests no examples
|
@pytorchbot merge -f "Lint + MPS are green" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@pytorchbot cherry-pick --onto release/2.9 --c critical |
Before returning a comand buffer, as subsequent calle are very likely to allocate their own encoder, which results in the following runtime error ``` tryCoalescingPreviousComputeCommandEncoderWithConfig:nextEncoderClass:]:1090: failed assertion `A command encoder is already encoding to this command buffer' ``` Added regression test to `test_mps_extension` Please note, that `torch::mps::get_command_buffer()` should be called with dispatch_queue held, both before and after this change, but many implementations skip that Fixes #163721 Pull Request resolved: #164093 Approved by: https://github.com/atalman, https://github.com/Skylion007 (cherry picked from commit 8f32adc)
Cherry picking #164093The cherry pick PR is at #164365 and it is recommended to link a critical cherry pick PR with an issue. The following tracker issues are updated: Details for Dev Infra teamRaised by workflow job |
[MPSHooks] Release pending command encoder (#164093) Before returning a comand buffer, as subsequent calle are very likely to allocate their own encoder, which results in the following runtime error ``` tryCoalescingPreviousComputeCommandEncoderWithConfig:nextEncoderClass:]:1090: failed assertion `A command encoder is already encoding to this command buffer' ``` Added regression test to `test_mps_extension` Please note, that `torch::mps::get_command_buffer()` should be called with dispatch_queue held, both before and after this change, but many implementations skip that Fixes #163721 Pull Request resolved: #164093 Approved by: https://github.com/atalman, https://github.com/Skylion007 (cherry picked from commit 8f32adc) Co-authored-by: Nikita Shulga <nikita.shulga@gmail.com>
Stack from ghstack (oldest at bottom):
Before returning a comand buffer, as subsequent calle are very likely to allocate their own encoder, which results in the following runtime error
Added regression test to
test_mps_extensionPlease note, that
torch::mps::get_command_buffer()should be called with dispatch_queue held, both before and after this change, but many implementations skip thatFixes #163721