Skip to content

[MLX] Add aten.bitwise_or op handler#19869

Open
devin-lai wants to merge 2 commits into
pytorch:mainfrom
devin-lai:mlx-add-bitwise-or
Open

[MLX] Add aten.bitwise_or op handler#19869
devin-lai wants to merge 2 commits into
pytorch:mainfrom
devin-lai:mlx-add-bitwise-or

Conversation

@devin-lai
Copy link
Copy Markdown

@devin-lai devin-lai commented May 29, 2026

Summary:

  • Add an MLX schema node and op handler for aten.bitwise_or Tensor and Scalar overloads.
  • Execute the node through MLX bitwise_or in the runtime interpreter.
  • Add bool, integer, and scalar op test coverage.

Fixes #18926.

Testing:

  • cmake --build cmake-out-mlx --target op_test_runner -j2
  • PATH="$PWD/.venv-mlx/bin:$PATH" .venv-mlx/bin/python -m executorch.backends.mlx.test.run_all_tests bitwise_or_bool bitwise_or_int bitwise_or_scalar --timeout 180
  • cmake --build cmake-out-mlx --target strict_compile_test -j2
  • git diff --check
  • py_compile on changed Python files
  • lintrunner --take FLAKE8,CLANGFORMAT,NEWLINE,ETCAPITAL,LICENSELINT on touched files

cc @metascroy

Add MLX delegate support for aten.bitwise_or Tensor and Scalar overloads, following the existing bitwise_and path through the FlatBuffer schema, Python op handler, C++ interpreter, and tests.

Fixes pytorch#18926.
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented May 29, 2026

🔗 Helpful Links

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

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

❌ 1 Unclassified Failure

As of commit af4d4bf with merge base 40b0a35 (image):

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

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 May 29, 2026
@devin-lai
Copy link
Copy Markdown
Author

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example @pytorchbot label "release notes: none"

For more information, see https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@pytorchbot label "release notes: apple"

@pytorch-bot pytorch-bot Bot added the release notes: apple Changes to the Apple backend delegate label May 29, 2026
@nil-is-all nil-is-all added the module: mlx Issues related to MLX Backend: Metal-accelerated inference on Apple Silicon label Jun 1, 2026
@nil-is-all nil-is-all requested a review from metascroy June 1, 2026 17:32
@nil-is-all
Copy link
Copy Markdown
Contributor

@claude review

@claude
Copy link
Copy Markdown

claude Bot commented Jun 1, 2026

Claude finished @nil-is-all's task in 2m 43s —— View job


Code Review: [MLX] Add aten.bitwise_or op handler

  • Read changed files and gather context
  • Review backends/mlx/ops.py changes
  • Review backends/mlx/runtime/MLXInterpreter.h changes
  • Review backends/mlx/serialization/schema.fbs changes
  • Review backends/mlx/test/test_ops.py changes
  • Post review feedback

Verdict: Looks good. This is a clean, well-structured addition that follows the exact same pattern as the existing bitwise_and op. No issues found.

Details

Schema (schema.fbs):

  • BitwiseOrNode table correctly mirrors the BitwiseAndNode structure with a, b, and out fields
  • Appended to the OpNode union at the end (respecting backward compatibility rules)

Runtime (MLXInterpreter.h):

  • exec_bitwise_or follows the identical pattern as exec_bitwise_and — delegates directly to MLX's bitwise_or
  • Dispatch case OpCode::BITWISE_OR correctly wired up in the switch statement

Python ops (ops.py):

  • Registers both Tensor and Scalar overloads via the _BINARY_OPS table with has_scalar=True
  • Correctly imports BitwiseOrNode from the generated schema

Tests (test_ops.py):

  • bitwise_or_bool and bitwise_or_int cover Tensor-Tensor via the _BINARY_OP_TESTS table
  • BitwiseOrScalarTest covers the Scalar overload with bool, int32, and int64 dtypes
  • Test structure is identical to the existing BitwiseAndScalarTest

No correctness, performance, or compatibility concerns. The implementation correctly leverages MLX's native bitwise_or which handles broadcasting, and the scalar variant is handled by the framework's existing _BINARY_OPS infrastructure that wraps scalars into tensors before calling the op.


@metascroy
Copy link
Copy Markdown
Contributor

LGTM...let's see what CI says

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Jun 1, 2026

Workflows were awaiting approval. CI has now been triggered for the ciflow labels on this PR.

@metascroy
Copy link
Copy Markdown
Contributor

@devin-lai it looks like [MLX / backend-tester (models) / test-mlx-backend-models (push)] is failing. Can you have a look at why?

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

Labels

ciflow/mlx CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: mlx Issues related to MLX Backend: Metal-accelerated inference on Apple Silicon release notes: apple Changes to the Apple backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Good First Issue: Add MLX Op Handler for aten.bitwise_or

3 participants