-
Notifications
You must be signed in to change notification settings - Fork 233
Reduce latency cycles for _mm_movemask_epi8 on Armv7-A
#769
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: master
Are you sure you want to change the base?
Changes from all commits
dd51d88
54feef7
c14eecf
9cfd894
cf8a5e5
21ab6ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| name: Benchmark A32 | ||
|
|
||
| on: [push, pull_request, workflow_dispatch] | ||
|
|
||
| jobs: | ||
| benchmark-a32: | ||
| name: Benchmark A32 (Armv7-A) on A64 Native | ||
| if: contains(toJSON(github.event.head_commit.message), 'Merge pull request ') == false | ||
| timeout-minutes: 30 | ||
| runs-on: ubuntu-24.04-arm | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Reported latency will describe the A64 host's A32 execution path, not Armv7-A, so it cannot validate the stated Armv7-A cycle reduction. Run this performance benchmark on an Armv7-A runner/device, or label results as A64 A32-compat measurements. Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The goal of this PR is a ~9-cycle latency reduction validated by microbenchmark, but the numbers come from a shared, frequency-scaling arm64 VM where nanosecond-level latency/dependent-chain measurements are noisy and hard to reproduce. The results are indicative only and could easily hide or falsely confirm the expected gain. Consider documenting the expected variance, running more repetitions, or validating on pinned/dedicated ARMv7 hardware before treating the gain as confirmed. Prompt for AI agents |
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Download dependencies | ||
| run: | | ||
| sudo dpkg --add-architecture armhf | ||
| sudo apt-get update -q -y | ||
| sudo apt-get install -q -y libc6:armhf crossbuild-essential-armhf g++-arm-linux-gnueabihf | ||
|
|
||
| - name: Build and Run movemask benchmark natively in A32 mode | ||
| run: | | ||
| echo "=== Optimized Implementation (VPADD) ===" | ||
| make clean > /dev/null | ||
| make bench-movemask CROSS_COMPILE=arm-linux-gnueabihf- EXEC_WRAPPER= | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The steps run a cross-compiled ARMv7 armhf binary natively on the AArch64 Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The job builds an ARMv7 (armhf) binary and runs it natively via EXEC_WRAPPER= on an AArch64 runner. Native AArch32 execution only works if the runner CPU implements the AArch32 state at EL0 (e.g. Cortex-A / Ampere Altra) and the kernel has CONFIG_COMPAT; Neoverse V-class and similar cores drop AArch32, in which case the binary fails with 'Exec format error'/SIGILL and the benchmark never runs. The repo already uses qemu-arm for ARMv7 in main.yml for exactly this reason. Consider verifying the runner's AArch32 support or falling back to $(EXEC_WRAPPER) (QEMU) as insurance so the job reports numbers rather than dying. Prompt for AI agents |
||
|
|
||
| echo "=== Original Implementation (VSRA Baseline) ===" | ||
| make clean > /dev/null | ||
| make bench-movemask CROSS_COMPILE=arm-linux-gnueabihf- EXEC_WRAPPER= CXXFLAGS="-DBENCH_ORIGINAL_MOVEMASK_EPI8" | ||
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.
P2: This benchmark runs A32 instructions on an AArch64 core in compatibility mode, so the resulting cycle counts reflect Ampere Altra's ARMv7 execution rather than real ARMv7-A silicon. The latency numbers here are a sanity check and cannot validate the microarchitecture-specific latency-reduction claim in the PR description; the on-hardware ARMv7-A benchmarking mentioned there remains necessary before relying on the ~30→21 cycle figures.
Prompt for AI agents