Enable MLAS f16<->f32 cast kernel on Apple ARM64 - #31993
Enable MLAS f16<->f32 cast kernel on Apple ARM64#31993Justin Chu (justinchuby) wants to merge 6 commits into
Conversation
The NEON f16<->f32 cast kernel (cast_kernel_neon.cpp) was excluded on all Apple targets as part of a blanket "temporary disable" of fp16 intrinsics (mlas.h lines 96-100). The exclusion prevented compilation because the intrinsics (vcvt_f32_f16/vcvt_f16_f32) require -march=armv8.2-a+fp16. This change narrows the exclusion: it enables only the cast kernel on macOS ARM64 via a new MLAS_CAST_F16_NEON_SUPPORTED macro, kept separate from MLAS_F16VEC_INTRINSICS_SUPPORTED so that the full fp16 arithmetic family stays disabled on Apple. The gating is __APPLE__ + ARM64 in both CMake and the preprocessor. All Apple Silicon (M1+) supports FEAT_FP16, making the armv8.2-a+fp16 flag safe for macOS arm64. The flag is scoped to the single source file, not applied globally. Intel Mac and universal2 are out of scope; iOS is not implied. Tests: - Extended test_cast_fp16.cpp with special-value coverage (±0, ±Inf, quiet and signalling NaN, denormals, round-to-nearest-even) and non-vector-width-aligned lengths. - Added a compile-time reachability check that the kernel dispatch is active on platforms that define the new macro. - Tests were NOT run on this host (Linux x86-64); validation depends on the macOS arm64 CI leg only. Performance is unmeasured. No speedup claim is made. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
S1: TestKernelIsDispatched now directly asserts that the MLAS_PLATFORM dispatch pointers (CastF16ToF32Kernel, CastF32ToF16Kernel) are non-null when MLAS_F16VEC_INTRINSICS_SUPPORTED or MLAS_CAST_F16_NEON_SUPPORTED is defined, and null otherwise. This is the exact condition cast.cpp checks at runtime, so a non-null assertion genuinely proves the NEON kernel is wired in — unlike the previous test which converted 1.0 and would pass identically on the scalar fallback. The two paths (NEON and scalar) are designed to be bit-exact, so no runtime value pattern can distinguish them. The dispatch-pointer check is the only honest assertion. S2: Add signalling NaN (0x7C01) alongside quiet NaN (0x7E00) in TestSpecialValues to catch payload-quieting divergence between NEON vcvt and the scalar reference. Add mid-range and negative denormals (0x0200, 0x8001) to surface flush-to-zero differences if they exist. Add signaling_NaN to the f32->f16 direction as well. Include mlasi.h (already used by other MLAS unit tests) to access GetMlasPlatform(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes the clangformat lint check. onnxruntime/core/mlas/** is excluded from clang-format in .lintrunner.toml, but onnxruntime/test/mlas/** is not, so the new unit test must be formatted. Verified with 'diff -w' that the change is whitespace-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the #else branch in TestKernelIsDispatched() that asserted null dispatch pointers on non-ARM64 Apple (x86_64). Intel Mac and universal2 are out of scope; iOS is not implied. The positive non-null assertion under MLAS_CAST_F16_NEON_SUPPORTED remains — it proves the NEON kernel is actually reached on macOS arm64 rather than the scalar fallback. The compile-time gate (__APPLE__ && MLAS_TARGET_ARM64) and the portable non-Apple scalar fallback are unchanged. Validation: macOS arm64 CI leg only. This host is Linux x86-64 (AMD EPYC) with no ARM cross-compiler; tests were NOT run here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6665165 to
68ee0de
Compare
- NaN assertions: The MLAS scalar reference (MLAS_Float2Half) canonicalizes
every NaN to 0x7E00, discarding the payload. Hardware FCVTN preserves/
truncates the source payload. For FCVTL (f16->f32), hardware quiets
signalling NaNs. Because payload semantics differ between the scalar
reference and hardware, the test now asserts only:
1. The result is a NaN (exponent all-ones, mantissa non-zero).
2. The sign bit matches.
Payload bits are deliberately unasserted. This is still non-vacuous:
it catches any kernel bug that would turn a NaN into Inf (mantissa=0)
or a finite value, or flip the sign.
- Verified under emulated AArch64 (QEMU 8.2, aarch64-linux-gnu-g++ 13.2):
sNaN f32 0x7FA00000 -> NEON 0x7F00, scalar 0x7E00 (payload differs).
qNaN f32 0x7FC12300 -> NEON 0x7E09, scalar 0x7E00 (payload differs).
The old 'payload modulo quiet bit' assertion would have failed on both.
- FEAT_FP16 clarification: the kernel uses vcvt_f32_f16 / vcvt_f16_f32,
which are baseline ARMv8-A conversion instructions (FCVTL/FCVTN).
FEAT_FP16 (ARMv8.2-A +fp16) enables fp16 *arithmetic* on float16x8_t
and is NOT required for this kernel. The -march=armv8.2-a+fp16 flag
was already removed; this commit corrects the narrative to match.
- Runtime evidence: all cast tests (special values, bulk at 17 different
lengths including non-multiples of vector width 4/8, ±0, ±Inf, NaN,
denormals, RNE tie 1+2^-11) pass under qemu-aarch64-static on Linux
x86_64. This is QEMU emulation of AArch64 NEON, not native Apple
Silicon — QEMU faithfully implements FCVTN/FCVTL IEEE semantics.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
02a9f34 to
5ba2500
Compare
The conversion intrinsics (vcvt_f32_f16 / vcvt_f16_f32) are baseline ARMv8-A; FEAT_FP16 governs fp16 arithmetic which this kernel does not use. Correct the comment to avoid implying a dependency on FEAT_FP16. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Enables NEON FP16↔FP32 conversion kernels on macOS ARM64 while keeping other FP16 arithmetic disabled.
Changes:
- Adds a macOS ARM64-specific capability macro and CMake source inclusion.
- Enables NEON cast-kernel dispatch.
- Expands conversion, special-value, and dispatch tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cmake/onnxruntime_mlas.cmake |
Includes the NEON cast kernel for Darwin ARM64 builds. |
onnxruntime/core/mlas/inc/mlas.h |
Defines macOS ARM64 cast support. |
onnxruntime/core/mlas/lib/cast_kernel_neon.cpp |
Updates kernel support documentation. |
onnxruntime/core/mlas/lib/mlasi.h |
Exposes NEON cast declarations under the new capability. |
onnxruntime/core/mlas/lib/platform.cpp |
Enables runtime dispatch under the new capability. |
onnxruntime/test/mlas/unittest/test_cast_fp16.cpp |
Expands conversion and dispatch tests. |
Suppressed comments (1)
onnxruntime/test/mlas/unittest/test_cast_fp16.cpp:146
- This F32→F16 case set has only positive NaNs and no inputs whose half result is denormal. Therefore it does not exercise the NaN sign contract or the advertised denormal coverage in this conversion direction. Add negative NaNs and exact positive/negative half-denormal equivalents.
std::vector<float> f32_input = {
0.0f, -0.0f,
std::numeric_limits<float>::infinity(),
-std::numeric_limits<float>::infinity(),
std::numeric_limits<float>::quiet_NaN(),
std::numeric_limits<float>::signaling_NaN(),
1.00048828125f};
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const uint16_t kQNaN = 0x7E00; // quiet NaN | ||
| const uint16_t kSNaN = 0x7C01; // signalling NaN (payload 0x001) | ||
| const uint16_t kDenormMin = 0x0001; // smallest positive denormal | ||
| const uint16_t kDenormMid = 0x0200; // mid-range positive denormal | ||
| const uint16_t kNegDenorm = 0x8001; // smallest negative denormal | ||
|
|
||
| std::vector<uint16_t> special_bits = { | ||
| kPosZero, kNegZero, kPosInf, kNegInf, kQNaN, kSNaN, | ||
| kDenormMin, kDenormMid, kNegDenorm}; |
| size_t cnt = 0; | ||
| for (size_t n : {1, 7, 15, 16, 31, 32, 63, 64, 128, 255, 256, 1024, 65536}) { | ||
| // Various lengths including non-multiples of vector width (4/8) | ||
| for (size_t n : {1, 2, 3, 5, 7, 9, 15, 16, 17, 31, 32, 63, 64, 128, 255, 256, 1024, 65536}) { |
| if (CMAKE_SYSTEM_NAME STREQUAL "Darwin") | ||
| list(APPEND mlas_platform_srcs ${MLAS_SRC_DIR}/cast_kernel_neon.cpp) |
|
Review — PR #31993: Enable MLAS f16↔f32 cast kernel on Apple macOS arm64 Summary of changes (6 files, targeted +100/−~10)
The technical claim is correct — I verified it
Scope isolation is well-done I traced every other consumer of
Only the two cast-dispatch slots ( NaN test contract is right I initially raised an eyebrow at the asymmetric NaN assertion ( Universal2 clarification worth making The PR description says "No universal2", but I traced the CMake and that's misleading — the description means "we don't add any x86_64-Apple dispatch", not "universal2 arm64 slices don't get the kernel". Concretely:
That's actually the correct behavior (arm64 slice gets NEON, x86_64 slice stays scalar). The PR description should be tweaked to say "no new x86_64-Apple dispatch; universal2 arm64 slice inherits the macOS-arm64 behavior naturally" so the Copilot review comment (medium severity) doesn't get read as "code is wrong". Either fix the description or add a one-line CMake comment explaining the interaction with Comments / suggestions
Non-issues I re-verified
Recommendation Approve after (1) the This is a clean, well-scoped, correctly-reasoned enabling PR — the author has already done the hard part of separating conversion (baseline) from arithmetic (FEAT_FP16), and the test contract for NaN handling is exactly right for hardware-vs-software round-tripping. |
What this enables
The NEON f16↔f32 conversion kernel (
cast_kernel_neon.cpp) is excluded on all Apple targets today —mlas.hgatesMLAS_F16VEC_INTRINSICS_SUPPORTEDbehind#if !defined(__APPLE__), andcmake/onnxruntime_mlas.cmakeexcludes the source withif (NOT APPLE). The comment marks this "temporary"; it was never revisited. macOS ARM64 therefore falls back to a scalarSource[i].ToFloat()loop.This enables only that cast kernel, on macOS arm64 only, via a new
MLAS_CAST_F16_NEON_SUPPORTEDmacro kept separate fromMLAS_F16VEC_INTRINSICS_SUPPORTED, so fp16 arithmetic, i8mm and bf16 stay disabled on Apple.Scope
Gated on
TARGET_OS_OSXplusMLAS_TARGET_ARM64in C++, andCMAKE_SYSTEM_NAME STREQUAL "Darwin"plus arm64 in CMake — deliberately not CMake'sAPPLE, which is also true for iOS, tvOS and visionOS. No universal2, no x86_64 Apple slices, no iOS/tvOS/visionOS.No
-marchflag is neededvcvt_f32_f16/vcvt_f16_f32are baseline AArch64 conversion instructions (FCVTL/FCVTN) —arm_neon.hguards them under#if (__ARM_FP & 2), not__ARM_FEATURE_FP16_VECTOR_ARITHMETIC. FEAT_FP16 governs fp16 arithmetic, which this kernel does not perform. An earlier revision added-march=armv8.2-a+fp16; it was unnecessary and has been removed.NaN contract
For NaN results the tests assert NaN-ness (exponent all ones, mantissa non-zero) and the sign bit — the payload is deliberately not asserted. Raw-bit equality is retained for every non-NaN value.
That asymmetry is required, not a convenience: hardware
FCVTNpreserves the NaN payload, while the software reference canonicalises every NaN to0x7E00. Measured by cross-compiling and executing real AArch64 NEON:FCVTN0x7FA000000x7F000x7E000x7FC123000x7E090x7E00A bit-exact NaN assertion would fail on any real AArch64 target.
Test coverage
CastFp16ShortExecuteTest(lengths 1–259)CastFp16SpecialValuesTest1 + 2^-11CastFp16KernelDispatchTestGetMlasPlatform().CastF16ToF32Kernelis non-null — i.e. the NEON kernel is dispatched rather than the scalar fallbackWhere these have and have not run
Emulated AArch64. I cross-compiled the kernel with
aarch64-linux-gnu-g++ -O2 -staticand ran it underqemu-aarch64-static: all special values and all 17 bulk lengths pass. This validates the NEON instruction semantics — which is what the NaN behaviour above turns on — but it is emulation on a Linux host, not Apple Silicon, and I am not presenting it as hardware validation.Native Apple Silicon. The
cpu / build-and-test (arm64, arm64, …)lanes inmac.ymlrun withmachine == target, which satisfies the workflow's test gate, andbuild.py --testinvokesonnxruntime_mlas_testdirectly on non-Windows hosts. Those jobs pass on this PR, including the "Running Tests (build.py --test)" step.To be precise about the limits of that: I have confirmed the test step executes and succeeds on native arm64 runners, and that the binary it runs contains these tests. I have not been able to retrieve per-test output from the job log to show
CastFp16KernelDispatchTestby name. If a maintainer would like that made explicit, a targeted test registration or a filtered invocation could be added — I did not want to change what the MLAS suite runs across all platforms just to surface one line.No performance claims
Nothing was measured. This replaces a scalar conversion loop with a vectorised one on macOS arm64; whether and where that matters is not established here.
CI status
All checks are green: 86 passing, 1 skipped (CodeQL, expected for this diff). Two transient infra issues surfaced during review and are documented here for the record — neither was caused by this change:
4. Build Minimal (Globally Allowed Types)(Linux CPU Minimal Build E2E) satqueuedfor ~20.5 hours while its 9 sibling jobs in the same run completed normally within minutes — an orphaned runner-scheduling issue. Cancelling and re-triggering (rerun-failed-jobs) picked up a healthy runner slot; the job then passed immediately.cancelled, not a real test failure: the Detox/iOS-simulator step hung after boot and sat idle until the job'stimeout-minutes: 90fired (react_native.yml). This diff touches only 6 MLAS C++/CMake files and nothing underjs/react_native; 2 of the last 5main-branch runs of this same workflow show the identical hang, confirming a pre-existing, recurring flake. Rerunning passed cleanly.No code changes were needed for either issue.