Repository navigation
Fix crashes and lowering failures in the AOTI, OpenVINO and Qualcomm backends - #24222
Open
shoumikhin wants to merge 5 commits into
Open
shoumikhin wants to merge 5 commits into
shoumikhin wants to merge 5 commits into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24222
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New Failure, 2 Pending, 2 Unrelated FailuresAs of commit 55b4840 with merge base 92721a8 ( NEW FAILURE - The following job has failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was 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
Bot
added
the
CLA Signed
Contributor
Author
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 22m 42s —— View job Approve: nothing blocks. The main open point is that on OpenVINO before 2026.4, the delegate blob now stays in memory next to OpenVINO's own copy for the whole life of the method. Three smaller robustness and test gaps are noted below. PR Review: #24222SummaryThe PR does two things:
Nothing blocks merging. The OpenVINO memory cost on versions before 2026.4 is worth settling, and three smaller points follow. The PyTorch Inductor and OpenVINO sources are not in this checkout, so I mark statements about their internals as low confidence. I ran no tests. Code Quality
Testing
Performance
RecommendationApprove Nothing here blocks the merge. The one point to settle is the OpenVINO memory cost: either free the blob on versions before 2026.4, or document the cost. That can happen here or in a follow-up.
|
Gasoonjia
approved these changes
Oct 28, 2026
…kends An AOTI backend (CUDA, Metal) checks at lowering time that every kernel Inductor leaves to ATen exists in the backend's shim library, so a missing one fails the export with a clear list. The check watches two code paths that emit a C shim call, but scatter and index_put fallbacks write their call directly. A program that needs one of those kernels lowers without an error, and the process then dies on the first run with an undefined symbol. For example, Inductor leaves an int64 scatter_add_ to aoti_torch_cuda_scatter_reduce_two_out, which the CUDA shim does not have. Watch those two paths too. They do not pass the kernel name to a method the check can see, so read it from the line they write. This checks the name the generated code really calls.
Since OpenVINO 2026.4, ov_core_import_model wraps the blob it is given in an ov::Tensor and the compiled model can keep reading that memory. Earlier versions read it through a stream during the import only. OpenvinoBackend::init frees the blob right after the import. When the program is loaded from a buffer, Free() does nothing, so nothing goes wrong. When it is loaded from a file, the blob is really freed: with a memory-mapped file the first execute crashes with a segmentation fault, even for a two-layer model, and with a file read into memory the model reads freed memory. Free the blob in init only when the OpenVINO runtime is older than 2026.4, as before. With newer versions keep it: ExecuTorch owns it and frees it after destroy(), so it lives exactly as long as the compiled model. The op tests loaded every program from a buffer, so they never saw this. They now load from a file, as most users do.
…gions
torch.export records a `with torch.autocast(...)` block as a
wrap_with_autocast call on a submodule. DecomposeWrapWithAutocast inlines
that submodule into the graph. It had two bugs:
1. It mapped the submodule's placeholders to the operands by name. When
the region reads inputs or parameters, the placeholders are named like
arg2_1, arg0_1, arg1_1, not after the operands, so the pass failed with
a KeyError such as 'arg2_1'. For example:
with torch.autocast("cpu", enabled=False):
y = self.linear(x)
2. It only looked at the top graph. A region nested in another region was
copied out with its get_attr, whose submodule name only means something
inside the outer submodule. In the top module it resolved to the outer
body, so the program silently ran the wrong code.
Map placeholders to operands by position, and inline nested regions
before their parent.
Edge turns `x.detach()` into `aten.detach_copy`. Inductor has no lowering
or C shim for it, so an AOTI backend (CUDA, Metal) cannot lower any graph
that calls detach, for example:
return (x * 2).detach() + 1
fails with "missing fallback kernels: aten::detach_copy".
ReplaceViewCopyWithViewPass already maps copy ops Inductor cannot lower
(slice_copy, select_copy, split_copy) back to views. Map detach_copy to
alias, which is what detach means without autograd. Mapping to detach
itself would still fail Edge verification under the default compile
config, because detach is a non-core view op that the partitioner
preserves.
AOTInductor can return a method output as a view into a larger buffer with strides that do not match a contiguous tensor, for example a slice `y[:, :, :14]` of a [1, 50, 32] result. The CUDA backend copies outputs into the planned ETensor only from contiguous SlimTensors, so execute() failed with "SlimTensor must be contiguous" although lowering accepted the program. The copy already has a slow path that rearranges elements through host memory when the strides differ from the ETensor's. Let it take non-contiguous views too: copy the span the view reaches instead of nbytes(), and check that span against the storage, with checked arithmetic. Wrapping a SlimTensor without a copy still requires a contiguous one. A CUDA graph replay copies each output's raw bytes. For such a strided output, and also for a contiguous output whose ETensor uses another dim order, the replay was silently wrong. A raw copy is known to be right when both are contiguous in the default dim order, so for any other layout warmup now turns graphs off and the capture step refuses to replay, so the method runs without a graph.
shoumikhin
force-pushed
the
fix-lerobot-export-bugs
branch
from
October 28, 2026 09:44
35f7db1 to
55b4840
Compare
shoumikhin
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five backend bugs, one commit each. AOTI is AOTInductor, the ahead-of-time compiler behind the CUDA and Metal backends.
CUDA and Metal: missing scatter and index_put kernels pass lowering
AotiBackendchecks at lowering time that every kernel Inductor leaves to ATen exists in the backend's shim library. Scatter and index_put fallbacks write their call directly, so the check missed them and the program died at run time on an undefined symbol. Now the check also reads the kernel names they write.OpenVINO 2026.4: loading a
.ptefrom a file crashesSince OpenVINO 2026.4 the compiled model can keep reading the model data, but
OpenvinoBackend::initfreed it right away. Loading from a file really frees it, so the firstexecutecan crash. Nowinitfrees it only before 2026.4, whose import copies it. The op tests now load from a file.Qualcomm:
DecomposeWrapWithAutocastfails on autocast regionsIt matched an autocast submodule's inputs to the operands by name, but the names often differ (
KeyError: 'arg2_1'). A nested region also silently ran the wrong code. Now it matches by position and inlines nested regions first.CUDA and Metal:
detach()cannot be loweredEdge turns
x.detach()intodetach_copy, which Inductor cannot lower. The AOTI pass that mapsslice_copyback to a view now also mapsdetach_copytoalias.CUDA: a strided method output fails at run time
AOTInductor can return an output as a slice of a larger buffer, such as
y[:, :, :14], but the output copy took only contiguous tensors ("SlimTensor must be contiguous"). The slow copy path now handles them. A CUDA graph replay copies raw bytes, which is wrong for these and for outputs whose ETensor has another dim order, so such methods now run without a graph.Testing
test_detach,test_missing_scatter_and_index_put_kernels_fail_at_exportandCopiesAStridedViewfail without the fixes and pass with them; the rest oftest_cuda_export.pyandtest_sort_shim.pypass. A Linear followed by a slice now runs and matches eager, also with CUDA graphs on. Metal was not run.initstill frees the data.