Skip to content

Document and test the memory planning a device-activation export needs - #21703

Open
shoumikhin wants to merge 1 commit into
mainfrom
gh/shoumikhin/99/head
Open

Document and test the memory planning a device-activation export needs#21703
shoumikhin wants to merge 1 commit into
mainfrom
gh/shoumikhin/99/head

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

The problem

ExecuTorch can export a model so its activations stay on the GPU. You hand over memory
that is already on the GPU, the model runs on it, and the output stays there. No copying
back and forth, which is the point of the feature.

Two settings control this, and they live in different places. The first says "do not
insert copies at the delegate boundary":

PropagateDeviceConfig(
    skip_h2d_for_method_inputs=True,
    skip_d2h_for_method_outputs=True,
)

The second is memory planning, which by default reserves a buffer for every graph input
and output. That is the right default for an ordinary model, and wrong here: the runtime
reserves its own GPU buffer for an input you already allocated, and then fills it by
copying your memory into it. The copy the export asked to skip comes back at run time.

It is worse than a wasted copy. For device memory that copy is a plain host memcpy
into a GPU pointer, which is undefined and crashes the process with no message.

Nothing said the two settings had to be paired, and no test exported such a program and
checked it. The result is a documented feature whose documented usage does not work.

The change

Documentation and a test. No behavior change.

The configuration now states the pairing, with a complete example:

ExecutorchBackendConfig(
    propagate_device_config=PropagateDeviceConfig(
        skip_h2d_for_method_inputs=True,
        skip_d2h_for_method_outputs=True,
    ),
    enable_non_cpu_memory_planning=True,
    memory_planning_pass=MemoryPlanningPass(
        alloc_graph_input=False, alloc_graph_output=False
    ),
)

The measurable difference, for a model with two small inputs:

graph inputs allocated (default):  768 planned bytes, the runtime copies into its own buffer
graph inputs not allocated:        256 planned bytes, the runtime points at the caller's memory

The extra 512 bytes are a duplicate of the caller's two inputs.

Test plan

Added test_skipping_copies_requires_unallocated_graph_io, which pins the pairing:
with default planning the graph inputs are planned, and with the pairing none of the
inputs or outputs are. It needs no GPU, because it inspects the exported program rather
than running it.

Confirmed the test fails without the pairing:

AssertionError: True is not false : no graph input should be planned
when the caller provides it, got [True, True]

Ran the full test_propagate_device_pass file, 24 tests, all passing.

Separately verified on hardware that a program exported with the pairing runs correctly
from C++ with GPU memory in and out, matching eager PyTorch exactly on two different GPU
architectures.

[ghstack-poisoned]
@shoumikhin

shoumikhin commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Copilot AI lite review requested due to automatic review settings August 9, 2026 23:15
@pytorch-bot

pytorch-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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 499bd94 with merge base 730b77a (image):

BROKEN TRUNK - The following job 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 Aug 9, 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.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants