Skip to content

Fix crashes, wrong outputs and lowering failures in several backends and in exir - #24222

Merged
shoumikhin merged 37 commits into
pytorch:mainfrom
shoumikhin:fix-lerobot-export-bugs
Oct 29, 2026
Merged

shoumikhin merged 37 commits into
pytorch:mainfrom
shoumikhin:fix-lerobot-export-bugs

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes found while exporting models to ExecuTorch backends.

Fixes

  • exir: constant_prop_pass keeps tensors that in-place ops write, also through views or in no_grad/autocast blocks. to_backend keeps the output node last. to_edge lowers torch._check_tensor_all and torch.sym_ite. GroupBasedPartitioner no longer makes partitions that depend on each other, unless caller node groups already do. Tensor constants get static shapes; deduplicated constants keep their data.
  • AOTI (AOTInductor) and CUDA: the missing-kernel check catches scatter and index_put fallbacks. detach lowers, also inside torch.cond. Strided outputs are copied. Transposed convolutions (1-D, 2-D, no dilation) and torch._check_tensor_all lower.
  • XNNPACK: the quantizer skips bfloat16 tensors, and cat respects scoped configs. ELU is delegated when XNNPACK can run it.
  • Qualcomm: autocast regions are inlined correctly. Unquantized bfloat16 nodes, data-dependent shapes and HTP partitions without runtime inputs stay on the CPU. Fixes for stacks, 0-d selects, RMSNorm and int64 constants.
  • OpenVINO: model data stays alive while OpenVINO 2026.4 reads it. Partitions are converted with their input types.
  • Core ML: bfloat16 ops, and ops above rank 5 outside static layout chains, stay on the CPU.
  • MLX, Arm, Cortex-M, Vulkan, NXP, portable: dtype checks for MLX tensor lists, one constant slot per weight, slice updates on a constant base; TOSA keeps unused delegate inputs for VGF; MatmulToBmmPass takes a constant operand; Vulkan constants above 4 dims; non-strict programs in the NXP quantizer; cummin.out.

Behavior changes

  • A CUDA .pte loads only the weights it was exported with. A .ptd with other weights, for example after fine-tuning, needs a newly exported .pte.
  • CUDA graphs turn off when outputs do not match the ETensor layout. If only the capture call sees such a layout, that call returns an error.
  • GroupBasedPartitioner is about 2x slower on large graphs, and some graphs get a different number of partitions. to_backend keeps the original order of top-level nodes.
  • XNNPACK: weight-only quantized linears, and 8da4w or 8da8w linears (8-bit dynamic activations, 4-bit or 8-bit weights) lowered with to_backend, fail at export instead of giving wrong results. Delegated ELU flushes subnormal values to zero. Cats outside a scoped quantizer config are no longer quantized.
  • Ethos-U raises an error for a delegate with unused inputs instead of returning wrong values.

We would like to cherry-pick this PR to release/1.6 after it merges. It needs #23915 there first.

Testing

Each fix has a test that fails without it. Run on an A100 (CUDA, Vulkan), an x86 CPU (OpenVINO, QNN SDK), macOS (Core ML, MLX), the TOSA reference model and VGF emulation (Arm), and the eIQ Neutron SDK (NXP). Not run: Metal, ROCm, Windows CUDA.

Copilot AI balanced review requested due to automatic review settings October 28, 2026 03:40
@pytorch-bot

pytorch-bot Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

🔗 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 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 1 New Failure, 70 Pending

As of commit 204bbc8 with merge base 254635c (image):

NEW FAILURE - The following job has failed:

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 Oct 28, 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 insufficient remaining quota.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

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: #24222

Summary

The PR does two things:

  • AOTI lowering now reports missing scatter and index_put fallback kernels.
  • OpenvinoBackend::init no longer frees a blob that the compiled model may still read.

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

  • backends/aoti/aoti_backend.py:208-237: the scatter check rebuilds Inductor's private symbol name instead of checking what Inductor writes.

    • The other three wrappers check a name that Inductor passes in. This one copies the naming logic inside CppWrapperCpu._generate_scatter_fallback (get_c_shim_func_name(...).replace("__", "_") + "_out", lines 220-223). It also spells out all 8 of that method's parameters, so a signature change turns every scatter fallback into a TypeError.
    • The test asserts the same rebuilt string (backends/cuda/tests/test_cuda_export.py:580).
      • If a torch bump changes the name Inductor emits, CI stays green, but the check reports a name the .wrapper.so never calls.
      • Once someone adds that name to get_supported_fallback_kernels, lowering passes and the program crashes on its first run again.
    • Comparing against what the original method writes would catch this drift at lowering time. For example, wrap self.writeline for the duration of the call, and fail if the emitted call is not kernel.
    • Low confidence: one check over the aoti_torch_* calls in the generated wrapper would also cover direct-emission paths that nothing watches yet. The CUDA shim defines no aoti_torch_item_float16 or aoti_torch_item_float64 (backends/cuda/runtime/shims/memory.cpp:451-458), and I believe Inductor's codegen_tensor_item emits those calls directly.

    Fix this →

  • backends/openvino/tests/ops/base_openvino_op_test.py:71-80: the mapped .pte is released only on success.

    • If load_method, execute, or the assert raises, the traceback keeps program and method alive while TemporaryDirectory cleans up.
    • On Windows (the case the comment on line 79 is about), the cleanup then raises PermissionError on top of the real failure.
    • tempfile.TemporaryDirectory(ignore_cleanup_errors=True) avoids this; pyproject.toml requires Python 3.11 or newer, so it is available.

    Fix this →

Testing

  • backends/aoti/aoti_backend.py:251-256: the Metal part of fix 1 has no test.

    • The PR says Metal was not run, and nothing under backends/apple/metal/tests/ uses scatter, index_put, or the missing-kernel error.
    • The patch is on CppWrapperCpu. The CUDA test shows that the CUDA wrapper picks it up, but nothing shows that the MPS wrapper does not override _generate_scatter_fallback or _generate_index_put_fallback. This part is low confidence, since I could not read Inductor.
    • If the MPS wrapper does override them, Metal programs still lower and then crash on their first run, because the .so is opened with RTLD_LAZY (backends/apple/metal/runtime/metal_backend.cpp:347).
    • Adding the Int64Accumulate case to backends/apple/metal/tests/test_metal_backend.py, built with export_model_to_metal (test_modules.py:1218), would cover this.

    Fix this →

Performance

  • backends/openvino/runtime/OpenvinoBackend.cpp:336-337: on OpenVINO before 2026.4, the blob now stays alive next to OpenVINO's own copy for the life of the method.

    • The PR description accepts this cost, but neither the code comment nor the docs mention it.
    • 2025.1 to 2026.3 are still advertised as supported. docs/source/build-run-openvino.md:64 and the runtime-not-found error at OpenvinoBackend.cpp:215 both say openvino>=2025.1.0,<2027.0.0.
    • What the extra copy costs depends on the loader:
      • With the default Module load mode, LoadMode::File (extension/module/module.h:100), it is a heap buffer the size of the compiled model.
      • With the LLM runner's default, MmapUseMlockIgnoreErrors (extension/llm/runner/llm_runner_helper.h:112), the pages stay locked in RAM whenever mlock succeeds.
    • For the examples/openvino/llama and qwen2_5 exports, the blob is the whole quantized model.
    • Freeing only on versions before 2026.4 gives those versions back exactly the behavior they had before this PR. This needs ov_get_openvino_version and ov_version_free declared in OpenvinoApi.h and loaded with the other LOAD_SYMs. At minimum, state the cost in the comment and on the docs page.

    Fix this →

Recommendation

