Skip to content

Commit fbf91c2

Browse files
authored
Accept AOTI's compact clones of view buffers in the CUDA weight collector (pytorch#23355)
## What is wrong today When the CUDA backend compiles a model, AOTInductor hands back every constant as a pair: the tensor to save, and a `TensorProperties` record that describes it (storage size, offset, shape, strides). The CUDA weight collector writes the tensor's storage to the `.ptd` file and records that metadata so the runtime can rebuild the tensor. AOTInductor clones every buffer into compact storage first. The clone covers only the bytes the buffer needs and starts at offset 0. But `TensorProperties` is built from the original buffer. That breaks when a buffer is a view into a larger tensor: ```python big = torch.arange(32.0, device="cuda").reshape(8, 4) self.register_buffer("w", big[2:5]) # 48 bytes, at offset 8 of a 128-byte storage ``` The clone is 48 bytes at offset 0, while `TensorProperties` says 128 bytes at offset 8. The collector compares the two, and lowering with `CudaPartitioner` fails: ``` RuntimeError: AOTI cloned storage is smaller than its TensorProperties (48 < 128 bytes) ``` A strided view fails in the bounds check instead, because its `TensorProperties` has no storage size. A column slice `big[:, 1:3]` of an 8x8 tensor fails with: ``` RuntimeError: AOTI view 'w' requires 236 bytes from a 232-byte cloned storage ``` In both cases the clone holds every byte the buffer needs. Only the metadata describes a different storage. ## What this change does When the tensor is AOTInductor's compact clone of the view (a different storage, with the same shape and strides as `TensorProperties`), the collector skips the comparison with the original storage size and takes the offset from the clone it writes. Any other tensor goes through the existing path unchanged. The check that the view fits inside the bytes actually written still runs for every weight, so a truncated clone is still rejected. A compact clone is also checked to span its storage exactly, so a clone with bytes past its view is rejected too. ## What was tested - New unit tests in `test_cuda_partitioner.py`: a clone of `big[2:5]` paired with the view's `TensorProperties` is written as 48 bytes at offset 0; a clone of the strided `big[:, 1:3]` is written as its 232-byte storage, byte for byte; a clone with too little storage or with bytes past its view is rejected; a parameter that slices a fused tensor and keeps its storage still records its own offset and storage size; a value with the same shape but other strides is not taken for a clone; and a tensor with another layout keeps the offset its `TensorProperties` records. The two clone tests fail before this change with the errors above. - On a CUDA GPU (Linux aarch64), the module above lowered with `CudaPartitioner`: before, the error above; after, it exports, and running the `.pte` with the ExecuTorch runtime gives exactly the eager output. - `test_cuda_partitioner.py` and `test_cuda_weight_metadata.py` pass with this change.
1 parent 223455b commit fbf91c2

2 files changed

Lines changed: 124 additions & 5 deletions

File tree

‎backends/cuda/cuda_weight_collector.py‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,15 +399,29 @@ def materialize(
399399
storages: Dict[str, FileBackedData] = {}
400400

401401
for fqn, (tensor, properties) in weights.items():
402+
is_offgraph_kv = _is_offgraph_kv_fqn(fqn)
402403
storage = tensor.untyped_storage()
403404
storage_nbytes = storage.nbytes()
405+
# AOTI clones buffers into compact storage that starts at the view,
406+
# with the same shape and strides, so the original storage size and
407+
# offset do not describe it.
408+
is_compact_clone = (
409+
not is_offgraph_kv
410+
and getattr(properties, "storage_ptr", None)
411+
not in (None, storage.data_ptr())
412+
and tuple(tensor.shape) == tuple(getattr(properties, "shape", ()))
413+
and tuple(tensor.stride()) == tuple(getattr(properties, "stride", ()))
414+
)
404415
del storage
405416
device_type = device_type_for_weight(tensor)
406-
is_offgraph_kv = _is_offgraph_kv_fqn(fqn)
407417
expected_storage_nbytes = int(
408418
getattr(properties, "storage_size", None) or 0
409419
)
410-
if not is_offgraph_kv and storage_nbytes < expected_storage_nbytes:
420+
if (
421+
not is_offgraph_kv
422+
and not is_compact_clone
423+
and storage_nbytes < expected_storage_nbytes
424+
):
411425
raise RuntimeError(
412426
"AOTI cloned storage is smaller than its TensorProperties "
413427
f"({storage_nbytes} < {expected_storage_nbytes} bytes)"
@@ -427,7 +441,11 @@ def materialize(
427441
strides = tuple(
428442
int(stride) for stride in getattr(properties, "stride", tensor.stride())
429443
)
430-
storage_offset = int(getattr(properties, "offset", tensor.storage_offset()))
444+
storage_offset = (
445+
tensor.storage_offset()
446+
if is_compact_clone
447+
else int(getattr(properties, "offset", tensor.storage_offset()))
448+
)
431449
required_nbytes = _required_view_nbytes(
432450
fqn, sizes, strides, storage_offset, tensor.element_size()
433451
)
@@ -436,6 +454,11 @@ def materialize(
436454
f"AOTI view {fqn!r} requires {required_nbytes} bytes from a "
437455
f"{storage_nbytes}-byte cloned storage"
438456
)
457+
if is_compact_clone and required_nbytes != storage_nbytes:
458+
raise RuntimeError(
459+
f"AOTI compact clone of {fqn!r} should span exactly its "
460+
f"{storage_nbytes}-byte storage, but its view needs {required_nbytes}"
461+
)
439462
if is_offgraph_kv:
440463
# Preserve the AOTI view contract; the runtime supplies storage.
441464
storage_nbytes = max(

‎backends/cuda/tests/test_cuda_partitioner.py‎

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
from executorch.exir.backend.partitioner import PartitionResult
4040
from executorch.exir.delegate import executorch_call_delegate
4141
from torch._export.utils import is_buffer, is_lifted_tensor_constant, is_param
42+
from torch._inductor.compile_fx import clone_preserve_strides
4243
from torch.export import export
4344
from torch.export.pt2_archive._package_weights import TensorProperties, Weights
4445
from torch.fx.passes.utils.fuser_utils import validate_partition
@@ -232,8 +233,8 @@ def test_different_fqn_views_have_distinct_logical_storage(self) -> None:
232233
weights = Weights(
233234
{
234235
"base": (base, TensorProperties(base)),
235-
# AOTI may return a cloned value tensor; TensorProperties is
236-
# the source of truth for reconstructing the original view.
236+
# The value shares the view's storage, so the offset from
237+
# TensorProperties applies.
237238
"view": (base, TensorProperties(view)),
238239
}
239240
)
@@ -251,6 +252,101 @@ def test_different_fqn_views_have_distinct_logical_storage(self) -> None:
251252
for storage in artifact.storages.values():
252253
storage.close()
253254

255+
def test_compact_clone_of_a_view_uses_the_written_storage(self) -> None:
256+
# AOTI clones a buffer that views a larger tensor into compact storage
257+
# but reports TensorProperties of the original view.
258+
view = torch.arange(32, dtype=torch.float32).reshape(8, 4)[2:5]
259+
clone = view.clone()
260+
weights = Weights({"w": (clone, TensorProperties(view))})
261+
262+
with tempfile.TemporaryDirectory() as directory:
263+
artifact = self._materialize(weights, directory)
264+
entry = artifact.entries[0]
265+
self.assertEqual(0, entry.storage_offset)
266+
self.assertEqual(48, entry.storage_nbytes)
267+
self.assertEqual((3, 4), entry.sizes)
268+
data = artifact.storages[entry.storage_key].to_bytes()
269+
self.assertEqual(bytes(clone.untyped_storage()), data)
270+
for storage in artifact.storages.values():
271+
storage.close()
272+
273+
def test_compact_clone_with_too_little_storage_is_rejected(self) -> None:
274+
view = torch.arange(32, dtype=torch.float32).reshape(8, 4)[2:5]
275+
truncated = view.clone()
276+
truncated.untyped_storage().resize_(8 * truncated.element_size())
277+
with tempfile.TemporaryDirectory() as directory:
278+
with self.assertRaisesRegex(RuntimeError, "requires 48 bytes"):
279+
self._materialize(
280+
Weights({"w": (truncated, TensorProperties(view))}), directory
281+
)
282+
283+
def test_compact_clone_with_bytes_past_its_view_is_rejected(self) -> None:
284+
view = torch.arange(32, dtype=torch.float32).reshape(8, 4)[2:5]
285+
padded = view.clone()
286+
padded.untyped_storage().resize_(16 * padded.element_size())
287+
with tempfile.TemporaryDirectory() as directory:
288+
with self.assertRaisesRegex(RuntimeError, "span exactly its 64-byte"):
289+
self._materialize(
290+
Weights({"w": (padded, TensorProperties(view))}), directory
291+
)
292+
293+
def test_value_with_the_same_shape_but_other_strides_is_not_a_clone(self) -> None:
294+
view = torch.arange(32, dtype=torch.float32).reshape(8, 4)[2:5]
295+
transposed = torch.arange(12, dtype=torch.float32).reshape(4, 3).t()
296+
with tempfile.TemporaryDirectory() as directory:
297+
with self.assertRaisesRegex(
298+
RuntimeError, "smaller than its TensorProperties"
299+
):
300+
self._materialize(
301+
Weights({"w": (transposed, TensorProperties(view))}), directory
302+
)
303+
304+
def test_compact_clone_of_a_strided_view_spans_its_storage(self) -> None:
305+
# A strided view's clone keeps the gaps between its rows, so it spans
306+
# more bytes than its elements take.
307+
view = torch.arange(64, dtype=torch.float32).reshape(8, 8)[:, 1:3]
308+
clone = clone_preserve_strides(view)
309+
weights = Weights({"w": (clone, TensorProperties(view))})
310+
311+
with tempfile.TemporaryDirectory() as directory:
312+
artifact = self._materialize(weights, directory)
313+
entry = artifact.entries[0]
314+
self.assertEqual(0, entry.storage_offset)
315+
self.assertEqual(232, entry.storage_nbytes)
316+
self.assertEqual((8, 1), entry.strides)
317+
data = artifact.storages[entry.storage_key].to_bytes()
318+
self.assertEqual(bytes(clone.untyped_storage()), data)
319+
for storage in artifact.storages.values():
320+
storage.close()
321+
322+
def test_view_sharing_its_storage_keeps_the_view_offset(self) -> None:
323+
# AOTI does not clone parameters, so a parameter that slices a fused
324+
# tensor arrives in the fused tensor's storage.
325+
fused = torch.arange(36, dtype=torch.float32).reshape(9, 4)
326+
view = fused[3:6]
327+
weights = Weights({"w": (view, TensorProperties(view))})
328+
329+
with tempfile.TemporaryDirectory() as directory:
330+
artifact = self._materialize(weights, directory)
331+
entry = artifact.entries[0]
332+
self.assertEqual(12, entry.storage_offset)
333+
self.assertEqual(144, entry.storage_nbytes)
334+
for storage in artifact.storages.values():
335+
storage.close()
336+
337+
def test_value_with_other_layout_keeps_the_view_offset(self) -> None:
338+
# A value in a different storage that is not a compact clone of the view
339+
# (other shape) keeps the offset its TensorProperties records.
340+
base = torch.arange(32, dtype=torch.float32).reshape(8, 4)
341+
view = base[:, 1:]
342+
weights = Weights({"w": (base.clone(), TensorProperties(view))})
343+
344+
with tempfile.TemporaryDirectory() as directory:
345+
artifact = self._materialize(weights, directory)
346+
self.assertEqual(1, artifact.entries[0].storage_offset)
347+
for storage in artifact.storages.values():
348+
storage.close()
349+
254350
def test_identical_values_keep_distinct_fqn_keys(self) -> None:
255351
first = torch.zeros(4)
256352
second = torch.zeros(4)

0 commit comments

Comments
 (0)