MLX delegate: add integer support for aten.bitwise_not#19053
MLX delegate: add integer support for aten.bitwise_not#19053metascroy merged 5 commits intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19053
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: ❌ 14 New Failures, 15 Pending, 5 Unrelated FailuresAs of commit bbe7c56 with merge base 56da964 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label "release notes: apple" |
There was a problem hiding this comment.
Pull request overview
Extends the MLX delegate’s lowering for aten.bitwise_not to support integer tensors by introducing a new BitwiseInvertNode backed by MLX bitwise_invert, while keeping the existing boolean lowering via LogicalNotNode.
Changes:
- Add
BitwiseInvertNodeto the FlatBuffers schema andOpNodeunion. - Add MLX runtime execution support (
exec_bitwise_invert) and interpreter dispatch for the new opcode. - Update the Python op handler to emit
BitwiseInvertNodefor non-bool inputs and add a basic integer test case.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backends/mlx/serialization/schema.fbs | Adds BitwiseInvertNode table and appends it to the OpNode union for serialization. |
| backends/mlx/runtime/MLXInterpreter.h | Implements exec_bitwise_invert() and adds interpreter dispatch for OpCode::BITWISE_INVERT. |
| backends/mlx/ops.py | Updates aten.bitwise_not handler to emit BitwiseInvertNode for non-bool inputs. |
| backends/mlx/test/test_ops.py | Adds a unary op test for integer torch.bitwise_not. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {"op_name": "abs", "op_fn": torch.abs}, | ||
| {"op_name": "neg", "op_fn": torch.neg}, | ||
| {"op_name": "logical_not","op_fn": torch.logical_not, "shapes": [(2, 3, 4), (10,), (4, 8)], "dtypes": [torch.bool], "input_fn": _bool_input_fn()}, | ||
| {"op_name": "bitwise_not_int", "op_fn": torch.bitwise_not, "shapes": _SHAPES_3, "dtypes": [torch.int32, torch.int64], "input_fn": _int_input_fn()}, |
There was a problem hiding this comment.
I consciously decided to test only what was asked to be added, lmk if you need also the bool test case
There was a problem hiding this comment.
Test whatever dtypes you assert as supported in the handler.
Also, can you paste the outcome of successful test in the PR description?
There was a problem hiding this comment.
is the test summary in the description of the PR any good?
|
@claude review this PR |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Overall looks great @AlessandroVacca! Just a few feedbacks, and then we can merge :) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Removed support for int8, int16, and uint8 data types.
|
Should be good now! @metascroy |
There was a problem hiding this comment.
Pull request overview
Extends the MLX delegate’s lowering for aten.bitwise_not beyond bool tensors by introducing a dedicated BitwiseInvertNode (backed by MLX bitwise_invert) and dispatching to it for supported integer dtypes.
Changes:
- Added
BitwiseInvertNodeto the MLX FlatBuffers schema and appended it to theOpNodeunion. - Added MLX runtime execution support (
exec_bitwise_invert) and interpreter dispatch for the new opcode. - Updated the Python lowering handler to dispatch:
bool -> LogicalNotNode, supported integers ->BitwiseInvertNode, otherwise fallback. - Added a unary op test entry for integer
torch.bitwise_not(currently int32/int64).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| backends/mlx/serialization/schema.fbs | Defines BitwiseInvertNode and appends it to the OpNode union for serialization/runtime compatibility. |
| backends/mlx/runtime/MLXInterpreter.h | Implements and dispatches execution for the new bitwise invert op in the MLX interpreter. |
| backends/mlx/ops.py | Updates aten.bitwise_not lowering to route bool to logical-not and integers to bitwise-invert. |
| backends/mlx/test/test_ops.py | Adds a new unary op test case for integer bitwise_not. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {"op_name": "abs", "op_fn": torch.abs}, | ||
| {"op_name": "neg", "op_fn": torch.neg}, | ||
| {"op_name": "logical_not","op_fn": torch.logical_not, "shapes": [(2, 3, 4), (10,), (4, 8)], "dtypes": [torch.bool], "input_fn": _bool_input_fn()}, | ||
| {"op_name": "bitwise_not_int", "op_fn": torch.bitwise_not, "shapes": _SHAPES_3, "dtypes": [torch.int32, torch.int64], "input_fn": _int_input_fn()}, |
There was a problem hiding this comment.
Not advertised anymore whatsoever
Summary
Fixes #18924
Extends
aten.bitwise_notsupport in the MLX delegate to handle integer tensors, not just boolean tensors.Previously the handler only dispatched to
LogicalNotNodeforbooland raisedNotImplementedErrorfor all other dtypes. This adds a dedicatedBitwiseInvertNodebacked bymlx::core::bitwise_invert, and updates the handler to dispatch based on dtype:bool→LogicalNotNode(unchanged)int32,int64→BitwiseInvertNodeChanges:
serialization/schema.fbs: addBitwiseInvertNodetable and append toOpNodeunionruntime/MLXInterpreter.h: addexec_bitwise_invert()and dispatch caseops.py: update_bitwise_not_handlerto dispatch toBitwiseInvertNodefor integerstest/test_ops.py: addbitwise_not_inttest forint32andint64Test plan
All tests were ran on a machine with an Apple M1 Pro CPU, macOS 26.4.1.
python3 -m py_compile backends/mlx/ops.py backends/mlx/test/test_ops.pypython3 backends/mlx/serialization/generate.pypython3 -m executorch.backends.mlx.test.run_all_tests bitwise_not_intTest output
cc @metascroy