Approve

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.


  • Gather context and fetch the diff (the checkout is shallow, so I compared against the merge base 8ecf0fb)
  • Run three independent reviews, then consolidate and fact-check them
  • Post review (I ran no tests)

@shoumikhin
shoumikhin force-pushed the fix-lerobot-export-bugs branch from 35f7db1 to 55b4840 Compare October 28, 2026 09:44
@shoumikhin
shoumikhin requested a review from psiddh as a code owner October 28, 2026 09:44
Copilot AI balanced review requested due to automatic review settings October 28, 2026 09:44
@shoumikhin shoumikhin changed the title Fix two backend crashes that show up only when a program runs Fix crashes and lowering failures in the AOTI, OpenVINO and Qualcomm backends Oct 28, 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 insufficient remaining quota.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Points 1, 2 and 4 are fixed in the commits that own the code. Point 3 has no code change, see below.

  1. Scatter and index_put check: it no longer rebuilds the kernel name or spells out the method's arguments. It reads the kernel name from the line the fallback writes, so it checks what the generated code really calls. The item path you mention, and view_dtype, also write their calls directly. I left them for a follow-up that checks the built library's symbols instead of hooking one path at a time.
  2. OpenVINO test cleanup: the temporary folder now ignores cleanup errors, so a failing test on Windows reports only its own error.
  3. Metal: the Metal tests need torchao's MPS kernels, which my setup does not have, so I did not add a Metal test. I checked instead that the MPS wrapper does not override the two hooked methods, and that an MPS compile with this check reports both kernels as missing.
  4. OpenVINO memory: init now frees the blob as before when the OpenVINO runtime is older than 2026.4, because those versions copy it during the import. With newer versions the blob is kept until the delegate is destroyed.

@shoumikhin shoumikhin changed the title Fix crashes and lowering failures in the AOTI, OpenVINO and Qualcomm backends Fix crashes and lowering failures in the AOTI, OpenVINO and Qualcomm backends and in constant_prop_pass Oct 28, 2026
@shoumikhin
shoumikhin force-pushed the fix-lerobot-export-bugs branch from 55b4840 to 52e8dcf Compare October 28, 2026 15:04
@shoumikhin
shoumikhin force-pushed the fix-lerobot-export-bugs branch from 52e8dcf to c39d57f Compare October 28, 2026 19:42
Copilot AI balanced review requested due to automatic review settings October 28, 2026 19:42
@shoumikhin shoumikhin changed the title Fix crashes and lowering failures in the AOTI, OpenVINO and Qualcomm backends and in constant_prop_pass Fix crashes, wrong outputs and lowering failures in several backends and in constant_prop_pass Oct 28, 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 insufficient remaining quota.

@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Oct 28, 2026
@shoumikhin
shoumikhin force-pushed the fix-lerobot-export-bugs branch from c39d57f to d11bd4b Compare October 28, 2026 21:47
Every CUDA program writes its weights to a file named aoti_cuda_blob.ptd,
and keys each weight by its parameter name. When two programs with the
same parameter names are exported into one folder, the second export
overwrites the first program's weights file. The first program still
loads, finds every key it asks for in the other program's file, and
silently runs with the other program's weights. For example, a Linear
layer exported twice with different seeds: the first program then
returned the second one's outputs.

Add each weight's SHA-256, which the exporter already computes while it
writes the weight, to the weight's key. A program then finds only keys
written for its own weights, and loading it with another program's file
fails with "CUDA FQN storage ... is missing from named data". The bytes
are not hashed again at load, so this catches files that belong to
another program, not files corrupted in place.
Identical weights still get the same key, so methods that share a
weight still store it once. Within one export, a later method that
stores different bytes under the same name is still rejected.

The file name stays aoti_cuda_blob.ptd, because runners and scripts look
for it. Two programs still need their own folders or file names, but a
mix-up now fails at load instead of giving wrong results. This also means
a .ptd with different weights, for example from a fine-tuned model, needs
the .pte exported with it.

test_missing_weights_blob.py exports two programs with different weights
into one folder and checks that the first one refuses the second one's
file. Without this change it loads. Tests that read the stored keys now
take them from the delegate's manifest, which is what the runtime reads.
@shoumikhin
shoumikhin force-pushed the fix-lerobot-export-bugs branch from 9136f25 to 204bbc8 Compare October 29, 2026 15:09
Copilot AI balanced review requested due to automatic review settings October 29, 2026 15:09

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 insufficient remaining quota.

@shoumikhin
shoumikhin merged commit 054de1f into pytorch:main Oct 29, 2026
546 of 549 checks passed
@shoumikhin

Copy link
Copy Markdown
Contributor Author

@pytorchbot cherry-pick --onto release/1.6 -c critical

@pytorchbot

Copy link
Copy Markdown
Collaborator

Cherry picking #24222

The cherry pick PR is at #24298 and it is recommended to link a critical cherry pick PR with an issue.

Details for Dev Infra team Raised by workflow job

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@pytorchbot cherry-pick --onto release/1.6 -c critical

@pytorchbot

Copy link
Copy Markdown
Collaborator

Cherry picking #24222

Command git -C /home/runner/work/executorch/executorch rebase origin/cherry-pick-24222-by-pytorch_bot_bot_ returned non-zero exit code 1

