Skip to content

CUDA backend: CUDA graph fixes for mask indexing and changed input shapes, masked copies as where, tensor checks dropped before compiling, float64, _reshape_copy, scatter_reduce and varlen_attn, follow-ups - #24326

Open
shoumikhin wants to merge 71 commits into
pytorch:mainfrom
shoumikhin:cuda-graph-data-dependent

Conversation

@shoumikhin

@shoumikhin shoumikhin commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

What is wrong today

With enable_cuda_graph_for_method set, the CUDA backend runs a method a few times normally, captures a CUDA graph on the fourth call, and replays that graph after it. Some calls go wrong:

  • A call that cannot be captured fails. A mask index, such as x[mask] or x[mask] = v, reads a count back to the host, and a capture cannot record that. The capture call returned an error. An output that is first strided at the capture call failed the same way.
  • A replay can return a wrong result with no error. Replay checked only each input's byte count. An input that changes from (2, 3, 4, 4) to (2, 4, 3, 4) keeps its byte count, so it replayed the captured shape and returned the wrong output.

What this change does

  • Replay compares each input's shape with the captured one and refuses a different shape with InvalidArgument. The error names the dim and both sizes. A captured graph only works for the input shapes of the capture call, and the code comment on the graph phases now says so.
  • When a 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 retry is safe because a capture records kernels without running them, and it happens only if the new graph never started running. It covers failures that come back as errors; a scalar read during a capture, such as a torch.cond predicate on the GPU, still aborts, as before.
  • A method whose program keeps a constant in host memory, such as a torch.cond flag updated on every call, now runs without a graph. Host code runs only during the capture, so a graph would keep the branch of the capture call, and a retry after a failed capture would take the other one. The log names the constant found in host memory, or says the constants could not be listed.
  • An expected capture failure, such as the mask index refusing to run inside a capture, is logged at Info instead of Error, since the call then succeeds. A real failure of the call without a graph is still logged at Error. AOTInductor's generated code still prints its own line for the refused call.
  • The allocations a captured graph makes are recorded only after its first launch succeeds. Before, a failed first launch left addresses that were never allocated, and a later cleanup passed them to cudaFree. A failed capture now cleans up with CudaGraphState::release() instead of a copy of it.
  • Nothing changes in the exported program, so new programs still load on older runtimes, and older programs get the fix too.

Smaller commits:

  • The CUDA random shims seed from std::random_device instead of the clock, so two processes started in the same second no longer draw the same numbers. If std::random_device throws, or the copy of the seed to the GPU fails, the shim logs an error and frees what it allocated, and the next call tries again.
  • The wheel's shared library check now lists the mask index and index_put shims.
  • With PyTorch 2.14, a program that writes a one-element buffer, such as a counter, kept returning its first update, because Inductor folded the update into a constant. The AOTI backends (CUDA and Metal) now turn that folding off for a program that writes a buffer.
  • Unit tests for aoti_torch_get_size.
  • Comments on the index shims and on set_output_data_ptr that described accumulation, the sort count limit and the output index wrongly are corrected.
  • The mask index tests move to a new test_index_shims.py, since they need the Python runtime. They add a 2-D mask, and index_put with repeated indices in deterministic mode (half and bfloat16 accumulation, last write wins, an int32 index).
  • test_output_shape_changes_at_capture ends with a call that only a live graph rejects, so it fails if graphs are off. The KV cache recapture test checks that dropping the graph also clears the captured input shapes.

Follow-ups to #24314

  • Delete test_align_corners_compiles_with_dynamic_sizes. It could not fail: it passed with no resize rewrite at all and with the min and max in the align_corners ratio replaced by a plain if, because torch.compile compiles again for an output size of 1. With torch 2.14 and CI set it also failed for an unrelated reason, and it is the one failing test of the CUDA unit test job on main. test_bicubic_antialias_resize already runs an exported program at an output height of 1, where a plain if breaks.
  • The comment on that min and max now gives the real reason: an exported program keeps the branch it was traced with, so a plain if divides by zero at an output size of 1.
  • A backend that keeps its data in the .pte, such as Metal, now refuses a data_name compile spec built by hand too, before compiling anything. Before, only generate_data_name_compile_spec refused it, and a hand-built spec was ignored with no name check.
  • The CUDA docs warn that Windows and macOS usually treat data names that differ only in case as the same file.

Copies through a boolean mask no longer read the mask count back to the host

out[mask] = src[mask] ran as a mask index and a mask index_put. Each of them read the mask's count back to the host and waited for the GPU, on every call. Transformers' SmolVLM vision embeddings do this for their position ids.

When the values are the same mask's picks from a tensor shaped like the destination, every element goes back to where it came from. So a new pass in the CUDA backend writes the copy as torch.where(mask, src, out), which gives the same result, bit for bit. It also drops the index and the checks on its size when nothing else reads them and each check tests only the index's count and holds for any mask. A user check on the count, such as torch._check(x[mask].shape[0] >= 1), keeps the index. The pass compares the two masks with a work list, so masks with long histories do not hit Python's recursion limit. On an A100, five calls of the SmolVLM position id code went from 10 count reads and 10 stream waits to none.

Tensor checks no longer stop the CUDA export

The argument checks in torch.distributions export as aten._assert_async.msg. The CUDA backend compiled each one into a device assert inside a Triton kernel. While compiling, AOTInductor runs every kernel once to pick its launch settings, on made-up buffers: random floats, and zeros for integers and bools. So a check that reads a bool from an earlier kernel saw false and failed, even though it holds for every real input. The export then stopped with "device-side assert triggered". A Gaussian policy head, MultivariateNormal(mean, scale_tril=...) with 14 actions, hits this.

A new pass in the CUDA backend now removes these checks before compiling, so the CUDA delegate no longer runs them. The rest of ExecuTorch already removes the same checks before a program is saved. A failed device assert could not come back as an error anyway: it breaks the CUDA context for the rest of the process. Checks on sizes (_assert_scalar) stay, since they run on the host.

float64 weights load in the CUDA backend

A program with a float64 weight lowered and saved, then failed to load with Malformed CUDA FQN weight metadata. This happened with a float64 buffer, and with float64 math that constant propagation folds into one constant, such as a sinusoidal time embedding. The export wrote the weight correctly, but the runtime had no float64 dtype: the weight cache refused it, and the runtime's tensor type and dtype shims had none either.

  • The runtime's tensor type now has Double, with its 8 byte size. The same type wraps a method's inputs and outputs, so a float64 input or output works too.
  • The shims export aoti_torch_dtype_float64, which compiled code calls to name the dtype of a float64 constant.
  • The weight cache accepts Double, and uses 8 bytes when it checks that a weight fits its storage.
  • The shims that still refused float64 accept it: a float64 .item() (aoti_torch_item_float64, which did not exist, so the first execute ended the process), the single-element read those entry points share, index_put with accumulate, and the sort shim. A float16 .item() had the same problem with aoti_torch_item_float16 and works now too.

aten._reshape_copy lowers to the CUDA backend

Torch-TensorRT turns aten.view into aten._reshape_copy. A view that TensorRT leaves to PyTorch reaches the CUDA backend in that form, and the export failed with missing fallback kernels: aten::_reshape_copy, since Inductor has no lowering for it and torch registers no decomposition. The CUDA backend now decomposes it the way ATen computes it: a contiguous copy, viewed to the new shape.

varlen_attn lowers to the CUDA backend

torch.nn.attention.varlen.varlen_attn runs attention over several sequences packed into one tensor. Lowering it failed with missing fallback kernels: torch_attn::_varlen_attn, since it calls Flash or cuDNN attention, which this backend does not ship. The CUDA backend now decomposes it into ops Inductor lowers: attention over all the packed tokens, with a mask that keeps each query in its own sequence. The output is contiguous, like the op's output for a query in the usual (tokens, heads, dim) order, so reshaping it before the output projection works.

The decomposition matches the op for causal masks, sliding windows, grouped-query attention, and query and key sequences of different lengths. For a query sequence longer than max_k + 1, Flash attention 2 (PyTorch's default) and Flash attention 3 give different results for some windows, because version 2 changes the window based on max_k. So a window other than (left, 0) lowers only when max_q <= max_k + 1 is known at export time, as in self-attention, and raises a clear error otherwise. seqused_k and block_table (paged KV) raise a clear error too. Tokens before the first sequence or past the last one, such as the padding of a fixed-size buffer, do not change any sequence's output, even when they hold NaN. Unlike in Flash attention, a NaN or inf in one sequence's values does reach the other sequences. It holds a float32 score per head for every pair of packed tokens and runs much slower than Flash attention, so it suits packs of a few thousand tokens, such as a few camera images.

scatter_reduce with amax, amin, prod and mean lowers to the CUDA backend

Lowering x.scatter_reduce(dim, index, src, "amax") failed with missing fallback kernels: aoti_torch_cuda_scatter_reduce_two_out, and so did "amin", "prod" and "mean". Inductor writes a scatter itself only when it can add atomically, and leaves the other reductions to an ATen kernel that this backend did not ship. A sum into int64 or bool, a sum into bfloat16 on GPUs older than sm_90, and deterministic mode use the same kernel.

  • A new shim next to the index shims implements it the way ATen does: include_self, elements the index does not name, the count and integer rounding of the mean, NaN in amax and amin, and bool for sum only. Half and bfloat16 sums add in float, as ATen does in deterministic mode. A product rounds to the tensor's dtype at every step and a mean rounds its sum before dividing, as ATen does, so half overflows and integers wrap where ATen's do. A mean also counts its terms in the tensor's dtype, as ATen does, so 128 int8 ones average to 1.
  • It sorts the writes by the element they land on and reduces each element's values in index order, so the result is deterministic. It also works inside a CUDA graph. The sort is now one helper that index_put uses too.
  • It is slower than ATen's atomic kernel: 1.5 to 3.2 times on an A100 at 512 output rows, 4096 source rows and 1024 columns, and up to 7 times when many values land on few elements. A decomposition into ops Inductor lowers natively was measured first. It has to compare every output element with every source element, and it was 8 to 89 times slower than ATen's kernel.
  • aoti_cuda_shims.lib and the wheel's shared library check list the new shim. The import library now also lists the shims the DLL exported but it left out, such as aoti_torch_dtype_float16 and the aoti_torch_item_ getters, so a float16 model lowered for Windows links.

CUDA programs load without torch on machines with no libgomp

On Linux, Inductor adds OpenMP (-fopenmp, -lgomp) to every link. Unless the compiler passes --as-needed to the linker, as Ubuntu's GCC does by default, the compiled library lists libgomp.so.1 as a dependency even when it calls no OpenMP function. PyTorch ships a libgomp, so this went unnoticed while torch was imported. A process without torch failed to load the program with libgomp.so.1: cannot open shared object file on any machine with no system libgomp, such as a minimal container.

The CUDA backend now links with -Wl,--as-needed, so the linker keeps only the libraries the program uses. A program that does call OpenMP keeps libgomp. PyTorch already does the same for MPS builds on macOS, and pytorch/pytorch#202902 adds it to Inductor for Linux. The torch-free tests in test_pybindings_cuda_dlpack.py now run without a system libgomp, and the ones that check which libraries got loaded also check for libgomp. On an A100, a small MLP and CNN exported with this change load without torch and match eager where the loader cannot find libgomp.

A failed CUDA load no longer leaves its library in the temporary directory

To load a method, the CUDA backend writes the program's compiled library to a temporary file, such as /tmp/executorch_cuda_<pid>_0.so. Only destroy() removed that file, and destroy() never runs when the load fails, so every load that failed after writing the file left it behind. A scope guard now removes the file unless the load succeeds. test_missing_weights_blob.py now checks that a load that fails for a missing weights blob leaves nothing in the temporary directory. Without the fix, that check fails.

The XNNPACK high-rank permute fix moved to #24380.

The Triton SDPA leaves keys past the end out of the softmax

With triton_kernel_mode on (the default), the CUDA backend runs scaled_dot_product_attention as its Triton kernel, triton::sdpa. For power-of-two head sizes, its main kernel reads keys in tiles of 32, 64 or 128. When the number of keys is not a multiple of the tile, the last tile reaches past the last key. The kernel loaded those positions as zeros but never removed them from the softmax, so without an attention mask or a causal mask each one got a score of 0 and took part of the attention weight. Calls that pass kv_len with neither a mask nor is_causal, such as the DFlash draft model's global attention layers, had the same problem. On an A100, with the query at 0 and the values at 1, where the right output is 1, one key gave 0.031, 33 keys gave 0.516 and 80 keys gave 0.625. With random inputs and one key, the output was off by 3.4 when the largest value was 3.67. All of these now match eager PyTorch.

The kernel now gives every position past the last key, or past kv_len, a score of minus infinity, as the other attention kernels in sdpa.py already do. The first version of the kernel did this, and a later rewrite dropped it. Checking every tile cost up to 8% where no tile reaches past the end, so only calls that need the check pay for it. A mask or a causal bound already drops those keys, so calls with one skip it. The launcher also passes the largest tile size that divides the number of keys, and tiles that divide it skip the check at compile time. So calls with a mask or a causal bound, and tiles that divide a key count known at export time, compile to the same code as before. Without a mask or a causal bound, a kv_len or a symbolic key count checks every tile. test_triton_sdpa.py drops its note that called this a kernel limitation, runs its basic test with a single key, and runs every tile without a mask over 80 and 96 keys and over a kv_len of 80. These fail without the fix.

Testing

On an A100, with the CUDA backend built from this branch:

  • test_index_shims.py, test_output_stride_rearrange.py, test_cuda_export.py, test_antialiased_resize.py, test_merge_ptes.py and test_cuda_weight_metadata.py pass.

  • With the backend built from main, the new CUDA graph tests fail.

  • The replay error names the dim, for example "input 0 has size 4 in dim 1, captured with 3".

  • test_cuda_kv_cache passes, and fails when release() keeps the captured input shapes.

  • With the min and max replaced by a plain if, test_bicubic_antialias_resize fails.

  • With the new data name refusal removed, or moved back behind the compile, the data name test fails.

  • A torch.cond on a host-memory flag matches eager on every call; with a graph it returned the captured branch.

  • A random program run from two processes started together drew the same numbers in 2 of 3 tries before the seed change, and different ones in 3 of 3 after.

  • Mask index programs exported from this branch match eager PyTorch on every call, both on a runtime built from main with CUDA graphs off and on this branch's runtime with CUDA graphs on.

  • The shared shim unit tests pass in a standalone googletest build.

  • test_replace_masked_copy_with_where.py passes, and fails when the pass does nothing, is not registered, or skips its accumulate, same-mask, boolean mask, shared picks or count check. Two masks computed with over a thousand ops each raised RecursionError before. A check that reads the index's count but tests another size keeps the index; before, the pass removed it.

  • A masked copy lowered through the CUDA partitioner matches eager for masks that pick some, none and all positions. The test records the graph the pass returns, and fails when the pass is not registered or leaves the index in place.

  • A module that adds one to a device buffer on every call and reads a mask's count returns the call number on every call, so the capture call that runs again without a graph updates the buffer once. With the retry made to run the call twice, the capture call returns one too many. With PyTorch 2.14 it returned one on every call even without a graph, because Inductor folded the written buffer into a fill; it passes now with 2.14 and with 2.15 and 2.16 nightlies.

  • The mask tests check the runtime log for the failed capture, so they fail when CUDA graphs are off.

  • With the debug check on the phase before the retry turned into a release check, the mask tests pass.

  • test_distribution_argument_check passes with PyTorch 2.14 and fails with "device-side assert triggered" without the new pass. It writes the MultivariateNormal check as torch._assert_async, since PyTorch 2.14 cannot export the distribution itself. It also checks that the pass drops the check and the math that only fed it. A Gaussian policy head with hidden sizes 16, 64 and 256 exports, runs, and matches eager PyTorch.

  • A float64 buffer and a folded float64 constant load and match eager PyTorch. Before, both failed to load. test_float64_weight_loads fails without the fix.

  • test_reshape_copy fails without the fix. Programs that call _reshape_copy on transposed, sliced and contiguous inputs match eager PyTorch.

  • test_varlen_attn fails without the fix. It lowers a program that flattens the output, compares the decomposition with the op, including a window in self-attention and NaN values outside the key sequences, and checks that a window the Flash attention versions disagree on raises. It also fails when the output is left non-contiguous, when the window check is removed or refuses every such window, and when the values outside the key sequences are not zeroed. Programs that call varlen_attn with no window, a causal mask, a window (8, 0) and a window (4, 2), then reshape the output, with cu_seq_q and cu_seq_k given only at run time, lower, run and match eager PyTorch. With max_q and max_k from .item(), the decomposed program compiles with AOTInductor and the backend's options, calls no fallback kernel, and matches the op, also for windows (4, 4) and (8, -1).

  • test_missing_weights_blob.py, test_cuda_weight_reuse.py and test_device_placement.py pass.

  • test_scatter_reduce now runs the sum too, which for int64 goes to the new shim. The test that a missing scatter kernel fails at export now lowers an int64 scatter without a reduction, whose kernel the backend still lacks, so it still checks the scatter fallback now that the scatter_reduce shim exists. It fails when the check stops watching that fallback. test_scatter_mean_counts_in_its_dtype fails with the old count and when a count that wraps to zero is not turned into one. test_float64_and_float16_values (a sort and .item() in each dtype) and the float64 masked accumulate fail with the old shims, and all pass now. The .item() scale is a whole number because of an AOTInductor bug: its Triton launcher takes a float read with .item() as an integer, so a fraction is lost.

  • test_scatter_reduce and test_sort_shim.py pass. test_scatter_reduce fails without the fix, and also when the shim ignores include_self, rounds the integer mean toward zero, or lets a NaN lose in amax and amin. Programs with random float32, float16 and bfloat16 data, 3-D tensors, a negative dim, a strided src, overflowing int8, int32 and half values, half and bfloat16 sums of 1000 values in deterministic mode, bool sums and an empty index match eager CUDA. With CUDA graphs on, a scatter_reduce program matches eager on every replay.

  • The weight cache, runtime tensor and shim unit tests pass in a standalone googletest build. Without the fix, the weight cache test of a float64 entry fails.

  • With the seed copy made to fail once, the random shim logs it, fails that call, and seeds on the next one; before, it went on with an unseeded state.

Not tested: a failed first graph launch (it cannot be triggered from a test), a std::random_device that throws, Windows, macOS case-insensitive file systems, and the buffer folding change on Metal. No test covers the seed change, since a test cannot make two processes start in the same second.

Checked on real robot policies

On the release/1.6 stack with this change, on A100 and H100:

  • SmolVLA on the CUDA backend: one device-to-host copy per action chunk (only the action download), matching eager within the usual tolerance.
  • A Gaussian policy whose torch.distributions check used to stop the CUDA export: exports and runs, 0.76 ms per chunk.
  • A float64 weight that used to fail at load with "Malformed CUDA FQN weight metadata": loads and runs, max difference 0.
  • Four processes started together: four different random draws, against two out of four before.

A CUDA graph replays the shapes it captured. Before each replay, the
backend copied the new input into the captured input buffer, and it
checked only that the byte count matched. An input can change shape
without changing its byte count, for example from (2, 3, 4, 4) to
(2, 4, 3, 4). Such a call replayed the captured shape and returned an
output with the old shape and wrong values, with no error.

The backend now records each input's shape at capture, and a replay
with a different shape fails with InvalidArgument, the same error a
different byte count already gave.

test_output_stride_rearrange.py captures with (2, 3, 4, 4) inputs, then
calls with (2, 4, 3, 4), for contiguous and channels-last outputs. Without
this change those calls return the wrong shape.
The wheel check names every kernel shim that libaoti_cuda_shims must
define, so a library built without some of its CUDA sources fails the
check. The mask index and index_put shims were missing from that list.
Every getter in the shared AOTI shims has a unit test except
aoti_torch_get_size, so its handling of negative dims and of dims out of
range never ran. The new tests cover both, on CPU and CUDA, and the null
argument tests now include it.
The CUDA index shims run mask indexing and index_put. Their tests
covered only 1-D masks and int64 accumulation, and the test of a capture
call with a new output shape passed even when CUDA graphs were off.

test_mask_index_and_index_put moves from test_cuda_export.py to the new
test_index_shims.py. It runs programs through the Python runtime, and
test_cuda_export.py is also a Buck test target that does not depend on
that runtime. The new file adds a 2-D mask, and index_put with repeated
indices in deterministic mode, which sends half and bfloat16
accumulation, the last-write-wins case and an int32 index to the shim.

test_output_shape_changes_at_capture now ends with a call at the warmup
batch size. A live graph rejects it with InvalidArgument, because a
replay needs the captured input shape, so the test fails if graphs are
off.
Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:02
@pytorch-bot

pytorch-bot Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

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 30, 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
shoumikhin force-pushed the cuda-graph-data-dependent branch from d3a670f to a5c0e8b Compare October 30, 2026 05:20
Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:20

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.

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.
@shoumikhin
shoumikhin force-pushed the cuda-graph-data-dependent branch from a5c0e8b to fdbc332 Compare October 30, 2026 05:28
Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:28

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 30, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 24m 41s —— View job


This PR makes CUDA graph replay reject changed input shapes and reruns a failed capture call without a graph; I found no blockers, only minor issues: an undocumented replay contract, Error logs on calls that now succeed, two run_once structure nits, a pre-existing stale-pointer case, and two small test gaps.

PR Review: #24326

Summary

Replay now compares input shapes instead of byte counts. When a capture fails before its graph launches, the same call runs again without a graph. Three independent reviewers found no blockers. Everything below is minor, and the first item is a design question.

API Design

  • A shape change after capture is still a hard error (backends/cuda/runtime/cuda_backend.cpp:773-778).
    • A failed capture now falls back to running without a graph, but a replay with a new input shape still returns InvalidArgument. For a method exported with a Dim, every later call whose shape differs from the capture call's fails. One example is test_output_shape_changes_at_capture call 6, which uses batch 1 under Dim(min=1).
    • This is not a regression, because a change in byte count already failed on main.
    • The contract isn't written down anywhere. The Replay bullet of the phase comment (cuda_delegate_handle.h:118-121) doesn't mention it, and no Markdown doc covers enable_cuda_graph_for_method. Please either document it there or fall back instead.
    • A fallback needs two more changes:
      • The check has to run over all inputs before the first copy is queued. Today it runs inside the copy loop.
      • Replay has to resize outputs back to their captured shapes. It relies on them staying fixed (:1098-1099), but an eager call resizes them (utils.h:114-116).

Code Quality

  • The replay refusal no longer says what differs (cuda_backend.cpp:777).
    • The old message printed the expected and actual byte counts; the new one prints only the input index.
    • Report both dim counts when they differ, and otherwise the first differing dim with both sizes. Fix this →
  • The main mask case still logs two Error lines on a call that now succeeds (shims/index.cu:337-340, cuda_backend.cpp:1064-1068).
    • On the 4th call of every mask-index method, the shim refuses through ET_CHECK_OR_RETURN_ERROR, and then the run() failure check logs again. Both lines are Error and come before the Info fallback line.
    • The PR moved the two layout messages (:1090, :1105) to Info for this reason. Log these two at Info during capture too; the eager retry still reports real failures at Error.
    • In the layout and resize cases, the fallback is now announced twice at Info: once at :1090 or :1105, then again at :820. Fix this →
  • run_once splits the public overrides (cuda_backend.cpp:831, :1227). It opens a new private:/public: pair between execute and destroy, but the class already has private helper sections at :169 and :1287. Fix this →
  • graph_ran is a side-channel out-parameter (cuda_backend.cpp:835-841).
    • Only the capture branch sets it. The retry at :825 passes it again with is_capture_step = false, where nothing writes it.
    • Splitting the capture attempt from the eager run would let the capture function report directly whether its graph launched.
  • Pre-existing: stale graph_allocations after a failed first launch (cuda_backend.cpp:1124, next to the new graph_ran = true).
    • note_graph_allocations() runs before the first cudaGraphLaunch. If that launch fails, CaptureGuard destroys the graph but keeps graph_allocations, and release() later calls cudaFree on addresses of a graph that never ran.
    • Suspected effects: a stray error left for the next cudaGetLastError(), or freeing another graph's allocation if the address was reused.
    • Recording the allocations after the launch succeeds fixes this. It would also let CaptureGuard call release() instead of hand-copying it; this PR had to extend that copy and the original separately (:878, cuda_delegate_handle.h:233). Fix this →

Testing

  • The recapture test doesn't cover static_input_sizes (backends/cuda/runtime/test/test_cuda_kv_cache.cpp:933-952).
    • It seeds and checks only static_input_ptrs and static_input_nbytes.
    • If release() stopped clearing the new field, a recapture after KV growth would append a second set of shapes, and replay would check inputs against the first capture's shapes. Fix this →
  • assertIn("error 0x12", str(out)) also matches a set_inputs() rejection (test_output_stride_rearrange.py:402, :432).
    • The pybindings use the same "failed with error 0x…" wording for method->set_inputs() (extension/pybindings/pybindings.cpp:3771, :3940) as for execute() (:3995).
    • Match "execute() failed with error 0x12" so that only the replay check passes. Fix this →

Recommendation

Approve. Nothing blocks merging. The replay contract needs a decision, plus a comment documenting it if the hard error stays. The rest are small cleanups.


Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:58

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 30, 2026 •

Copy link
Copy Markdown

Claude finished @shoumikhin's task in 27m 4s —— View job


This PR makes CUDA graph replay reject changed input shapes and runs a failed capture call again without a graph; nothing blocks merging, but the hard error on a new input shape after capture should be documented or replaced with a fallback, and there are smaller logging, structure and test issues.

PR Review: #24326

Summary

Replay now checks input shapes instead of byte counts. When a capture fails before its graph launches, the same call runs again without a graph. Three independent reviewers covered all 8 changed files and found no blockers. The first item below is a design question and the rest are minor. This was a static review: there's no GPU on this runner, so nothing was built or run.

API Design

  • After capture, a new input shape is a hard error, and nothing documents that (backends/cuda/runtime/cuda_backend.cpp:773-778).
    • A failed capture now falls back to eager. A replay whose input shape differs from the capture call's still returns InvalidArgument, even inside the exported Dim bounds, and it does so on every later call with that shape.
    • test_output_shape_changes_at_capture now uses that refusal on call 6 (batch 1 under Dim(min=1)) as its proof that graphs are on, so it locks the behavior in.
    • This isn't a regression, because a change in byte count already failed on main.
    • The Replay bullet of the phase comment (cuda_delegate_handle.h:118-121) doesn't mention the rule, and no .md file covers enable_cuda_graph_for_method. Please either document it there or fall back to eager.
    • A fallback also needs replay to resize outputs back to their captured shapes. Replay relies on them keeping that shape (cuda_backend.cpp:1098-1099), but an eager call resizes them (utils.h:114-116).

Code Quality

  • The replay refusal no longer says what differs (cuda_backend.cpp:777).
    • The old message printed the expected and actual byte counts, and the new one prints only the input index.
    • Print both shapes with tensor_shape_to_c_string, as cuda_kv_pool.cpp:48-51 does. Fix this →
  • A mask-index method still logs two Error lines on a call that now succeeds (shims/index.cu:337-340, cuda_backend.cpp:1064-1068).
    • On the capture call, both the shim's refusal ("a mask index cannot run inside a CUDA graph capture") and the AOTInductorModelContainerRun failed check log at Error. Then the retry returns Ok.
    • The PR moved the layout and resize messages (:1090, :1105) to Info for this reason. These two should be Info during capture too; the eager retry still reports real failures at Error.
    • In the layout and resize cases, the fallback is now announced twice at Info: at :1090 or :1105, then again at :820. Fix this →
  • The retry depends on an out-parameter and on a side effect of CaptureGuard (cuda_backend.cpp:814-826, :835-841, :888, :1150).
    • execute() decides whether to retry from bool& graph_ran. One line of a 390-line function sets it, and it is set when the launch is queued, not when the graph has run. The retry at :825 passes it to a call that never writes it.
    • The retry only runs eagerly because CaptureGuard's destructor has already set the phase to Disabled (:888). That selects handle->run (:1040-1044) and skips the warmup decrement (:1173). If the guard stopped doing that, the retry would run as a warmup step and the next call would capture again.
    • A separate capture function that returns whether its graph launched would make both explicit.
  • run_once splits the public overrides (cuda_backend.cpp:831, :1227).
    • It opens a new private:/public: pair between execute and destroy, while the class already has private sections at :169 and :1287. Fix this →
  • Low confidence: when the .so lacks run_single_threaded, the retry probably fails too (cuda_backend.cpp:817-825, cuda_delegate_handle.h:290-295).
    • Without that export, the capture step calls run(), which records AOTI's completion event inside the capture.
    • If the capture then fails after run() returned (output layout, resize, end capture, instantiate, launch), the retry's run() queries that event. The header comment says that query fails.
    • The mask case should be unaffected, because the shim fails before run() records the event. This is no worse than main, but the comment at :818-819 claims more than holds. Either skip the retry when run_single_threaded is null, or note the limit there.
  • Pre-existing: stale graph_allocations after a failed first launch (cuda_backend.cpp:1124, next to the new graph_ran = true at :1150).
    • note_graph_allocations() runs before the first cudaGraphLaunch. If that launch fails, CaptureGuard destroys the graph but keeps the list, and release() later calls cudaFree on allocations of a graph that never ran.
    • Suspected effects: a stray error left for a later cudaGetLastError(), or freeing another graph's allocation if the address was reused.
    • Recording the allocations once the launch succeeds fixes this. It would also let CaptureGuard call release() instead of a hand-copied version of it. This PR had to extend both copies separately (:878, cuda_delegate_handle.h:233). Fix this →

Testing

  • No test checks the retry's safety claim with state (backends/cuda/tests/test_output_stride_rearrange.py:404-414).
    • _MaskedRows and _MaskedAdd are stateless, so a retry that applied an update twice would still pass.
    • Add a module that increments a registered buffer before x[mask] and returns something that depends on it. The AOTI partitioner keeps mutated buffers in the delegate (aoti_partitioner.py:104), and the existing per-call comparison with eager would catch a double update. Fix this →
  • The recapture test doesn't cover static_input_sizes (backends/cuda/runtime/test/test_cuda_kv_cache.cpp:933-934, :951-952).
    • It seeds and checks only static_input_ptrs and static_input_nbytes.
    • If release() stopped clearing the new field, a recapture would append a second set of shapes, and replay would compare inputs against the first set. Fix this →
  • assertIn("error 0x12", str(out)) also matches a set_inputs() rejection (test_output_stride_rearrange.py:402, :432).
    • The pybindings use the same "failed with error 0x…" wording for set_inputs() (extension/pybindings/pybindings.cpp:3771, :3940) as for execute() (:3995).
    • Match "execute() failed with error 0x12" so that only the replay check passes. Fix this →
  • test_two_dim_mask draws an unseeded random mask (backends/cuda/tests/test_index_shims.py:88), so a failure that depends on the mask can't be reproduced. Seed it, as test_output_stride_rearrange.py does. Fix this →

Recommendation

Approve. Nothing blocks merging. The replay shape rule needs a decision, plus a comment documenting it if the hard error stays. The rest are small cleanups.

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.
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.
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.
The KV cache recapture test now also checks that dropping the graph
clears the captured input shapes, so a recapture cannot replay against
the first capture's shapes. The replay tests match the execute() error,
so a set_inputs() failure with the same code no longer passes them.
test_align_corners_compiles_with_dynamic_sizes could not fail. It passed
with no rewrite at all, since torch.compile then calls ATen's kernel, and
it passed with the min and max in the align_corners ratio replaced by a
plain if, since torch.compile treats an output size of 1 as a constant and
compiles again. With torch 2.14 and CI set it also failed for a reason
unrelated to the resize. test_bicubic_antialias_resize already runs an
exported program at an output height of 1, which is where a plain if
breaks.
The comment on the min and max in the align_corners ratio blamed
torch.compile, which handles a conditional by compiling again. The real
reason is export: an exported program keeps the branch it was traced
with, so a plain if divides by zero at an output size of 1 when the
program was traced with a larger one.
@shoumikhin shoumikhin removed the release notes: xnnpack Changes to the XNNPack backend delegate label Oct 31, 2026
The comment on sequence_ends in the CUDA backend's varlen_attn
decomposition called every distance negative. The clamp measures the
tokens past cu[-1] from cu[-1] instead, so theirs are 0 or more.

The comment on the contiguous output said the op's output is
contiguous. The op lays it out in the query's dimension order, so it is
contiguous for a query in the usual (tokens, heads, dim) order, even a
slice of a packed tensor.

Only comments change.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 08:46

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. Try requesting a review at a lower effort level.

@shoumikhin

Copy link
Copy Markdown
Contributor Author

@claude /pr-review

@claude

claude Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Approve, with no blockers. Three things to settle: a constant-folding switch that now applies to every program with mutable buffers and has no performance data; a .item() read that aborts during CUDA graph capture, before the new retry can run; and runtime checks the description reports that are not in the committed tests.

Detailed review

PR Review: #24326

Summary

This PR changes the CUDA backend in four ways:

  • A CUDA graph method now runs eagerly after a failed capture, or when the method keeps constants in host memory.
  • Masked copies are rewritten as where.
  • Tensor checks are dropped before compiling.
  • It adds float64, _reshape_copy, scatter_reduce and varlen_attn.

No blockers. The issues found are:

  • a program-wide folding switch with no performance data
  • a capture abort that the new retry cannot catch
  • a pass that skips subgraphs
  • runtime checks from the description that are missing from the tests

Code Quality

  • .item() on a device tensor aborts the process during a CUDA graph capture, so the new retry never runs.

    • read_one_element (backends/cuda/runtime/shims/memory.cpp:69-100) reads through SlimTensor::item() (backends/aoti/slim/core/slim_tensor.h:549-551), which calls DeviceTraits<CUDA>::memcpy (backends/aoti/slim/core/storage.h:185-192).
    • That function does a blocking copy, or an async copy followed by a stream sync, and checks the result with ET_CUDA_CHECK, which aborts on error.
    • Neither is allowed on a capturing stream, so no error ever reaches the retry at backends/cuda/runtime/cuda_backend.cpp:840.
    • mask_positions now checks cudaStreamIsCapturing first and returns NotSupported (backends/cuda/runtime/shims/index.cu:336-344). The item shims need the same check.
    • The abort predates this PR. This PR routes float64 and float16 through the same path (memory.cpp:83-87, :479-480).
    • This comes from reading the code; I did not run it.

    Fix this →

  • Float .item() values reach Triton kernels truncated, and only a test comment says so.

    • backends/cuda/tests/test_index_shims.py:222-225 uses a whole number to avoid an AOTInductor bug: the Triton launcher takes the float as int64_t, so 1.5 becomes 1.
    • Before this PR, float64 and float16 programs that call .item() failed on the missing aoti_torch_item_float64 and aoti_torch_item_float16 symbols. With the new shims (memory.cpp:479-480), they run and silently use the truncated value.
    • Link the upstream issue in the test comment and note the limitation in docs/source/backends/cuda/cuda-overview.md.

    Fix this →

Testing

  • The runtime checks the description reports are not committed, and the new decompositions are never run compiled.

    • test_reshape_copy and test_varlen_attn (backends/cuda/tests/test_cuda_export.py:483-530) only lower the program. The varlen_attn cases compare the decomposition with the op in eager, so the program AOTInductor compiles never runs in a test.
    • The description says varlen_attn programs "lower, run and match eager PyTorch" with no window, a causal mask, windows (8, 0) and (4, 2), and cu_seq given at run time. None of those runs is in the tests.
    • test_scatter_reduce (backends/cuda/tests/test_index_shims.py:169-193) covers 2-D tensors, dim=1, a contiguous src, and four dtypes (float32, float16, bfloat16, int64). The shim also dispatches float64, int32, int16, uint8 and bool (backends/cuda/runtime/shims/index.cu:1419-1439).
    • The description's 3-D, negative-dim, strided-src, int32 and half overflow, bool-sum, empty-index and CUDA graph replay cases are also missing from the tests.
    • TestIndexShims._run and _run_with_cuda_graph already lower, run and compare with eager, so each case takes a few lines.

    Fix this →

Performance

  • Joint-graph constant folding is now off for every program that writes any buffer (backends/aoti/aoti_backend.py:56-68, applied at :310).

    • Any entry in buffers_to_mutate turns it off. Every CUDA or Metal program that keeps a KV cache in mutable buffers now compiles without folding.
    • The docstring blames one-element buffers before torch 2.15, but the gate checks neither the torch version nor the buffer size.
    • The docstring says the option "keeps the buffers a program writes out of constant folding". It actually turns folding off for the whole program.
    • The description gives no latency numbers and lists Metal as untested.
    • The same switch at backends/cuda/cuda_backend.py:1338-1350 applies only to cell_layout programs.
    • Suggest: gate it on torch older than 2.15, fix the docstring, and post a decode-latency comparison for one KV-cache model. If one-element buffers are the only trigger, narrow the gate to them; _MaskedCounter already covers that case.

    Fix this →

  • The masked-copy rewrite skips torch.cond branches and other subgraphs.

    • backends/cuda/passes/replace_masked_copy_with_where.py:90 walks only the top-level graph.
    • RemoveAssertAsyncPass, also new in this PR, walks graph_module.modules() (backends/cuda/passes/remove_assert_async.py:33).
    • A masked copy inside a branch therefore keeps the index shims, along with their host read of the count on every call.

    Fix this →

Recommendation

Approve

Nothing blocks merging. Before merging, I'd measure or narrow the folding switch, since it changes how every KV-cache program on CUDA and Metal compiles.

SS-JIA's approval (2026-10-30T14:37Z) came before the commits that add RemoveAssertAsyncPass, float64, _reshape_copy, scatter_reduce, varlen_attn and the folding switch. A CUDA backend owner should look at those parts again.

  • Gather context
  • Run three independent reviewers
  • Consolidate findings
  • Fact-check findings
  • Post review

View job run

…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.
The runtime.cuda docstring and the pybindings README said that loading a method's CUDA weights waits for the whole GPU. Only weights an older export keeps in one blob do that (cudaDeviceSynchronize). Current exports copy each weight with a plain cudaMemcpy, which waits for the legacy default stream and blocking streams but not for a non-blocking stream. Measured with a 2 s kernel on a non-blocking stream: load_method took 0.01 s. Docs 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.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 16:04

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. Try requesting a review at a lower effort level.

Summing half and bfloat16 in float keeps more precision, but a term far smaller than the running sum can still vanish, as the index_put comment already says. Comment only.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 17:27

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. Try requesting a review at a lower effort level.

A CUDA backend program carries a shared library that AOTInductor
compiles. Inductor puts OpenMP (`-fopenmp`, `-lgomp`) on every Linux
link. Unless the compiler asks the linker to link as needed, as Ubuntu's
GCC does by default, the linker records `libgomp.so.1` as a dependency
even when nothing in the library calls OpenMP. Programs exported with
GCC on Red Hat based systems, or with Clang, carry it. PyTorch ships its
own libgomp, which hides this while torch is imported. A process
without torch then fails to load the program on any machine that has no
system libgomp, such as a minimal container:

    Failed to load /tmp/executorch_cuda_<pid>_0.so with error:
    libgomp.so.1: cannot open shared object file: No such file or
    directory

Link with `-Wl,--as-needed` while the CUDA backend compiles. The linker
then records only the libraries the program references, so an unused
libgomp is dropped, while a program that does call OpenMP keeps it.
PyTorch already does the same for MPS builds on macOS. The flag applies
only to ahead-of-time links on Linux that neither link libtorch nor
cross-compile for Windows, and only in threads inside the backend's
compile context.

The torch-free tests in `test_pybindings_cuda_dlpack.py` no longer probe
for libgomp before they run. The ones that check which libraries got
loaded now also require that libgomp is not among them, so they fail on
any Linux host if the dependency comes back. That check now matches
library file names only. The pybindings README no longer asks users to
install libgomp.

Test plan, on Linux x86_64 (CentOS Stream 9) with an A100:
- Exported a small MLP and CNN before and after this change. Before, the
  compiled library lists `libgomp.so.1` and references no OpenMP symbol.
  After, `libgomp` and the unused `libm` are gone, and `libcuda`,
  `libstdc++`, `libgcc_s` and `libc` remain.
- Ran both programs from Python without torch, where the loader cannot
  find libgomp. Before fails with the error above. After loads, runs,
  and matches eager.
- With the same kernels, only the link line differs, and timing showed
  no change beyond noise.
- A graph whose CPU part Inductor parallelizes with OpenMP still records
  libgomp under the flag.
- The torch-free tests pass in both output modes. With the backend
  change reverted, the ones that check loaded libraries fail at the
  libgomp check, and without a system libgomp all of them fail to load.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 18:04

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. Try requesting a review at a lower effort level.

The CUDA backend swaps scaled_dot_product_attention for its Triton
kernel, triton::sdpa, when triton_kernel_mode is on, which is the
default. The kernel reads keys in tiles of 32, 64 or 128. When the
number of keys is not a multiple of the tile, the last tile reaches past
the last key. The kernel loads those positions as zeros, but it never
gives them a score of minus infinity. Without an attention mask or a
causal mask, each one enters the softmax with a score of 0 and a value
of 0, and takes weight away from the real keys. With every real score
equal, where the right output is 1, one key gave 0.031 and 80 keys gave
0.625. Passing kv_len without a mask has the same problem.

Set the score of every position at or past kv_len to minus infinity, as
the other attention kernels in this file already do. The first version
of this kernel did this, and a later rewrite dropped it.

Checking every tile made some shapes up to 8% slower, so the check only
runs where it can matter. A mask or a causal bound already drops those
keys, so calls with one skip it. The launcher also passes the largest
tile size that divides the number of keys, and tiles that divide it
skip the check at compile time. So every call that was already right
compiles to the same code as before. A kv_len, or a symbolic key count,
is only known at run time, so those calls check every tile.

The test file called this a kernel limitation, and its basic test only
used key lengths of 64 and up. Drop that note and run the basic test
with a single key too. A new test runs every tile without a mask over 80
and 96 keys and over a kv_len of 80. On 96 keys only the 32-key tiles
skip the check. Without the fix, these cases are off by up to 3.6.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 18:39

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. Try requesting a review at a lower effort level.

To load a method, the CUDA backend writes the compiled library from the
program to a temporary file, such as `/tmp/executorch_cuda_<pid>_0.so`,
and loads it from there. Only `destroy()` removes that file, and
`destroy()` only runs for a handle that `init()` returned. When `init()`
fails, for example because the library cannot be loaded or the weights
blob is missing, it returns no handle, so the file stays behind. Every
load that fails after writing the file leaves it in the temporary
directory.

Remove the file with a scope guard unless `init()` succeeds, so every
failure path removes it. A method that loads keeps its file until
`destroy()`, as before. Nothing else changes on a failure: the handle
of a late failure is still not freed, and, as in `destroy()`, a library
that was already loaded stays loaded.

`test_missing_weights_blob.py` now also checks that a load that fails
for a missing weights blob leaves nothing in the temporary directory.

Test plan, on Linux x86_64 with an A100:
- The extended test fails without this change, with the library left in
  the temporary directory, and passes with it.
- A load that fails because the library itself cannot be loaded also
  leaves no file now.
- The other tests in `test_missing_weights_blob.py` still pass.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 18:41

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. Try requesting a review at a lower effort level.

The CUDA backend adds `-Wl,--as-needed` to its AOTInductor links until
Inductor does it itself. pytorch/pytorch#202902 makes that change in
Inductor, so the TODO now names it: the hook can go once the pinned
PyTorch includes it.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 19:38

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. Try requesting a review at a lower effort level.

Lowering x.scatter(dim, index, src) to the CUDA backend failed for a
bfloat16, int64 or bool src:

    Method forward missing fallback kernels (1 total):
      - aoti_torch_cuda_scatter_src_out

Inductor writes a scatter from a tensor itself only in a dtype it can
add atomically, so it leaves int64, bool and, on GPUs older than sm_90,
bfloat16 to an ATen fallback kernel, and in deterministic mode every
dtype. This backend shipped the scatter_reduce shim but not this one. A
mixture-of-experts layer that writes its expert outputs back with
torch.zeros_like(h).scatter(1, index, src) in bfloat16 hits it on an
A100.

Add the shim next to the scatter_reduce shim, sharing its checks, its
sort of the writes by the element they land on, and its kernel: a
scatter is one more reduction, Assign, where each write replaces the one
before it. So repeated writes to an element keep the last one in index
order, as ATen does in deterministic mode; ATen's default CUDA kernel
keeps any one of them. As in ATen, an index out of range triggers a
device-side assert. It takes every dtype the scatter_reduce shim takes,
bool included.

Also allow it in the backend's list of fallback kernels, add the name to
aoti_cuda_shims.lib, the import library that lowering for Windows links
against, regenerated with mingw binutils 2.41 as its README describes,
and to the wheel's shared library symbol check.

Test: test_scatter_with_tensor_source lowers a scatter with repeated
indices and a column it never writes, in bfloat16, int64 and bool, runs
it and compares with eager CUDA in deterministic mode. Without this
change it fails at lowering. The test that a missing scatter kernel
fails at export lowered an int64 scatter from a tensor, which now works,
so it lowers a scatter of a scalar in deterministic mode instead, whose
kernel the backend still lacks.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 20:02

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. Try requesting a review at a lower effort level.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: backends

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants