Skip to content

Commit cc4b514

Browse files
authored
Fix device placement of constants, shared inputs and delegate outputs (#24088)
## What is wrong today When a delegate runs on a GPU, `PropagateDevicePass` copies its inputs to the device (`_h2d_copy`) and its outputs back (`_d2h_copy`). `skip_h2d_for_method_inputs` and `skip_d2h_for_method_outputs` remove those copies at the method's edges. 1. A transposed parameter is stored contiguous, but a delegate reading it is compiled for its original strides. `to_edge` does this silently. 2. When a host op writes a delegate output in place, the next delegate on that device reads the old value. Such a `relu_` makes memory planning fail. 3. A lowered module returning one tensor, called from another module, gets no `_d2h_copy`. 4. With the output skip flag, a returned output that something else reads comes back on the host. 5. A constant that feeds a device delegate makes `to_executorch` fail. With the input skip flag, it arrives as host memory marked as device memory. 6. With the input skip flag, an input with more than one reader is refused. 7. A delegate targeting the CPU gets copies that fail at run time. ## What this change does Numbered as above: 1. `to_edge` makes such a parameter contiguous before partitioning, and warns for each tensor whose layout it changes. 2. The host op writes the host copy of the output. The copies stay when it writes between two delegates, also through a view. 3. The output gets its `_d2h_copy`. 4. Returned outputs stay on the device. One that a host op writes in place is returned on the host, with a warning. 5. The constant gets a device copy, with a warning. One neither contiguous nor channels-last is refused. 6. The input stays on the device of its first delegate. Shape-only ops read it there. Other readers share one `_d2h_copy` of it, with a warning. An input written in place on the host is refused. 7. The delegate gets no copies. Also: - A tensor that several delegates on one device read is copied there once, unless written in between. - New option `PropagateDeviceConfig(error_on_per_call_copies=True)` turns the warnings in 4 to 6 into errors when the matching skip flag is set. ## What was tested - Device pass tests and new `exir/tests/test_passes.py` tests pass. All new tests but one fail without their fix. - `backends/cuda/tests/test_device_placement.py` matches eager PyTorch with real CUDA delegates on an H100. The `unittest-cuda` job now runs when these passes change, and fails if that test skips.
1 parent 20d9f8f commit cc4b514

10 files changed

Lines changed: 1955 additions & 97 deletions

File tree

‎.github/workflows/cuda.yml‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ on:
4343
- examples/models/muse-glimmer/**
4444
- extension/pybindings/**
4545
- runtime/__init__.py
46+
- exir/passes/convert_constant_dim_order_pass.py
47+
- exir/passes/insert_write_back_for_buffers_pass.py
48+
- exir/passes/propagate_device_config.py
49+
- exir/passes/propagate_device_pass.py
4650
workflow_dispatch:
4751
inputs:
4852
models:
@@ -469,8 +473,9 @@ jobs:
469473
unittest-cuda:
470474
name: unittest-cuda (${{ matrix.shard }})
471475
needs: [changed-files, run-decision]
472-
# This job runs the pybindings device test, so it also has to fire on the
473-
# two pybindings paths the workflow filter lists.
476+
# This job runs the pybindings device test and the device placement test,
477+
# so it also has to fire on the pybindings and device pass paths the
478+
# workflow filter lists.
474479
if: |
475480
contains(needs.changed-files.outputs.changed-files, 'backends/cuda') ||
476481
contains(needs.changed-files.outputs.changed-files, 'backends/aoti') ||
@@ -481,6 +486,10 @@ jobs:
481486
contains(needs.changed-files.outputs.changed-files, '.ci/scripts/test_model_e2e.sh') ||
482487
contains(needs.changed-files.outputs.changed-files, 'extension/pybindings') ||
483488
contains(needs.changed-files.outputs.changed-files, 'runtime/__init__.py') ||
489+
contains(needs.changed-files.outputs.changed-files, 'exir/passes/convert_constant_dim_order_pass.py') ||
490+
contains(needs.changed-files.outputs.changed-files, 'exir/passes/insert_write_back_for_buffers_pass.py') ||
491+
contains(needs.changed-files.outputs.changed-files, 'exir/passes/propagate_device_config.py') ||
492+
contains(needs.changed-files.outputs.changed-files, 'exir/passes/propagate_device_pass.py') ||
484493
needs.run-decision.outputs.is-full-run == 'true'
485494
# kernel: Triton kernels and their autotuning. system: lowering passes, op
486495
# dispatch, export/runtime, and model tests. Together they do not fit the
@@ -567,6 +576,21 @@ jobs:
567576
echo "::error::a pybindings device test skipped in the CUDA job, which means the CUDA backend or a visible GPU is missing"
568577
exit 1
569578
fi
579+
580+
# Same for the device placement test, which skips without a GPU.
581+
set +e
582+
python -m pytest backends/cuda/tests/test_device_placement.py -v -rs -o "addopts=" \
583+
2>&1 | tee /tmp/device_placement_test.log
584+
device_placement_status=${PIPESTATUS[0]}
585+
set -e
586+
if [ "${device_placement_status}" -ne 0 ]; then
587+
echo "::error::a device placement test failed"
588+
exit "${device_placement_status}"
589+
fi
590+
if grep -qE "[0-9]+ skipped" /tmp/device_placement_test.log; then
591+
echo "::error::a device placement test skipped in the CUDA job, which means the CUDA backend or a visible GPU is missing"
592+
exit 1
593+
fi
570594
fi
571595
572596
# Build ExecuTorch with CUDA support
@@ -589,7 +613,7 @@ jobs:
589613
python -m pytest "${KERNEL_TESTS[@]}" -v -o "addopts=" --durations=20
590614
else
591615
IGNORES=()
592-
for t in "${KERNEL_TESTS[@]}" "${QUANTIZED_GEMM_TESTS[@]}"; do
616+
for t in "${KERNEL_TESTS[@]}" "${QUANTIZED_GEMM_TESTS[@]}" backends/cuda/tests/test_device_placement.py; do
593617
IGNORES+=("--ignore=${t}")
594618
done
595619
python -m pytest backends/cuda/tests backends/cuda/passes/tests -v -o "addopts=" --durations=20 \
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the BSD-style license found in the
5+
# LICENSE file in the root directory of this source tree.
6+
7+
import tempfile
8+
import unittest
9+
from pathlib import Path
10+
11+
import torch
12+
from executorch.backends.cuda.cuda_backend import CudaBackend
13+
from executorch.exir import (
14+
EdgeCompileConfig,
15+
ExecutorchBackendConfig,
16+
to_edge_transform_and_lower,
17+
)
18+
from executorch.exir.backend.compile_spec_schema import CompileSpec
19+
from executorch.exir.backend.partitioner import (
20+
DelegationSpec,
21+
Partitioner,
22+
PartitionResult,
23+
)
24+
from executorch.exir.dialects._ops import ops as exir_ops
25+
from executorch.exir.passes import MemoryPlanningPass
26+
from executorch.exir.passes.propagate_device_pass import (
27+
PropagateDeviceConfig,
28+
TARGET_DEVICE_COMPILE_SPEC_KEY,
29+
)
30+
31+
32+
class EachAddOnCuda(Partitioner):
33+
"""Lowers each add to its own CUDA delegate. Parameters stay outside, so
34+
they cross the delegate boundary as in a partial lowering."""
35+
36+
def partition(self, exported_program) -> PartitionResult:
37+
spec = DelegationSpec(
38+
CudaBackend.__name__,
39+
[
40+
CudaBackend.generate_method_name_compile_spec("forward"),
41+
CompileSpec(TARGET_DEVICE_COMPILE_SPEC_KEY, b"cuda:0"),
42+
],
43+
)
44+
tags = {}
45+
adds = [
46+
node
47+
for node in exported_program.graph_module.graph.nodes
48+
if node.op == "call_function"
49+
and node.target == exir_ops.edge.aten.add.Tensor
50+
]
51+
for i, node in enumerate(adds):
52+
node.meta["delegation_tag"] = f"add{i}"
53+
tags[f"add{i}"] = spec
54+
return PartitionResult(
55+
tagged_exported_program=exported_program, partition_tags=tags
56+
)
57+
58+
59+
def load(model, inputs, directory, config=None):
60+
"""Lower model with EachAddOnCuda and load its forward method from a file
61+
in directory."""
62+
from executorch.runtime import Runtime
63+
64+
edge = to_edge_transform_and_lower(
65+
torch.export.export(model, inputs, strict=True),
66+
partitioner=[EachAddOnCuda()],
67+
compile_config=EdgeCompileConfig(
68+
_check_ir_validity=False, _skip_dim_order=True
69+
),
70+
)
71+
path = Path(directory) / "model.pte"
72+
path.write_bytes(edge.to_executorch(config or ExecutorchBackendConfig()).buffer)
73+
return Runtime.get().load_program(path).load_method("forward")
74+
75+
76+
def run(model, inputs) -> list[torch.Tensor]:
77+
"""Lower model with EachAddOnCuda, run it in the runtime, return its outputs."""
78+
with tempfile.TemporaryDirectory() as directory:
79+
method = load(model, inputs, directory)
80+
return [output.cpu() for output in method.execute(list(inputs))]
81+
82+
83+
@unittest.skipUnless(torch.cuda.is_available(), "CUDA required")
84+
@unittest.skipIf(
85+
torch.version.hip is not None,
86+
"loads through the Python runtime, which the ROCm build does not include",
87+
)
88+
class CudaDevicePlacementTest(unittest.TestCase):
89+
def test_transposed_parameter_read_by_a_delegate(self):
90+
"""The program stores a transposed parameter contiguous and copies
91+
those bytes to the device. to_edge makes the parameter contiguous
92+
before the delegate is compiled, so the delegate expects that layout."""
93+
94+
class Model(torch.nn.Module):
95+
def __init__(self):
96+
super().__init__()
97+
weight = torch.arange(6.0).reshape(3, 2).t()
98+
self.weight = torch.nn.Parameter(weight, requires_grad=False)
99+
100+
def forward(self, x):
101+
return torch.add(x, self.weight)
102+
103+
model = Model()
104+
x = torch.arange(6.0).reshape(2, 3) * 10
105+
(output,) = run(model, (x,))
106+
torch.testing.assert_close(output, model(x))
107+
108+
def test_constant_read_by_two_delegates(self):
109+
"""Two delegates read a constant stored in the program from outside
110+
them, so the program copies it to the device for them."""
111+
112+
class Model(torch.nn.Module):
113+
def __init__(self):
114+
super().__init__()
115+
self.register_buffer("table", torch.arange(6.0).reshape(2, 3))
116+
117+
def forward(self, x):
118+
return torch.add(x, self.table), torch.add(x * 2, self.table)
119+
120+
model = Model()
121+
x = torch.arange(6.0).reshape(2, 3) * 10
122+
for output, expected in zip(run(model, (x,)), model(x), strict=True):
123+
torch.testing.assert_close(output, expected)
124+
125+
def test_input_on_device_also_read_on_host(self):
126+
"""With skip_h2d_for_method_inputs, the caller passes the input on the
127+
device. A delegate reads it there, and a host op reads a copy of it in
128+
host memory."""
129+
130+
class Model(torch.nn.Module):
131+
def forward(self, x, y):
132+
return torch.add(x, y), torch.sin(x)
133+
134+
config = ExecutorchBackendConfig(
135+
propagate_device_config=PropagateDeviceConfig(
136+
skip_h2d_for_method_inputs=True
137+
),
138+
enable_non_cpu_memory_planning=True,
139+
memory_planning_pass=MemoryPlanningPass(alloc_graph_input=False),
140+
)
141+
model = Model()
142+
inputs = (torch.randn(2, 3), torch.randn(2, 3))
143+
with tempfile.TemporaryDirectory() as directory:
144+
method = load(model, inputs, directory, config)
145+
outputs = method.execute([t.cuda() for t in inputs])
146+
for output, expected in zip(outputs, model(*inputs), strict=True):
147+
torch.testing.assert_close(output.cpu(), expected)

‎docs/source/backends/cuda/cuda-overview.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,29 @@ Do not pass a dict to `skip_h2d_for_method_inputs` itself. The pass reads that f
208208
truthiness, so any non-empty dict enables the skip for every method regardless of the values
209209
inside it.
210210

211+
**Some copies stay even with the skip flags.** In the cases below, `to_executorch` keeps a copy
212+
that runs on every call and logs a warning. To remove the copy, have the partitioner put more of
213+
the graph in the delegate.
214+
215+
- An input stays on the device of the first delegate that reads it. Other readers, such as an
216+
operator outside the delegate, a delegate on another device, or the method output without
217+
`skip_d2h_for_method_outputs`, read a copy in host memory. A delegate on another device then
218+
gets a second copy, from the host.
219+
- A constant, such as a weight, that a delegate reads from outside is copied to each device that
220+
reads it.
221+
- A returned output that an operator outside the delegate writes in place, such as a mutable
222+
custom operator left on the host, is copied to the host and returned there. The method
223+
metadata gives the device of each output.
224+
225+
Add `error_on_per_call_copies=True` to `PropagateDeviceConfig` to make `to_executorch` fail
226+
instead of warning. It fails only on a copy that one of the skip flags you set was meant to remove.
227+
228+
**`skip_h2d_for_method_inputs` refuses inputs that the method changes.** `to_executorch` fails if
229+
the method changes an input, even when the delegate makes the change. This includes a mutable
230+
buffer, such as a KV cache, even though the caller does not pass it. For a buffer, put it and every
231+
operator that uses it in the delegate. The CUDA partitioner already moves the buffer in, with
232+
`tag_mutated_buffer`, once those operators are there.
233+
211234
**The choice is baked into the `.pte`.** A program exported with copies expects host tensors; one
212235
exported without them expects device tensors. Passing the wrong kind is a caller error, not
213236
something the runtime corrects, so the two are not interchangeable at run time.

‎exir/passes/BUCK‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,9 @@ fbcode_target(_kind = runtime.python_library,
523523
"propagate_device_pass.py",
524524
],
525525
deps = [
526+
":convert_constant_dim_order_pass",
526527
":device_copy_ops_registry",
528+
":insert_write_back_for_buffers_pass",
527529
":propagate_device_config",
528530
"//caffe2:torch",
529531
"//executorch/exir:delegate",

‎exir/passes/convert_constant_dim_order_pass.py‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@
33
# This source code is licensed under the BSD-style license found in the
44
# LICENSE file in the root directory of this source tree.
55

6+
import logging
7+
68
import torch
79
from torch.export import ExportedProgram
810
from torch.export.graph_signature import InputKind
911

12+
logger: logging.Logger = logging.getLogger(__name__)
13+
1014

1115
def _should_transform(tensor: torch.Tensor) -> bool:
1216
"""
@@ -42,10 +46,20 @@ def _update_placeholder_meta(
4246
),
4347
None,
4448
)
45-
if input_spec is None:
49+
if placeholder_node is None:
4650
raise RuntimeError(f"Missing placeholder for {input_spec.arg.name}.")
4751

52+
strides = placeholder_node.meta["val"].stride()
4853
placeholder_node.meta["val"] = placeholder_node.meta["val"].contiguous()
54+
logger.warning(
55+
"%s '%s' has strides %s, which are neither contiguous nor channels-last. "
56+
"to_edge stores it contiguous, so partitioners and delegates see the "
57+
"contiguous layout, not the strides in the model. Make it contiguous "
58+
"where it is created to avoid this change.",
59+
kind.name.replace("_", " ").capitalize(),
60+
target,
61+
tuple(strides),
62+
)
4963

5064

5165
def convert_constant_dim_order_pass(
@@ -54,9 +68,9 @@ def convert_constant_dim_order_pass(
5468
"""
5569
Normalize lifted tensors to default or channels-last dim order.
5670
57-
Tensor constants and buffers with other dim orders or striding are converted
58-
to contiguous tensors. This pass acts in-place on the unlifted exported
59-
program.
71+
Tensor constants, buffers and parameters with other dim orders or striding
72+
are converted to contiguous tensors. This pass acts in-place on the unlifted
73+
exported program.
6074
6175
Args:
6276
exported_program: The ExportedProgram to transform.
@@ -84,6 +98,13 @@ def convert_constant_dim_order_pass(
8498
)
8599
_update_placeholder_meta(exported_program, key, kind)
86100

101+
for key, param in exported_program.named_parameters():
102+
if _should_transform(param):
103+
exported_program.state_dict[key] = torch.nn.Parameter(
104+
param.contiguous(), requires_grad=param.requires_grad
105+
)
106+
_update_placeholder_meta(exported_program, key, InputKind.PARAMETER)
107+
87108
# Convert buffers.
88109
for key, buffer in exported_program.named_buffers():
89110
if (

‎exir/passes/propagate_device_config.py‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,28 +48,52 @@ class PropagateDeviceConfig:
4848

4949
# When True, method-level input tensors that feed directly into a device
5050
# delegate are NOT wrapped with _h2d_copy. The user must provide tensors
51-
# already on the target device. Useful for pipelines where inputs are
52-
# pre-staged on GPU.
51+
# already on the target device: the device of the first delegate that
52+
# reads the input, in graph order. Any other reader of such an input, such
53+
# as a host op, a delegate on another device, or the method output without
54+
# skip_d2h_for_method_outputs, reads a copy of it in host memory, made on
55+
# every call, and the pass logs a warning. A delegate on another device
56+
# then gets a second copy, from the host. An input that an op writes in
57+
# place on the host makes to_executorch raise. Useful for pipelines where
58+
# inputs are pre-staged on GPU. Constants are not passed
59+
# by the caller, so they are still copied: their data stays on the host,
60+
# in the program or in its external data file. That copy runs on every
61+
# call, once for each device that reads the constant, and stays in device
62+
# memory until the last delegate there has read it.
5363
#
5464
# Pair with MemoryPlanningPass(alloc_graph_input=False), or the runtime reserves its own buffer
5565
# for the input and copies the caller's memory into it, which is what this exists to avoid.
5666
# A dict can be used to set per-method values, keyed by method name.
5767
skip_h2d_for_method_inputs: Union[bool, Dict[str, bool]] = False
5868

59-
# When True, device delegate outputs that are directly method outputs
60-
# are NOT wrapped with _d2h_copy. The method outputs stay on device.
61-
# Useful for cross-method GPU pipelines where the next method consumes
62-
# GPU tensors directly.
69+
# When True, device delegate outputs that the method returns stay on
70+
# device, with one exception: when an op outside the delegate, such as a
71+
# mutable custom op left on the host, writes the host copy of such an
72+
# output in place, the method returns that host copy, since the device
73+
# tensor never sees the write. The copy to the host runs on every call, and the pass logs a
74+
# warning about it.
75+
# Useful for cross-method GPU pipelines where the next method consumes GPU
76+
# tensors directly.
6377
#
6478
# Pair with MemoryPlanningPass(alloc_graph_output=False) for the same reason as the input flag.
6579
# A dict can be used to set per-method values, keyed by method name.
6680
skip_d2h_for_method_outputs: Union[bool, Dict[str, bool]] = False
6781

82+
# When True, the copies that the skip flags above cannot remove make
83+
# to_executorch raise instead of logging a warning: with
84+
# skip_h2d_for_method_inputs, a constant copied to the device and a method
85+
# input copied to the host for a reader off its device, and with
86+
# skip_d2h_for_method_outputs, a returned output copied to the host for a
87+
# host op that writes it. Each runs on every call. Use it to make sure a
88+
# program that skips its boundary copies has none left.
89+
error_on_per_call_copies: bool = False
90+
6891
def __hash__(self) -> int:
6992
return hash(
7093
(
7194
str(self.skip_h2d_for_method_inputs),
7295
str(self.skip_d2h_for_method_outputs),
96+
self.error_on_per_call_copies,
7397
)
7498
)
7599

@@ -79,4 +103,5 @@ def __eq__(self, other: object) -> bool:
79103
return (
80104
self.skip_h2d_for_method_inputs == other.skip_h2d_for_method_inputs
81105
and self.skip_d2h_for_method_outputs == other.skip_d2h_for_method_outputs
106+
and self.error_on_per_call_copies == other.error_on_per_call_copies
82107
)

0 commit comments

Comments
 (0)