Rebasing (1/4)
Auto-merging exir/pass_base.py
Auto-merging exir/tests/test_pass_infra.py
CONFLICT (content): Merge conflict in exir/tests/test_pass_infra.py
error: could not apply 0842bfad29... exir: Track unbacked symbols during pass replay (#24263)
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply 0842bfad29... # exir: Track unbacked symbols during pass replay (#24263)
Details for Dev Infra team Raised by workflow job

shoumikhin added a commit that referenced this pull request Oct 29, 2026
This branch was picked before #24263 and #24295 reached release/1.6, so
three files carried their content only in part. Take release/1.6 and use
the versions a clean pick of 054de1f gives for
backends/qualcomm/partition/qnn_partitioner.py,
backends/qualcomm/tests/test_passes.py and exir/tests/test_pass_infra.py.
shoumikhin added a commit that referenced this pull request Oct 30, 2026
…fer check (#24299)

## What is wrong today

Robot policies and follow-up checks on #24222 found these bugs in the
CUDA backend and the runtime:

1. **CUDA mask indexing:** `x[mask]` and `x[mask] = values` have no C
shims, so lowering fails with "missing fallback kernels". This blocks
SmolVLA.
2. **CUDA graphs:** a channels-last output turns CUDA graphs off,
although a raw copy is correct.
3. **CUDA graph shapes:** when the captured call has a new batch size,
it and every replay return the previous output shape.
4. **Output buffers:** `Method::set_output_data_ptr` checks a buffer
against the output's current size. After a dynamic output shrinks, a
too-small buffer is accepted and a later, larger call writes past it.

## What this change does

One commit per fix:

1. New shims for index and index_put follow ATen's indexing rules. The
Windows import library gains the new names. Inside a CUDA graph capture,
a mask index fails with a clear error.
2. The check compares output strides with the strides the ETensor's dim
order gives.
3. The capture step resizes each output to the captured shape.
4. The buffer is checked against the output's upper bound. Code that
binds a smaller buffer after a call now gets `InvalidArgument`.

## Tests

Each new test fails without its fix.

- **CUDA (A100):** mask reads and writes, also inside a capture; the
raw-copy check; a capture call with a new batch size.
- **Runtime:** a too-small output buffer is refused after a dynamic
output shrinks; the runtime tests pass.

On LeRobot policies, SmolVLA now lowers to CUDA and runs in 43 ms per
call on an A100, against 504 ms in eager PyTorch (cosine similarity
0.99996).
shoumikhin added a commit that referenced this pull request Oct 30, 2026
…fer check (#24299)

## What is wrong today

Robot policies and follow-up checks on #24222 found these bugs in the
CUDA backend and the runtime:

1. **CUDA mask indexing:** `x[mask]` and `x[mask] = values` have no C
shims, so lowering fails with "missing fallback kernels". This blocks
SmolVLA.
2. **CUDA graphs:** a channels-last output turns CUDA graphs off,
although a raw copy is correct.
3. **CUDA graph shapes:** when the captured call has a new batch size,
it and every replay return the previous output shape.
4. **Output buffers:** `Method::set_output_data_ptr` checks a buffer
against the output's current size. After a dynamic output shrinks, a
too-small buffer is accepted and a later, larger call writes past it.

## What this change does

One commit per fix:

1. New shims for index and index_put follow ATen's indexing rules. The
Windows import library gains the new names. Inside a CUDA graph capture,
a mask index fails with a clear error.
2. The check compares output strides with the strides the ETensor's dim
order gives.
3. The capture step resizes each output to the captured shape.
4. The buffer is checked against the output's upper bound. Code that
binds a smaller buffer after a call now gets `InvalidArgument`.

## Tests

Each new test fails without its fix.

- **CUDA (A100):** mask reads and writes, also inside a capture; the
raw-copy check; a capture call with a new batch size.
- **Runtime:** a too-small output buffer is refused after a dynamic
output shrinks; the runtime tests pass.

On LeRobot policies, SmolVLA now lowers to CUDA and runs in 43 ms per
call on an A100, against 504 ms in eager PyTorch (cosine similarity
0.99996).
shoumikhin added a commit that referenced this pull request Oct 30, 2026
The pick was made before #24298 (the #24222 pick) was on release/1.6, so
three files carried partial copies of #24222. This merges #24298's branch
and restores the files so that, on top of #24298, the change is exactly
#24299 (ff1bab3).
shoumikhin added a commit that referenced this pull request Oct 30, 2026
msluszniak added a commit to msluszniak/executorch that referenced this pull request Oct 30, 2026
…ork suite

pytorch#24222 fixed I64toI32 casting a lifted int64 constant before its
definition. Add the matching subtest to test_i64_to_i32.
msluszniak added a commit to msluszniak/executorch that referenced this pull request Oct 30, 2026
…ion stays on CPU

pytorch#24222 drops HTP partitions without runtime inputs. Cover the case of an
int64 buffer whose I64toI32 cast is left without a delegated consumer.
shoumikhin added a commit that referenced this pull request Oct 30, 2026
release/1.6 regenerated aoti_cuda_shims.lib to add
aoti_torch_dtype_element_size, and this branch regenerated it to add
aoti_torch_as_strided. Regenerate it per aoti_cuda_shims.lib.md with both
names, so it lists exactly the shims this tree exports.
shoumikhin added a commit that referenced this pull request Oct 30, 2026
Regenerate aoti_cuda_shims.lib per aoti_cuda_shims.lib.md so it lists
exactly the shims this tree exports: release/1.6's set, including
aoti_torch_dtype_element_size, plus aoti_torch_as_strided,
aoti_torch_cuda_index_Tensor, aoti_torch_get_size and
aoti_torch_index_put_out.
shoumikhin added a commit that referenced this pull request Oct 31, 2026
The pick was made on release/1.6, which does not have #24298 (the #24222
pick) yet. #24294 builds on #24222, so the pick depends on code that
release/1.6 lacks, such as _nodes_written_in_place in exir's
constant_prop_pass, and its conflict resolution brought in some of
#24222's OpenVINO changes but not others. This merges a clean pick of
7b14148 onto #24298's branch and takes the clean pick's side in the
OpenVINO files, so that, on top of #24298, the change is exactly #24294
(7b14148). A plain merge of the two would keep two copies of
_PRECISIONS in preprocess.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: arm Issues related to arm backend release notes: backends

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants