-
Notifications
You must be signed in to change notification settings - Fork 736
WIP: Add bitwise left/right #15893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
WIP: Add bitwise left/right #15893
Conversation
This PR needs a
|
|
@mergennachin has imported this pull request. If you are a Meta employee, you can view this in D87451202. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for bitwise left shift (<<) and right shift (>>) operations to the ExecutorTorch portable kernel library, following the existing pattern for bitwise operations (and, or, xor).
Key Changes:
- Implementation of
bitwise_left_shiftandbitwise_right_shiftoperators with both Tensor and Scalar variants - Comprehensive test coverage for both operations including edge cases (zero shift, different data types)
- Build system integration across Buck (bzl), CMake, and YAML configuration files
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
shim_et/xplat/executorch/kernels/portable/op_registration_util.bzl |
Adds Buck build targets for the new shift operators (ordering needs correction) |
kernels/test/targets.bzl |
Registers test targets for both shift operations (correctly ordered) |
kernels/test/CMakeLists.txt |
Adds test source files to CMake build (correctly ordered) |
kernels/portable/functions.yaml |
Defines kernel function signatures (should use - op: instead of - func:) |
kernels/portable/cpu/pattern/bitwise_op.h |
Adds shift operation functors and templates (contains redundant code) |
kernels/portable/cpu/op_bitwise_left_shift.cpp |
Implements left shift operation using the bitwise pattern |
kernels/portable/cpu/op_bitwise_right_shift.cpp |
Implements right shift operation using the bitwise pattern |
kernels/test/op_bitwise_left_shift_test.cpp |
Provides comprehensive test coverage for left shift |
kernels/test/op_bitwise_right_shift_test.cpp |
Provides comprehensive test coverage for right shift |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| op_target( | ||
| name = "op_bitwise_right_shift", | ||
| deps = [ | ||
| ":scalar_utils", | ||
| "//executorch/kernels/portable/cpu/pattern:bitwise_op", | ||
| "//executorch/kernels/portable/cpu/util:broadcast_util", | ||
| "//executorch/kernels/portable/cpu/util:dtype_util", | ||
| "//executorch/kernels/portable/cpu/util:elementwise_util", | ||
| ], | ||
| ), |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The op_bitwise_right_shift target is placed out of alphabetical order. It should be positioned between op_bitwise_or (line 357) and op_bitwise_xor (line 367) to maintain alphabetical ordering of the op targets.
| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | ||
|
|
||
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | ||
|
|
||
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | ||
|
|
||
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shift operations should use - op: instead of - func: to be consistent with the other bitwise operations (and, or, xor, not) which are ATen library operations. The - func: syntax is typically reserved for custom operations not in the ATen library.
Change to:
- op: bitwise_left_shift.Tensor_out
kernels:
- arg_meta: null
kernel_name: torch::executor::bitwise_left_shift_Tensor_out| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| - op: bitwise_left_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - op: bitwise_left_shift.Scalar_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - op: bitwise_right_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - op: bitwise_right_shift.Scalar_out |
| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | ||
|
|
||
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | ||
|
|
||
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | ||
|
|
||
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shift operations should use - op: instead of - func: to be consistent with the other bitwise operations (and, or, xor, not) which are ATen library operations. The - func: syntax is typically reserved for custom operations not in the ATen library.
Change to:
- op: bitwise_right_shift.Tensor_out
kernels:
- arg_meta: null
kernel_name: torch::executor::bitwise_right_shift_Tensor_out| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| - op: bitwise_left_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - op: bitwise_left_shift.Scalar_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - op: bitwise_right_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - op: bitwise_right_shift.Scalar_out |
| op_target( | ||
| name = "op_bitwise_left_shift", | ||
| deps = [ | ||
| ":scalar_utils", | ||
| "//executorch/kernels/portable/cpu/pattern:bitwise_op", | ||
| "//executorch/kernels/portable/cpu/util:broadcast_util", | ||
| "//executorch/kernels/portable/cpu/util:dtype_util", | ||
| "//executorch/kernels/portable/cpu/util:elementwise_util", | ||
| ], | ||
| ), |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The op_bitwise_left_shift target is placed out of alphabetical order. It should be positioned between op_bitwise_and (line 339) and op_bitwise_not (line 349) to maintain alphabetical ordering of the op targets.
| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | ||
|
|
||
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | ||
|
|
||
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | ||
|
|
||
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shift operations should use - op: instead of - func: to be consistent with the other bitwise operations (and, or, xor, not) which are ATen library operations. The - func: syntax is typically reserved for custom operations not in the ATen library.
Change to:
- op: bitwise_left_shift.Scalar_out
kernels:
- arg_meta: null
kernel_name: torch::executor::bitwise_left_shift_Scalar_out| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| - op: bitwise_left_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - op: bitwise_left_shift.Scalar_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - op: bitwise_right_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - op: bitwise_right_shift.Scalar_out |
| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | ||
|
|
||
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | ||
|
|
||
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | ||
| kernels: | ||
| - arg_meta: null | ||
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | ||
|
|
||
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shift operations should use - op: instead of - func: to be consistent with the other bitwise operations (and, or, xor, not) which are ATen library operations. The - func: syntax is typically reserved for custom operations not in the ATen library.
Change to:
- op: bitwise_right_shift.Scalar_out
kernels:
- arg_meta: null
kernel_name: torch::executor::bitwise_right_shift_Scalar_out| - func: bitwise_left_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - func: bitwise_left_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - func: bitwise_right_shift.Tensor_out(Tensor self, Tensor other, *, Tensor(a!) out) -> Tensor(a!) | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - func: bitwise_right_shift.Scalar_out(Tensor self, Scalar other, *, Tensor(a!) out) -> Tensor(a!) | |
| - op: bitwise_left_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Tensor_out | |
| - op: bitwise_left_shift.Scalar_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_left_shift_Scalar_out | |
| - op: bitwise_right_shift.Tensor_out | |
| kernels: | |
| - arg_meta: null | |
| kernel_name: torch::executor::bitwise_right_shift_Tensor_out | |
| - op: bitwise_right_shift.Scalar_out |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15893
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 2 Unrelated FailuresAs of commit 6861b0a with merge base 6e4e8fa ( NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.