From 119f68d7d441d66bdf8ebd117ca0deee2119fbcc Mon Sep 17 00:00:00 2001 From: Bernard Ladenthin Date: Tue, 20 Oct 2026 13:35:06 +0200 Subject: [PATCH] ggml-cpu : select the NEON fp32->fp16 conversion (#30505) The __ARM_NEON branch defines both GGML_CPU_COMPUTE_* macros but selects only FP16_TO_FP32, so FP32_TO_FP16 falls through to the portable software conversion while neon_compute_fp32_to_fp16, a single fcvt, stays defined and referenced by nothing. __POWER9_VECTOR__ and __riscv_zfhmin select both. Add an #error pair that refuses a branch defining a COMPUTE macro without selecting it. That is how this stayed unnoticed: the fallback silently supplies a correct software implementation, so the only symptom is a hardware instruction nothing ever reaches. The FP16_TO_FP32 half carries a !defined(__F16C__) carve-out, because PR 30343 keeps that direction on the lookup table on purpose. Replace the comment above the fallback: it singled out NEON and POWER9 and pointed at a selection that now happens in the branch itself. --- ggml/src/ggml-cpu/simd-mappings.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ggml/src/ggml-cpu/simd-mappings.h b/ggml/src/ggml-cpu/simd-mappings.h index ebb54a4ab..b7b390db4 100644 --- a/ggml/src/ggml-cpu/simd-mappings.h +++ b/ggml/src/ggml-cpu/simd-mappings.h @@ -42,6 +42,7 @@ extern "C" { #define GGML_CPU_COMPUTE_FP32_TO_FP16(x) neon_compute_fp32_to_fp16(x) #define GGML_CPU_FP16_TO_FP32(x) GGML_CPU_COMPUTE_FP16_TO_FP32(x) + #define GGML_CPU_FP32_TO_FP16(x) GGML_CPU_COMPUTE_FP32_TO_FP16(x) static inline float neon_compute_fp16_to_fp32(ggml_fp16_t h) { __fp16 tmp; @@ -150,9 +151,15 @@ extern float ggml_table_f32_ue4m3[1 << 8]; #define GGML_CPU_FP32_TO_FP16(x) GGML_CPU_COMPUTE_FP32_TO_FP16(x) #endif -// On ARM NEON, it's quicker to directly convert x -> x instead of calling into ggml_lookup_fp16_to_fp32, -// so we define GGML_CPU_FP16_TO_FP32 and GGML_CPU_FP32_TO_FP16 elsewhere for NEON. -// This is also true for POWER9. +// a branch that defines a GGML_CPU_COMPUTE_* macro must also select it, or the fallback takes over silently +#if defined(GGML_CPU_COMPUTE_FP16_TO_FP32) && !defined(GGML_CPU_FP16_TO_FP32) && !defined(__F16C__) // see: https://github.com/ggml-org/llama.cpp/issues/30342 +#error "a branch defined GGML_CPU_COMPUTE_FP16_TO_FP32 without selecting it as GGML_CPU_FP16_TO_FP32" +#endif +#if defined(GGML_CPU_COMPUTE_FP32_TO_FP16) && !defined(GGML_CPU_FP32_TO_FP16) +#error "a branch defined GGML_CPU_COMPUTE_FP32_TO_FP16 without selecting it as GGML_CPU_FP32_TO_FP16" +#endif + +// fall back to the lookup table / the portable software conversion for whichever direction no branch selected #if !defined(GGML_CPU_FP16_TO_FP32) inline static float ggml_lookup_fp16_to_fp32(ggml_fp16_t f) { uint16_t s;