[ET-VK] Name the int32 eq shader what the dispatcher asks for - #22507
[ET-VK] Name the int32 eq shader what the dispatcher asks for#22507msluszniak wants to merge 2 commits into
Conversation
add_binary_op_node builds its kernel name as "binary_" + op + storage + dtype, so an int32 aten.eq.Tensor looks for binary_eq_buffer_int32. The yaml instead declares that variant as binary_eq_int32_buffer, which generates shaders nothing ever references and leaves the name the dispatcher wants missing. The op is registered as supported, so the partitioner claims it and the model then aborts at dispatch with "Could not find ShaderInfo with name binary_eq_buffer_int32". Declare it as a second binary_eq_* variant restricted to int32 instead. The generated names do not collide with the half and float ones, and the exact comparison stays separate from the float path's epsilon compare. Found by lowering kokoro's synthesizer, whose mask comparison runs on int32.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22507
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
|
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
This fixes a generator/dispatcher naming contract, which can be tested without enabling the skipped aten.eq.Tensor correctness case. Could we add a small registry/name-generation assertion for binary_eq_{buffer,texture}_int32? Otherwise the exact missing-shader regression remains unprotected.
Guards the naming contract the previous commit fixed, without touching the skipped aten.eq.Tensor correctness case. Two assertions. The first names binary_eq_buffer_int32 and binary_eq_texture3d_int32 directly, so the exact regression is covered. The second is the general form: every variant the two binary op templates generate has to end in a storage suffix followed by a dtype suffix, because that is what add_binary_op_node builds. Both suffix sets are parsed out of ShaderNameUtils.cpp rather than restated, so adding a dtype there does not fail the test. Runs at codegen level with no built runtime and no GPU: the generator is loaded by file path and only its variant names are inspected. Reverting the yaml fix fails both assertions.
|
Added It asserts It runs at codegen level with no GPU and no built runtime: the generator is loaded by file path, the same way |
3 participants
Fixes #22505.
add_binary_op_nodebuilds its kernel name as"binary_" + op + storage + dtype, so an int32aten.eq.Tensorlooks forbinary_eq_buffer_int32. The yaml declares that variant asbinary_eq_int32_buffer, which generates shaders nothing references and leaves the name the dispatcher wants missing. The op is registered as supported, so the partitioner claims it and the model aborts at dispatch with "Could not find ShaderInfo with name binary_eq_buffer_int32".Declaring it as a second
binary_eq_*variant restricted to int32 fixes both the buffer and the texture path. The generated names do not collide with the half and float ones, and the exact comparison stays separate from the float path's epsilon compare.No test is added because
aten.eq.Tensorsits in askip=Truegroup incases.pywith a pre-existing correctness TODO. Verified on device instead: kokoro's synthesizer aborts at that shader lookup onmainand gets past it with this change on an Adreno 840.