Skip to content

[PyBindings] Wait for the device instead of per-result flags, and fix bfloat16 result layout and input errors - #23999

Open
shoumikhin wants to merge 1 commit into
pytorch:mainfrom
shoumikhin:pybindings-cuda-wait-followup
Open

shoumikhin wants to merge 1 commit into
pytorch:mainfrom
shoumikhin:pybindings-cuda-wait-followup

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to the Python bindings change that added torch-free execution and device memory support.

What was wrong

A CUDA result borrowed from a method, through get_outputs(clone_outputs=False) or a method that returns its own input, shares the method's memory. The bindings tracked "might another library still be writing this?" with a flag on each result object, and every borrowed result started with a fresh flag. So when another library wrote the memory through one result, and a second view of the same memory was passed back in, the call could read the old data.

A bfloat16 result on the host, passed back in, was read in the method's layout instead of its own. A result in another dim order came back permuted, with no error.

What this changes

The flags are replaced by device waits. Before a call runs, it waits once per device for its CUDA inputs, including results of these bindings. A method also waits for the device of an output buffer it writes again, since another library may still be reading a dropped result there. After a call, it waits once per output device. __dlpack__ and to_host() on a CUDA result always wait. In bindings built without CUDA, __dlpack__ now waits through torch, as calls already do, instead of failing.

A result of these bindings passed back in always takes the DLPack path, which keeps its dtype, shape and strides. This fixes the bfloat16 layout bug, and a bfloat16 result now keeps its own shape instead of taking the method's.

A failed device allocation no longer turns off the no-copy output return. A clone_outputs=False read is now its own borrowed view.

An error raised while reading an input keeps its Python type. A Python exception gets the input and method as a note, and C++ errors name them in the message.

The torch-free CI job now also builds a channels-last bfloat16 program, so the layout test runs there.

Tested

On Linux x86_64 with H100 GPUs, built the bindings with CUDA, without CUDA, and as the standalone torch-free module. Ran test_pybindings_cuda_dlpack.py, test_pybindings_no_aten.py, test_pybindings.py and runtime/test/test_runtime.py in both output modes. Every failure there also fails without this change: tests written for the other output mode, two quantized-op tests whose kernels are not registered here, and three subprocess tests that need the CUDA runtime library outside torch. Each new or changed test fails with its fix removed.

Copilot AI balanced review requested due to automatic review settings October 19, 2026 18:20
@shoumikhin shoumikhin added the release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.) label Oct 19, 2026
@pytorch-bot

pytorch-bot Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

⏳ 3 Pending, 2 Unrelated Failures

As of commit 9868f97 with merge base b1beef3 (image):

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

… bfloat16 result layout and input errors

A CUDA result borrowed from a method, through get_outputs(clone_outputs=False)
or a method that returns its own input, shares the method's memory. Whether
another library might still be writing that memory was tracked with a flag on
each result object, and every borrowed result started with a fresh flag. So
when another library wrote the memory through one result, and a second view of
the same memory was passed back in, the call could read the data from before
that write.

The flags are gone. Before a call runs, it now waits once for each device that
holds one of its CUDA inputs, including results of these bindings. A method
also waits for the device of each output buffer it writes again, because
another library may still be reading a dropped result that used it. After the
call runs, it waits once for each device that holds an output, as before.
Handing a CUDA result to another library through __dlpack__ or to_host() always
waits too. In bindings built without CUDA, __dlpack__ now waits through torch,
as calls already do, instead of failing.

A result of these bindings passed back in now always takes the DLPack path,
which keeps its real dtype, shape and strides. A bfloat16 result used to take
the buffer path, whose flat bytes were read in the method's layout, so a result
in another dim order came back permuted with no error. A bfloat16 result now
also keeps its own shape instead of taking the method's.

A failed device allocation no longer turns off the no-copy output return for
the rest of a method's life, because the new buffer is allocated into a local
first. A clone_outputs=False read is now its own borrowed view, not the object
a default read handed over.

An error raised while reading an input keeps its Python type. A Python
exception gets the input and method as a note instead of being rebuilt, which
failed for types such as UnicodeDecodeError. C++ runtime errors, the device
checks and the unsupported-type error name the input and method in their
message, and std::bad_alloc still becomes MemoryError.
@shoumikhin
shoumikhin force-pushed the pybindings-cuda-wait-followup branch from 2e4bd22 to 9868f97 Compare October 20, 2026 06:17
Copilot AI balanced review requested due to automatic review settings October 20, 2026 06:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@shoumikhin shoumikhin changed the title [PyBindings] Wait for the device around every call, and fix result layout and error types [PyBindings] Wait for the device instead of per-result flags, and fix bfloat16 result layout and input errors Oct 20, 2026

This branch has not been deployed

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: api Changes to public facing apis (any interfaces, pybinded runtime methods, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants