Repository navigation
Vulkan: register the missing logical_not runtime op - #22786
msluszniak wants to merge 2 commits into
Conversation
pytorch-bot
Bot
added
the
module: vulkan
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22786
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 1 Awaiting Approval, 2 New FailuresAs of commit 81d875d with merge base 161fbd5 ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
`aten.logical_not.default` is in the partitioner's supported set but has no `VK_REGISTER_OP` anywhere under `runtime/`, so it partitions into a delegate subgraph and the resulting `.pte` aborts at load with a missing operator error. Any model that reaches it is unrunnable rather than merely slow, which is worse than not claiming the op at all: an RF-DETR segmentation export hits it 32 times through the decoder's attention-mask logic. Both `aten.bitwise_not.default` and `aten.logical_not.default` are registered for boolean tensors only, where the two are the same operation, so the existing kernel covers it. `logical_and` and `logical_or` already reuse their bitwise counterparts in BinaryOp.cpp the same way; `logical_not` was the one left out. The boolean ops had no delegate test at all, which is how this went unnoticed. Adds one for logical_not, selecting through the mask because a boolean output cannot go through torch.allclose. (cherry picked from commit 2d09bb1)
(cherry picked from commit 84e19e9)
84e19e9 to
81d875d
Compare
|
The motivation is good, but we should be careful here not breaking any existing contract and assumptions (as this PR touches the Android native runtime contract.) I imported this internally as D119925752 and I’m seeing multiple CI failures. Digging into it more until we establish clearunderstanding whether these are default-path regressions or only split-backend-mode failures. |
executorch-triage
Bot
added
the
community: contribution
…l_not (#23246) is_bitw8 did not include kBool, so bool staging required 8-bit storage buffers even for texture tensors. It now takes the bitw8 path. VulkanBackend::init rejects graphs with non-constant bool buffers with NotSupported on devices without 8-bit storage buffers, and destroys the partially built graph on any init error. aten.logical_not was already accepted by the partitioner but had no runtime registration; it now maps to the uint8 bitwise_not kernel. Part 7/15 of the Vulkan transformer and operator-conformance stack. #23245 has landed; this is now the bottom PR, reviewed directly against main. Integration PR: #23254. Related logical_not registration: #22786. Prior native validation: 2 passed and one expected skip on MoltenVK with portable CPU kernels. Rebased onto main at `91d26b314053`, including the upstream Adreno UBO indexing fix. The code patch is unchanged. The complete shader set compiles, all 15 graph-builder/serialization tests pass, and lintrunner and git diff --check pass. Hardware and SwiftShader CI will rerun on the rebased stack. Recreates #23207 through ghstack. Prior review discussion remains on that PR. Authored with OpenAI Codex; split planned with Claude Code. cc @SS-JIA @manuelcandales @digantdesai @cbilgin
|
Hi @msluszniak — #23246 has merged and appears to supersede this implementation. It includes the same Could you double-check whether anything from this PR is still missing on Bot note: This message was drafted by an AI review bot for the reviewer. |
|
Agreed, closing as superseded by #23246. |
4 participants
aten.logical_not.defaultis in the partitioner's supported set but has noVK_REGISTER_OPanywhere underruntime/. It partitions into a delegate subgraph and the resulting.ptethen aborts at load, so a model that reaches it is unrunnable rather than merely slow. An RF-DETR segmentation export hits it 32 times through the decoder's attention-mask logic.aten.bitwise_not.defaultandaten.logical_not.defaultare both registered for boolean tensors only, where the two are the same operation, so the existing kernel covers it.logical_andandlogical_oralready reuse their bitwise counterparts inBinaryOp.cppthe same way;logical_notwas the one left out.Verified on a Mali-G76 (Galaxy S10+) with a minimal
gt.Scalar -> logical_not -> wheremodel that lowers to a single Vulkan subgraph:assert failed (VK_HAS_OP(op_name)): Missing operator: aten.logical_not.default, exit 134The boolean ops had no delegate test at all, which is how this went unnoticed. Adds one for
logical_not, selecting through the mask because a boolean output cannot go throughtorch.allclose.cc @SS-JIA @manuelcandales @digantdesai @cbilgin