Skip to content

ltpf: add Arm Helium (MVE) SIMD backend - #90

Merged
hchataing merged 4 commits into
google:mainfrom
JUSTiceMaMo:ltpf-mve
Oct 23, 2026
Merged

hchataing merged 4 commits into
google:mainfrom
JUSTiceMaMo:ltpf-mve

Conversation

@JUSTiceMaMo

Copy link
Copy Markdown
Contributor

Summary

Add an M-Profile Vector Extension (MVE / Arm Helium) backend for the LTPF pitch-detection hotspot, mirroring the existing NEON and Arm-DSP backends.

dot / correlate use vmlaldavaq_s16 (8-lane int16 -> int64 multiply-accumulate). The integer sum and the (v + 32) >> 6 rounding match the scalar reference exactly, so the output is bit-exact.

The header is guarded by #if (__ARM_FEATURE_MVE & 1) and is inert on non-MVE targets, so existing scalar / NEON / DSP builds are unchanged.

Measurements

On-device (Cortex-M55 @197 MHz, 16 kHz mono LC3 encode):

  • LTPF correlate: 15105 -> 8370 cyc/frame (~ -44%)
  • Encoder output bit-exact vs the scalar path (FNV-1a checksum identical)

Testing

  • make test passes locally.
  • Encoder bitstream verified bit-exact against the scalar reference.

Add an M-Profile Vector Extension (MVE) backend for the LTPF
pitch-detection hotspot, mirroring the existing NEON and Arm-DSP
backends. `dot` / `correlate` use vmlaldavaq_s16 (8-lane int16 -> int64
multiply-accumulate); the integer sum and the (v + 32) >> 6 rounding
match the scalar reference exactly, so the output is bit-exact.

The header is guarded by `#if (__ARM_FEATURE_MVE & 1)` and is inert on
non-MVE targets, so existing scalar / NEON / DSP builds are unchanged.

Measured on-device (Cortex-M55 @197 MHz, 16 kHz mono LC3 encode):
 - LTPF correlate: 15105 -> 8370 cyc/frame (about -44%)
 - encoder output bit-exact vs the scalar path (FNV-1a checksum identical)

Signed-off-by: zhangyi101 <zhangyi101@xiaomi.com>
@google-cla

google-cla Bot commented Sep 14, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Add host coverage for the LTPF Arm Helium (MVE) backend, mirroring the
existing `test/arm` and `test/neon` suites. Previously the MVE `dot` /
`correlate` paths had no automated coverage: the reference tests run the
scalar path on x86, and only ARM and NEON had backend unit tests.

`test/mve/mve.h` provides a small host emulation of the two intrinsics
used by the backend (`vldrhq_s16`, `vmlaldavaq_s16`), matching the
`test/neon/neon.h` and `test/arm/simd32.h` approach, so the backend
compiles and runs on the CI host. `test/mve/ltpf_mve.c` compiles
`ltpf.c` with `TEST_MVE` defined and checks `mve_dot` / `mve_correlate`
against the scalar reference for bit-exactness.

`ltpf_mve.h` gains a `TEST_MVE` hook (guard opt-in and the `dot` /
`correlate` overrides wrapped in `#ifndef TEST_MVE`) so both the scalar
and MVE versions are visible to the test, and so the MVE backend stays
inert while the ARM / NEON tests build.

Signed-off-by: zhangyi101 <zhangyi101@xiaomi.com>
@JUSTiceMaMo

JUSTiceMaMo commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

Friendly ping @hchataing @zxzxwu — the CI and pypi workflow runs for this PR were created but never executed: they stayed in "waiting for approval".

  • Google CLA: passed
  • GitHub Actions Scan: passed
  • CI / pypi: 0 jobs ran on the PR itself (approval expired)

To check the changes independently, I ran the same workflows on my fork at the PR head commit 1237843, CI all passed. Run: https://github.com/JUSTiceMaMo/liblc3/actions/runs/39717188196

So the build and test status is verified for this change; what remains is maintainer approval to run CI on the PR itself. Could a maintainer approve or re-run the workflows? Happy to address any review comments in the meantime. Thanks!

Comment thread src/ltpf_mve.h Outdated

#endif /* dot */

/**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to override the implementation of correlate? The default implementation in ltpf.c has the same body.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I Removed it and put the loop in the test.

Comment thread src/ltpf_mve.h
@hchataing

Copy link
Copy Markdown
Collaborator

The change is ok, just added a couple of questions.

The MVE `correlate` override duplicates the default in ltpf.c: in an MVE
build `dot` is already aliased to `mve_dot`, so the default `correlate`
body calls the MVE kernel anyway. Drop the override and keep only the
`dot` backend. Move the reference loop into test/mve/ltpf_mve.c so the
host test still checks the MVE dot against the scalar path.
@JUSTiceMaMo

Copy link
Copy Markdown
Contributor Author

@hchataing, thanks for your review .
On correlate: the override is redundant. In an MVE build dot is already aliased to mve_dot, so the default correlate body calls the MVE kernel anyway. I kept it only so the TEST_MVE host test can compare both versions. I'll remove it and put the loop in the test.
On the scalar loop, I actually checked:
gcc 13.2 (-O3 -march=armv8.1-m.main+dsp+mve.fp+fp.dp) doesn't vectorize it — emits smlalbb/smlald and reports "couldn't vectorize loop".
clang 21 does, but at width 4 (vmovlb + vmlalva.s32), not the 8-lane vmlaldavaq_s16 the intrinsic produces.
So the intrinsic guarantees the 8-lane form instead of leaving it to each compiler's vectorizer, same as the NEON/Arm backends do. Reproducer and captured output are in mve_codegen_check.sh / mve_codegen_result.txt.
mve_codegen_result.txt
mve_codegen_check.sh

@hchataing

Copy link
Copy Markdown
Collaborator

Thanks for checking.
Can you add these details as comment to the mve_dot function to justify why this is necessary ?

gcc 13.2 (-O3 -march=armv8.1-m.main+dsp+mve.fp+fp.dp) doesn't vectorize it — emits smlalbb/smlald and reports "couldn't vectorize loop". clang 21 does, but at width 4 (vmovlb + vmlalva.s32), not the 8-lane vmlaldavaq_s16 the intrinsic produces. So the intrinsic guarantees the 8-lane form instead of leaving it to each compiler's vectorizer, same as the NEON/Arm backends do

@JUSTiceMaMo

Copy link
Copy Markdown
Contributor Author

Get it. Added the rationale to the mve_dot doc comment:

The intrinsic is used instead of relying on the compiler's auto-vectorizer: at normal optimization levels for an MVE target, e.g. -O3 -march=armv8.1-m.main+dsp+mve.fp+fp.dp, gcc does not vectorize the scalar loop, while clang vectorizes it only at width 4. The intrinsic guarantees the 8-lane vmlaldavaq_s16 form, same as the NEON and Arm backends.

@hchataing
hchataing merged commit 23225f2 into google:main Oct 23, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants