force neon vectorization for ios (#21479) - #21479
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21479
Note: Links to docs will display an error until the docs builds have been completed. ❌ 32 New Failures, 1 Cancelled Job, 10 Unrelated FailuresAs of commit 63d8cc0 with merge base 6bbb75a ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
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. |
|
@JakeStevens has exported this pull request. If you are a Meta employee, you can view the originating Diff in D113940046. |
This PR needs a
|
Summary: The default implementation auto-vectorized well for Android builds, but the specific LLVM Clang 21 toolchain + compiler flags did not auto-vectorize, resulting in only scalar operations. This commit selectively forces neon vectorization for iOS Differential Revision: D113940046
530ec25 to
57e01dc
Compare
Summary: The default implementation auto-vectorized well for Android builds, but the specific LLVM Clang 21 toolchain + compiler flags did not auto-vectorize, resulting in only scalar operations. This commit selectively forces neon vectorization for iOS Reviewed By: YIWENX14 Differential Revision: D113940046
57e01dc to
e3b5b27
Compare
|
|
||
| ET_INLINE void vec_scale(float* data, float scale, int64_t size) { | ||
| int64_t idx = 0; | ||
| #if defined(__APPLE__) && defined(__ARM_NEON) |
There was a problem hiding this comment.
I dont super love the #ifdefs throughout the kernel code I would rather have a singular layer in the kernel process that does dispatching similar to the torchao arm neon kernels we have. We can take that as a BE followup though
There was a problem hiding this comment.
have an internal task tracking, will fix when I also rename some stuff in a diff that supports scalar gated too
| state_row[v_idx] = updated; | ||
| output_t[v_idx] += updated * query_value; | ||
| int64_t idx = 0; | ||
| #if defined(__APPLE__) && defined(__ARM_NEON) |
There was a problem hiding this comment.
why only apple? Is the auto vectorization on android just better then the handwritten intrinsics here?
There was a problem hiding this comment.
yes, at least with our internal setup. This is why we were seeing such a gap initially, Android autovectorization worked really well, Apple did not at all. This attempt at forcing it improved for Apple (going scalar -> vectored) but actually regressed for Android.
Summary: The default implementation auto-vectorized well for Android builds, but the specific LLVM Clang 21 toolchain + compiler flags did not auto-vectorize, resulting in only scalar operations. This commit selectively forces neon vectorization for iOS Reviewed By: YIWENX14, JacobSzwejbka Differential Revision: D113940046
e3b5b27 to
63d8cc0
Compare
Summary:
The default implementation auto-vectorized well for Android builds, but the specific LLVM Clang 21 toolchain + compiler flags did not auto-vectorize, resulting in only scalar operations.
This commit selectively forces neon vectorization for iOS
Reviewed By: YIWENX14, JacobSzwejbka
Differential Revision: D113940046