Ship the TorchAO kernels on the Linux aarch64 wheel too - #22300
Open
shoumikhin wants to merge 2 commits into
Open
Ship the TorchAO kernels on the Linux aarch64 wheel too#22300shoumikhin wants to merge 2 commits into
shoumikhin wants to merge 2 commits into
Conversation
The wheel builds these kernels for macOS arm64 and for nothing else, so a Linux aarch64 install has
no `libexecutorch_kernels_torchao` even though it has exactly the hardware they target. Verified
against the published nightly: the aarch64 row ships seven libraries and none of them is this one.
The gate was one condition. `pybind.cmake` enables the option inside the Darwin branch under
`CMAKE_SYSTEM_PROCESSOR STREQUAL "arm64"`, and Linux aarch64 reports `aarch64`, so it never
matched. Nothing about these kernels is Apple specific: they are selected by
`TORCHAO_BUILD_CPU_AARCH64` and reach their NEON paths through `TORCHAO_ENABLE_ARM_NEON_DOT`, both
of which hold on any aarch64 host. The comment claiming otherwise is corrected here, along with two
others that repeated it.
x86 is deliberately left out. In torchao's `kernel_selector.h` every `return PackedWeightsFormat` is
behind an ARM guard and there is no portable one, so both format selectors fall through to
`throw std::runtime_error("No packed_weights_format was selected")`. Enabling it there would build
and then throw on every op, which is worse than shipping nothing.
Test plan:
Built on real Linux aarch64 hardware, gcc 11.4, at the pinned TorchAO commit, with the same options
the preset now sets:
configure rc=0
libtorchao_kernels_aarch64.a built
kernel_selector.h under gcc compiles, and that is the header the ExecuTorch ops include
The prerequisites the option demands are already satisfied on this row: `EXECUTORCH_BUILD_XNNPACK`
is on for the wheel and `check_required_options_on` forces `EXECUTORCH_BUILD_CPUINFO` and
`EXECUTORCH_BUILD_PTHREADPOOL` with it.
Checked the new condition matches only what it should:
aarch64 ON arm64 ON x86_64 off amd64 off i686 off armv7l off
so 32-bit ARM stays out. The macOS path is untouched, and `setup.py` already ships the library
whenever the flag is set, so no packaging change is needed.
Two GCC portability bugs exist upstream in TorchAO's own aarch64 tree, found while building its
test suite here: `embedding_lut.h` uses `std::memcpy` without including `<cstring>`, and
`test_bitpacking.cpp` brace-initialises `int8x16_t`. Neither file is reached by
`torchao_ops_executorch`, so neither affects this build, and both need fixing in the submodule
rather than here.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22300
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit c4a82b9 with merge base 9b558d9 ( BROKEN TRUNK - The following job 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. |
lintrunner failed on formatting: the repo wraps cmake comments at the width cmakelang 0.6.13
enforces, and I wrote them wider. Reflowed the three comment blocks to match, with no change to any
condition.
Test plan:
Ran the same formatter CI uses, cmakelang 0.6.13:
tools/cmake/preset/pybind.cmake format OK
tools/cmake/executorch-wheel-config.cmake format OK
and re-parsed the preset as Linux/aarch64 to confirm the reflow left the logic alone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #22299.
Summary
The wheel builds the TorchAO kernels for macOS arm64 and for nothing else, so a Linux aarch64
install has no
libexecutorch_kernels_torchaoeven though it has exactly the hardware they target.Verified against the published nightly
1.5.0.dev20260828cp310manylinux_2_28_aarch64: that rowships seven libraries and none of them is this one.
The gate was one condition.
pybind.cmakeenables the option inside the Darwin branch underCMAKE_SYSTEM_PROCESSOR STREQUAL "arm64", and Linux aarch64 reportsaarch64, so it never matched.Nothing about these kernels is Apple specific: they are selected by
TORCHAO_BUILD_CPU_AARCH64andreach their NEON paths through
TORCHAO_ENABLE_ARM_NEON_DOT, both of which hold on any aarch64host. The in-tree comment claiming otherwise is corrected here, along with two others that repeated
it.
x86 is deliberately left out
Worth stating, because "enable it everywhere" is the tempting fix and it is wrong. In torchao's
shared_kernels/linear_8bit_act_xbit_weight/kernel_selector.h:With no NEON define both format selectors have no reachable return and fall through to
throw std::runtime_error("No packed_weights_format was selected"). Enabling it on x86 would buildand then throw on every op, which is worse than shipping nothing.
Test plan
Built on real Linux aarch64 hardware (gcc 11.4, cmake 3.22,
/proc/cpuinforeportsasimddp),at the pinned TorchAO commit
b47f1a3655, with the same options the preset now sets:torchao/csrc/cpurc=0libtorchao_kernels_aarch64.akernel_selector.hunder gcc-march=armv8.4-a+dotprodPrerequisites are already satisfied on this row:
EXECUTORCH_BUILD_XNNPACKis on for the wheel andcheck_required_options_onforcesEXECUTORCH_BUILD_CPUINFOandEXECUTORCH_BUILD_PTHREADPOOLwith it.
Checked the new condition matches only what it should:
so 32-bit ARM stays out. The macOS path is untouched, and
setup.pyalready ships the librarywhenever the flag is set, so no packaging change is needed.
One thing for reviewers to confirm
I could build and syntax-check the kernels on aarch64 Linux, but I could not produce a full aarch64
wheel and run a lowbit export end to end, since PR wheel jobs publish
x86_64only and aarch64builds on the nightly cron. The aarch64 wheel row in this PR's CI is the real test of that.
Separately, two GCC portability bugs exist upstream in TorchAO's own aarch64 tree, found while
building its test suite on this host:
embedding_lut.husesstd::memcpywithout including<cstring>, andtest_bitpacking.cppbrace-initialisesint8x16_t. Neither file is reached bytorchao_ops_executorch, so neither affects this build, but both are worth fixing in the submodule.