ggml icon indicating copy to clipboard operation
ggml copied to clipboard

ARM compilation error for SME

Open peardox opened this issue 5 months ago • 9 comments

I've tried this on a Ampere Altra and Pi5

cmake -B build -DGGML_CPU_ARM_ARCH=OFF -DGGML_NATIVE=OFF -DGGML_CPU_ALL_VARIANTS=ON -DGGML_BACKEND_DL=ON -DBUILD_SHARED_LIBS=ON
cmake --build build

Results in,,,

cc1: error: unknown value ‘armv9.2-a+dotprod+fp16+sve+i8mm+sve2+sme’ for ‘-march’ cc1: note: valid arguments are: armv8-a armv8.1-a armv8.2-a armv8.3-a armv8.4-a armv8.5-a armv8.6-a armv8.7-a armv8.8-a armv8-r armv9-a native gmake[2]: *** [ggml/src/CMakeFiles/ggml-cpu-armv9.2_2.dir/build.make:76: ggml/src/CMakeFiles/ggml-cpu-armv9.2_2.dir/ggml-cpu/ggml-cpu.c.o] Error 1 gmake[1]: *** [CMakeFiles/Makefile2:1517: ggml/src/CMakeFiles/ggml-cpu-armv9.2_2.dir/all] Error 2

Suggest a new switch is added to [ggml/]src/CMakeLists.txt ...

option(GGML_SKIP_ARM9 "Skip ARM 9 compilation" OFF)

This would then be triggered later on as...

if(NOT GGML_SKIP_ARM9)
    ggml_add_cpu_backend_variant(armv9.2_1    DOTPROD FP16_VECTOR_ARITHMETIC SVE MATMUL_INT8 SME)
    ggml_add_cpu_backend_variant(armv9.2_2    DOTPROD FP16_VECTOR_ARITHMETIC SVE MATMUL_INT8 SVE2 SME)
endif()

I'll write a PR if nobody else does...

peardox avatar Jul 10 '25 19:07 peardox

If the compiler does not support some variants I think you should not use -DGGML_CPU_ALL_VARIANTS=ON in the first place.

ggerganov avatar Jul 11 '25 07:07 ggerganov

By extension no compiler should use GGML_CPU_ALL_VARIANTS as none currently support any as-yet unreleased opcodes that will be introduced over the coming years (and eventually end up in the compiler)

Also, by that argument, MSVC should not be used as it does not support AMX

if (NOT MSVC) # MSVC doesn't support AMX ggml_add_cpu_backend_variant(sapphirerapids SSE42 AVX F16C AVX2 BMI2 FMA AVX512 AVX512_VBMI AVX512_VNNI AVX512_BF16 AMX_TILE AMX_INT8) endif()

The above is in the same section of CMakeList.txt - a bit above the suggested option which has no effect whatsoever unless deliberately and specifically requested

By it's very nature ARM is a base CPU with many optional extras which not all compilers will support

peardox avatar Jul 11 '25 07:07 peardox

GCC < 14 on ARM does not support SME extensions

peardox avatar Jul 11 '25 09:07 peardox

GGML_CPU_ALL_VARIANTS can be made to work around this, in llama #14080 @chaxu01 actually proposed limiting the variants built to only those supported by the compiler.

@chaxu01, want to take a look, or should I?

ckastner avatar Jul 11 '25 21:07 ckastner

That looks a lot better than mine - I guess it should be applied to ggml so that both whisper and llama benefit from it.

There appear to be two options here

  1. Only allow compliers with full CPU supports
  2. Allow any compiler limited by the CPUs it supports

There should probably be a list of compilers that support (1) and (2) could output it? This would at least let the dev know that better options were available.

peardox avatar Jul 12 '25 05:07 peardox

Only allow compliers with full CPU supports

Sure, that should also be considered.

Adding the ability to build only variants that the compiler supports wouldn't be that hard, but it would still add some code to maintain.

Oh -- but checking if the current compiler is new enough also adds some code.

ckastner avatar Jul 12 '25 07:07 ckastner

@chaxu01, want to take a look, or should I?

I'm currently working on another PR, but I can take a look at it later — unless you'd prefer to handle it now.

chaxu01 avatar Jul 14 '25 06:07 chaxu01

I may have been over-eager, both maintainers already expressed leaning towards 1. (use the right compiler). I overlooked that, sorry

ckastner avatar Jul 15 '25 06:07 ckastner

So we might have a use case after all: @mbaudier reported that GCC in Debian 12 (bookworm) also runs into this error. That means when we ship a package for bookworm-backports, we will run into this problem.

Now the trivial solution for us is patch out the SME variants in our bookworm-backports build. But @mbaudier pointed out that the all-variants feature is about portability so implementing the feature check would technically be slightly cleaner and I tend to agree. So I thought I could ask upstream again if this is something they might consider after all.

The tradeoff is more complexity in the upstream CMake config. Not much, but still to consider if a simple workaround exists downstream (just patch it out locally).

For context, Debian 13 (trixie) has been released but only days ago. So 12 will still be a major use platform for maybe a year, hence the intention to ship a backport. I would expect similar usage for Ubuntu 24.04 LTS.

ckastner avatar Aug 18 '25 14:08 ckastner