Repository navigation
Fix CUDA mask indexing, CUDA graph outputs and the runtime output-buffer check - #24320
pytorchbot wants to merge 22 commits into
Conversation
…and in exir (#24222) 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. (cherry picked from commit 054de1f)
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.
…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). (cherry picked from commit ff1bab3)
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24320
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit b1f5f7e with merge base 4db778e ( BROKEN TRUNK - The following job failed but were 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. |
…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).
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.
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.
|
@claude /pr-review |
|
Claude finished @shoumikhin's task in 32m 48s —— View job PR Review: #24320Needs Discussion: with Detailed reviewSummaryThis cherry-pick of #24299 and #24326 makes four changes:
After fact-checking, one major issue (a mask index fails the CUDA graph capture call) and one minor test gap remain. Diff base: the shallow checkout has no merge base with Scope: 78 of those files are touched only by the #24222 cherry-pick this PR is stacked on ( Skimmed or not reviewed:
Code Quality
Testing
RecommendationNeeds Discussion The capture failure is not a regression, since a mask-index model could not be lowered to CUDA at all before this PR. It is still a deterministic error in the scenario this PR enables (SmolVLA) once CUDA graphs are on.
Review steps
|
With enable_cuda_graph_for_method set, a method that has a mask index, such as x[mask] or x[mask] = v, fails its fourth call, the one that captures the CUDA graph. The index shim has to read the number of selected elements back to the host, which it cannot do inside a capture, so the call returns an error. Only then does the backend turn graphs off, and the later calls run without one. Now, when the capture fails, the backend runs the same call again without a graph and returns its result. Graphs stay off for that method, as before. The same retry also handles other capture failures, such as an output that is first strided at the capture call. The retry is safe because a capture records kernels without running them, so until the new graph runs, the failed attempt has not touched the inputs or any state the program updates. A failure after the graph started running is not retried and fails the call, as before. The off-graph KV cache step is still prepared once and committed once. Nothing changes in the exported program, so programs load on older runtimes as before, and programs exported before this change get the fix too. test_output_stride_rearrange.py runs a mask index model and a mask index_put model with CUDA graphs requested for six calls, and compares each call with eager PyTorch. Without this change the fourth call fails. test_strided_output_first_seen_at_capture now expects that call to match eager PyTorch too, instead of failing. (cherry picked from commit fdbc332)
A mask index refuses to run inside a CUDA graph capture, and the backend then runs that call without a graph, which succeeds. But the refusal and the failed run were both logged at Error, so every method with a mask index printed two ExecuTorch error lines on its fourth call although nothing failed. Log both at Info during the capture. The call that runs without a graph still logs a real failure at Error. AOTInductor's generated code still prints its own line for the refused call, which the backend does not control. The layout and resize messages no longer say "running without a graph" themselves, because the backend already says that once for every failed capture. (cherry picked from commit 1d70dcc)
A CUDA graph records only the device work of the capture call. Host code runs once, during the capture, and never on replay. So for a program that keeps a constant in host memory, such as a torch.cond flag updated on every call, a graph is wrong either way: - a successful capture fixes the branch the capture call took, and replays keep returning it; - a failed capture has already run the host code, so running the call again without a graph takes the other branch. Both returned wrong results with no error. init now leaves the graph off for a method whose AOTI container has any constant in host memory, or when that cannot be checked, and logs why. Methods without host constants, such as the mask index ones, keep their graphs and the retry. The new test is a torch.cond on such a flag; with a graph it returned the captured branch on later calls. (cherry picked from commit 8c965a1)
Without the switch to Disabled, a capture that cannot succeed would be tried again on every later call, not on every fourth one: warmup is already over. (cherry picked from commit 5a43778)
…d away small terms The comment said half and bfloat16 values summed in float lose no small terms. Float still rounds, so a term far smaller than the running sum can vanish, for example many 2^-24 terms added one at a time to 1.0. Comment only.
No test passed a negative or int32 index to the CUDA index shim, so a shim that dropped the wrap or read int32 indices as int64 still passed. The mask test's column index is now -4 as int32, which eager PyTorch treats as column 2. With the shim's wrap removed, the test now fails with a device-side assert.
The replay check only said that an input had a different shape than at capture. It now names the dim and both sizes, or both dim counts, so the caller can see what changed. Also write down in the phase comment that a captured graph only works for the input shapes of the capture call, and that other shapes fail with InvalidArgument.
The backend recorded the allocations a captured graph makes before it launched the graph for the first time. A graph only allocates that memory when it launches, so if that first launch failed, a later cleanup called cudaFree on memory that was never allocated. Record them after the launch succeeds. A failed capture now cleans up with CudaGraphState::release() instead of a copy of it, so it also frees the memory a successful first launch allocated before a later step failed.
A method with a constant in host memory runs without a CUDA graph. The
init log said the method "keeps constants in host memory", also when the
constants could not be listed at all. It now names the constant it found,
or says the constants could not be listed:
CUDA graph requested for method 'forward', but constant 'flag' is in
host memory; running without a CUDA graph
The phase diagram in cuda_delegate_handle.h now shows when a method stays
Disabled and when Warmup falls back to Disabled. The MoveCondPredicateToCpu
pass docstring now says that a method it changes runs without a CUDA graph.
After a failed capture, later calls run eagerly only because the capture
guard has already set the phase to Disabled. A debug check before the
retry now states that.
Stacked on #24298. This branch starts with the #24222 cherry-pick, which is #24298. Please merge #24298 first, then update this branch. Until then GitHub also shows the #24222 files and their behavior changes here; they are described and reviewed in #24298. The change this PR adds is the #24299 cherry-pick and the #24326 follow-ups below, in the CUDA backend and the runtime.
What is wrong today
Robot policies and follow-up checks on #24222 found these bugs in the CUDA backend and the runtime:
x[mask]andx[mask] = valueshave no C shims, so lowering fails with "missing fallback kernels". This blocks SmolVLA.Method::set_output_data_ptrchecks 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:
InvalidArgument.Tests
Each new test fails without its fix.
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).
Follow-up fixes from #24326
This pick also carries fixes from #24326. A CUDA graph replay now refuses an input whose shape changed but kept its byte count. Before, such a call returned the captured output shape with no error, and the channels-last change above lets more outputs reach that path. The wheel's shared library check now lists the new index shims. Comments on the index shims and on
set_output_data_ptrthat described accumulation, the sort count limit and the output index wrongly are corrected.cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani