Vulkan: stop delegating nodes past their own support check - #21847
Open
msluszniak wants to merge 2 commits into
Open
Vulkan: stop delegating nodes past their own support check#21847msluszniak wants to merge 2 commits into
msluszniak wants to merge 2 commits into
Conversation
create_pattern_match_from_internal_match builds PatternMatch.all_nodes from
every entry in InternalMatch.nodes_map, which includes the target-graph nodes
bound to the PATTERN's placeholders. Those nodes merely feed the matched
subgraph; the fused op does not compute them, so the partitioner has no reason
to claim them.
That set becomes VulkanPartitioner's fusable_nodes, and _is_node_supported
returns True for anything in it before it consults the op's
are_node_inputs_supported_fn. Feeder nodes therefore have their support checks
silently waived, and an op can be delegated on shapes its implementation
rejects.
A rotary embedding hits this: `freqs_cos[input_pos]` feeds the hf_rope pattern,
so its aten.index.Tensor was delegated even though check_index_tensor_node
rejects a non-1-D `self`. The gather then read self[idx, 0, 0, 0] against a
(4096, 64) table, and resize_index_tensor_node set rank-1 sizes on a rank-2
tensor, aborting in virtual_resize with "new sizes cannot modify the
dimensionality of the tensor".
Exclude the placeholder bindings from the match. Since a gather along dim 0
never required a 1-D `self` in the first place, also generalize the op rather
than just declining it:
* check_index_tensor_node now requires only that the INDEX is 1-D.
* the buffer shader derives the gather axis from the output rank and passes
self's trailing coordinates through unchanged. The texture variant still
assumes the 1-D form, so pick_io_storage_fn routes a higher-rank `self` to
a contiguous buffer.
* resize_index_tensor_node computes index.sizes ++ self.sizes[1:] instead of
reusing the index's sizes, which was only correct for a 1-D `self`.
Verified on a Galaxy S26 Ultra (Adreno): LFM2.5-VL keeps its rotary embedding
inside the delegate and generates correctly.
VulkanBackend::execute computes `output_offset = args.size() - num_outputs` in size_t. If the serialized graph declares more outputs than the delegate call has arguments, that subtraction wraps and every subsequent output access reads through a wild pointer, so the failure surfaces as a SIGSEGV inside execute() with nothing pointing at the real cause. The reachable way to get there today is a mutated buffer serialized as a graph output. alias_buffer_mutations avoids that by aliasing the mutation onto the buffer's own value, but it is opt-in, and with it off a mutation that was functionalized into a computed node -- rather than arriving as the buffer placeholder, as sdpa_with_kv_cache's KV caches do -- still lands in output_ids. A model with such a buffer then declares more outputs than the call supplies and segfaults; LFM2.5's short-conv states declared 11 against a 3-argument call. Check the counts up front so this is a readable error naming both numbers.
pytorch-bot
Bot
added
the
module: vulkan
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21847
Note: Links to docs will display an error until the docs builds have been completed.
|
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two Vulkan backend fixes found while bringing up LFM2.5-VL (a hybrid
attention + gated short-conv VLM) on Android. Neither is specific to that model.
1. Nodes were delegated past their own support check.
Concretely: a rotary embedding's
freqs_cos[input_pos]feeds thehf_ropepattern, so its
aten.index.Tensorwas delegated even thoughcheck_index_tensor_noderejects a non-1-Dself. The gather then readself[idx, 0, 0, 0]against a(4096, 64)table, andresize_index_tensor_nodeset rank-1 sizes on a rank-2 tensor, aborting in
virtual_resizewith"new sizes cannot modify the dimensionality of the tensor".
This PR excludes the placeholder bindings from the match.
The texture shader still assumes the 1-D form, so
pick_io_storage_fnroutes a higher-rank
selfto a contiguous buffer.2.
VulkanBackend::executeunderflows on a delegate arg-count mismatch.output_offset = args.size() - num_outputsissize_t. If the serialized graphdeclares more outputs than the delegate call has arguments, that subtraction
wraps and every output access reads through a wild pointer — the failure
surfaces as a SIGSEGV inside
execute()with nothing pointing at the cause.The reachable route today is a mutated buffer serialized as a graph output.
LFM2.5's ten short-conv states declared 11 outputs against a 3-argument call.
This PR checks the counts up front so the result is a readable error naming both
numbers rather than a segfault. It does not change the mutation handling itself.
Test plan
test_vulkan_backend_index_tensor_higher_rank_selfinbackends/vulkan/test/test_vulkan_delegate.py, coveringtable[positions]with a 2-D table — the shape a rotary embedding uses. This fails before the
first change and passes after.
backends/vulkan/test/test_vulkan_delegate.pysuite.1.6B keep the rotary embedding inside the delegate, lower to a single Vulkan
call for the decoder, and generate correct text and image responses. Before
the first change the export aborted in
virtual_resize; with it, decode runsat ~150 tok/s (450M) and ~72 tok/s (1.6B) versus
100 and304 ms respectively.54 on XNNPACK, and185 ms and 565→text-only TTFT drops from 326→
cc @SS-JIA @manuelcandales @digantdesai @cbilgin