[Gasoonjia]

Summary:
The CUDA shim aoti_torch__reinterpret_tensor treated its offset argument as an absolute storage offset. PyTorch's _reinterpret_tensor (torch/csrc/inductor/inductor_ops.cpp) adds it to the source tensor's existing storage offset. When the source already had a nonzero offset, the shim returned success but read the wrong elements. For example, with a source at offset 4 and increment 2, it read storage [2, 3, 4] instead of [6, 7, 8].

This change adds self->storage_offset() to the increment, matching PyTorch, and renames the parameter to offset_increment. aoti_torch_as_strided now passes an increment of 0, so it keeps the source offset without adding it twice.

Differential Revision: D130858308

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 1 Unrelated Failure, 1 Unclassified Failure

As of commit a684f6a with merge base 0287ce1 ():

NEW FAILURE - The following job has failed:

• pull / unittest / macos / macos-job (gh)
  pytorch_tokenizers

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

• Test CUDA Builds / test-muse-glimmer-cuda-e2e-kquant-17gb-dflash-image-batching / linux-job (gh) (this job did not run on the merge base, so DrCI cannot tell whether the failure is pre-existing)
  Could not load credentials from any providers

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

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

• pull / android / run-emulator (gh) (trunk failure)
  adb: device offline

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

[meta-codesync]

@Gasoonjia has exported this pull request. If you are a Meta employee, you can view the originating Diff in D130858308.

[github-actions]

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.

[Gasoonjia]

@claude /pr-review

[claude]

Claude finished @Gasoonjia's task in 10m 57s —— View job

---

This PR makes aoti_torch__reinterpret_tensor add its offset to self's storage offset, matching PyTorch's _reinterpret_tensor; nothing blocks it, but there are three minor issues: a stale as_strided line in the header doc, a negative offset that aborts the process, and new tests that don't match the file's style.

Detailed review

PR Review: #24827

Summary

The PR makes aoti_torch__reinterpret_tensor add offset_increment to self->storage_offset(), matching PyTorch's _reinterpret_tensor, and changes aoti_torch_as_strided to pass 0. Three minor issues, none blocking:

• The header doc now contradicts itself.
• A negative combined offset aborts the process instead of returning an error.
• The new tests don't follow the test file's conventions.

API Design

• backends/cuda/runtime/shims/memory.h:168: the doc summary still describes the old meaning.
  • It says the function "is equivalent to torch.as_strided()". torch.as_strided's storage_offset is an absolute offset, which is the reading this PR removes. That contradicts the new @param offset_increment text at lines 175-176.
  • The next declaration, aoti_torch_as_strided (line 190), is also described as torch.as_strided. A reader sees two neighbouring shims claiming the same equivalence with different offset meanings.
  • Fix: describe this one as torch.ops.inductor._reinterpret_tensor, a view at offset self.storage_offset() + offset_increment. Fix this →

Code Quality

• backends/cuda/runtime/shims/memory.cpp:415-416: a negative combined offset aborts the process instead of returning InvalidArgument.
  • The sum self->storage_offset() + offset_increment goes unchecked into SlimTensor::as_strided_. That function's ET_CHECK_MSG(storage_offset >= 0, ...) (backends/aoti/slim/core/slim_tensor_view_incl.h:43-46) calls runtime_abort().
  • The shim's own argument checks (lines 390-409) return InvalidArgument. For example, an increment of -1 aborts on a source at offset 0 but works on a source at offset 4.
  • This is not a regression: a negative absolute offset aborted before too. But the argument is now an increment, so negative values are valid input, and this shim is the only place that sees both terms.
  • Fix: compute the sum once and reject a negative result with ET_CHECK_OR_RETURN_ERROR. The new tests only use increments of 0, 2 and 4, so also add two tests: a valid negative increment (source at offset 4, increment -4 gives offset 0 and values {0, 1, 2}) and the rejected case. Fix this →
• backends/cuda/runtime/shims/tests/test_aoti_torch__reinterpret_tensor.cpp:742-846: the new tests don't follow the file's conventions.
  • They use C arrays (const int64_t sizes[] = {3};) and the short createTestTensor({32}) form. The existing tests, including the neighbouring AsStridedKeepsStorageOffset_CUDA (lines 698-739), use std::vector<int64_t> with .data() and pass dtype and device explicitly.
  • The two _CPU tests were appended under the // CUDA Tests header (line 526). They belong in the CPU sections; OffsetIsRelativeToSelf_CPU fits in // Storage Offset Tests (line 182) next to WithStorageOffset_CPU.
  • Fix this →

Recommendation

Approve

Nothing blocks merging. The memory.h:168 wording is worth fixing in this PR, because it describes exactly the behaviour being removed.