ggml-cpu : split arch-specific implementations #13892

xctan · 2025-05-29T13:38:45Z

This PR reworks the arch-specific code organization, following the discussion in #13720. Key changes include splitting the former ggml-cpu-quants.c and ggml-cpu-aarch64.cpp into several more focused files. Additionally, aarch64-related naming and identifiers, originally specific to AArch64 but now applicable to other architectures, have been updated to use repack to improve clarity and avoid confusion.

xctan · 2025-05-29T13:51:12Z

I'll need a bit more time to get this PR rebased onto the latest master branch.

xctan · 2025-05-31T06:29:59Z

@ggerganov @slaren
Now that all CI checks have passed, could you please review this PR?

ggerganov · 2025-06-04T12:29:19Z

ggml/src/ggml-cpu/ggml-cpu-impl.h

+
+#define GGML_DO_PRAGMA_(x) _Pragma (#x)
+#define GGML_DO_PRAGMA(x) GGML_DO_PRAGMA_(x)
+#if defined(GGML_CPU_GENERIC) || defined(__HIPCC__)
+// weak alias not working
+# define GGML_WEAK_ALIAS(name, alias)
+#elif defined(__GNUC__)
+// GCC/Clang on *nix
+# define GGML_WEAK_ALIAS(name, alias) GGML_DO_PRAGMA(weak name = alias) // NOLINT
+#elif defined(_MSC_VER) && defined (_WIN64)
+// MSVC
+// Note: C name mangling varies across different calling conventions
+// see https://learn.microsoft.com/en-us/cpp/build/reference/decorated-names?view=msvc-170
+# define GGML_WEAK_ALIAS(name, alias) GGML_DO_PRAGMA(comment(linker, "/alternatename:" #name "=" #alias))
+#else
+# error "Unsupported compiler for GGML_WEAK_ALIAS"
+#endif
+
+#define GGML_CPU_NATIVE_IMPL(name) GGML_WEAK_ALIAS(name, name ## _generic)


"generic" implementations such as ggml_vec_dot_q4_0_q8_0_generic are always defined, regardless of the build configuration. And by making these definitions "weak", we allow them to be overwritten by arch-specific implementation.

Is my understanding of this logic correct?

Yes, on supported targets. The linker will first attempt to resolve implemented arch-specific symbols (like ggml_vec_dot_q4_0_q8_0). If an optimized version is not found, it automatically falls back to the _generic variants. I believe this approach offers improved maintainability at the cost of a slight increase in binary size due to these shadowed fallback implementations.

ggerganov · 2025-06-04T12:45:23Z

ggml/src/ggml-cpu/arch/x86/quants.c

+#endif
+    for (; ib < nb; ++ib) {
+        int sumi0 = 0;
+        int sumi1 = 0;
+
+        for (int j = 0; j < qk/2; ++j) {
+            const int v0 = (x[ib].qs[j] & 0x0F) - 8;
+            const int v1 = (x[ib].qs[j] >>   4) - 8;
+
+            sumi0 += (v0 * y[ib].qs[j]);
+            sumi1 += (v1 * y[ib].qs[j + qk/2]);
+        }
+
+        int sumi = sumi0 + sumi1;
+        sumf += sumi*GGML_FP16_TO_FP32(x[ib].d)*GGML_FP16_TO_FP32(y[ib].d);
+    }
+
+    *s = sumf;
+}


Currently, each arch re-implements the generic version. For this big refactoring this is probably the correct approach in order to minimize the risk of introducing bugs. But after we merge, we should consider ways to de-duplicate the scalar implementations by reusing the generic calls?

That should be feasible, though it's not as straightforward for tail elements like the ones in this function. Let's keep the deduplication work for another PR to make future bisecting easier.

ggml/src/ggml-cpu/quants.c

ggml/src/ggml-cpu/arch/arm/quants.c

ggerganov · 2025-06-05T10:25:58Z

Noting that #12995, #13966 and #13996 would need to be reapplied - sorry about the conflict, I should have been more careful and wait before merging these. Let's first finish the review and then resolve these.

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

zhouwg · 2025-06-10T04:50:51Z

@xctan, nice to meet you and sorry to bother you. I'm your fan. I know you are a highly-skilled student in China's No.1 university and did many excellent contributions for llama.cpp and other highly-difficult open-source projects(especially the RISC-V community).

here I have a minor question:

the fp32 4096x4096 addition's performance via the default ggml backend on Qualcomm Snapdragon 8Elite based phone(e.g. Xiaomi15) is 11 milliseconds after I rebase the latest source codes in llama.cpp:

the previous benchmark data of the default ggml backend is about 7-8 milliseconds, now the benchmark data of the default ggml backend is 11 milliseconds.

the previous benchmark data of the ggml-hexagon(via cDSP) is about 15 milliseconds, now is 6-7 milliseconds.

how to reproduce
pls refer to ggml-hexagon v0.97(v20250609) zhouwg/ggml-hexagon#46
currently I don't know the reason(although obviously 11 milliseconds is a good news for my candidate PR) after I carefully checked this commit(I don't think it's caused by this commit), could you help to figure out the reason if you have time?

thanks too much.

l15y · 2025-06-10T07:30:00Z

@xctan, nice to meet you and sorry to bother you. I'm your fan. I know you are a highly-skilled student in China's No.1 university and did many excellent contributions for llama.cpp and other highly-difficult open-source projects(especially the RISC-V community).

here I have a minor question:

the fp32 4096x4096 addition's performance via the default ggml backend on Qualcomm Snapdragon 8Elite based phone(e.g. Xiaomi15) is 11 milliseconds after I rebase the latest source codes in llama.cpp:

the previous benchmark data of the default ggml backend is about 7-8 milliseconds, now the benchmark data of the default ggml backend is 11 milliseconds.

the previous benchmark data of the ggml-hexagon(via cDSP) is about 15 milliseconds, now is 6-7 milliseconds.

how to reproduce
pls refer to ggml-hexagon v0.97(v20250609) zhouwg/ggml-hexagon#46

currently I don't know the reason(although obviously 11 milliseconds is a good news for my candidate PR) after I carefully checked this commit(I don't think it's caused by this commit), could you help to figure out the reason if you have time?

thanks too much.

Encountered a similar issue: the speed on ft-2000 is reduced by one fold.

ggerganov · 2025-06-10T07:40:43Z

If you have noticed a regression, please open a new issue with repro steps.

zhouwg · 2025-06-10T07:45:22Z

@xctan, nice to meet you and sorry to bother you. I'm your fan. I know you are a highly-skilled student in China's No.1 university and did many excellent contributions for llama.cpp and other highly-difficult open-source projects(especially the RISC-V community).
here I have a minor question:

the fp32 4096x4096 addition's performance via the default ggml backend on Qualcomm Snapdragon 8Elite based phone(e.g. Xiaomi15) is 11 milliseconds after I rebase the latest source codes in llama.cpp:

the previous benchmark data of the default ggml backend is about 7-8 milliseconds, now the benchmark data of the default ggml backend is 11 milliseconds.

the previous benchmark data of the ggml-hexagon(via cDSP) is about 15 milliseconds, now is 6-7 milliseconds.

how to reproduce
pls refer to ggml-hexagon v0.97(v20250609) zhouwg/ggml-hexagon#46

currently I don't know the reason(although obviously 11 milliseconds is a good news for my candidate PR) after I carefully checked this commit(I don't think it's caused by this commit), could you help to figure out the reason if you have time?

thanks too much.

Encountered a similar issue: the speed on ft-2000 is reduced by one fold.

hello, nice to meet you, (1)I'm not AI expert and have made a stupid mistake because of my little AI knowledge, (2)I'm not familiar with FT-2000(China's self-developed desktop/server SoC) and also doesn't have such device. Is this new issue submitted/filed by you or me?

thanks.

l15y · 2025-06-10T09:04:38Z

@xctan, nice to meet you and sorry to bother you. I'm your fan. I know you are a highly-skilled student in China's No.1 university and did many excellent contributions for llama.cpp and other highly-difficult open-source projects(especially the RISC-V community).
here I have a minor question:

the fp32 4096x4096 addition's performance via the default ggml backend on Qualcomm Snapdragon 8Elite based phone(e.g. Xiaomi15) is 11 milliseconds after I rebase the latest source codes in llama.cpp:

the previous benchmark data of the default ggml backend is about 7-8 milliseconds, now the benchmark data of the default ggml backend is 11 milliseconds.

the previous benchmark data of the ggml-hexagon(via cDSP) is about 15 milliseconds, now is 6-7 milliseconds.

how to reproduce
pls refer to ggml-hexagon v0.97(v20250609) zhouwg/ggml-hexagon#46

currently I don't know the reason(although obviously 11 milliseconds is a good news for my candidate PR) after I carefully checked this commit(I don't think it's caused by this commit), could you help to figure out the reason if you have time?

thanks too much.

Encountered a similar issue: the speed on ft-2000 is reduced by one fold.

hello, nice to meet you, (1)I'm not AI expert and have made a stupid mistake because of my little AI knowledge, (2)I'm not familiar with FT-2000(China's self-developed desktop/server SoC) and also doesn't have such device. Is this new issue submitted/filed by you or me?

thanks.

HI.
This new issue can be submitted by you, because the generation performance of FT-2000 is not important to me, I just did the test casually, and it is an outdated chip that cannot represent mainstream ARM level.

xctan · 2025-06-10T09:19:26Z

Encountered a similar issue: the speed on ft-2000 is reduced by one fold.

Can you provide the dynamic symbol table in your build (e.g. output of objdump -T bin/libggml-cpu.so) when submitting your issue? Also, the performance may vary between executions, and I haven't observed steady and significant regression on my side.

zhouwg · 2025-06-10T09:34:55Z

@xctan, nice to meet you and sorry to bother you. I'm your fan. I know you are a highly-skilled student in China's No.1 university and did many excellent contributions for llama.cpp and other highly-difficult open-source projects(especially the RISC-V community).
here I have a minor question:

the fp32 4096x4096 addition's performance via the default ggml backend on Qualcomm Snapdragon 8Elite based phone(e.g. Xiaomi15) is 11 milliseconds after I rebase the latest source codes in llama.cpp:

the previous benchmark data of the default ggml backend is about 7-8 milliseconds, now the benchmark data of the default ggml backend is 11 milliseconds.

the previous benchmark data of the ggml-hexagon(via cDSP) is about 15 milliseconds, now is 6-7 milliseconds.

how to reproduce
pls refer to ggml-hexagon v0.97(v20250609) zhouwg/ggml-hexagon#46

currently I don't know the reason(although obviously 11 milliseconds is a good news for my candidate PR) after I carefully checked this commit(I don't think it's caused by this commit), could you help to figure out the reason if you have time?

thanks too much.

Encountered a similar issue: the speed on ft-2000 is reduced by one fold.

hello, nice to meet you, (1)I'm not AI expert and have made a stupid mistake because of my little AI knowledge, (2)I'm not familiar with FT-2000(China's self-developed desktop/server SoC) and also doesn't have such device. Is this new issue submitted/filed by you or me?
thanks.

HI. This new issue can be submitted by you, because the generation performance of FT-2000 is not important to me, I just did the test casually, and it is an outdated chip that cannot represent mainstream ARM level.

thanks for your reply.

as I said before, I don't think that weird problem is caused by this commit after I carefully checked code changes in this commit.
this very complicated commit is from a highly-skilled and highly-reputation developer(both good at system software development and AI tech).
I don't want to make another stupid mistake although the performance of the default ggml backend on aarch64 is important to my candidate PR.

barracuda156 · 2025-06-12T03:59:05Z

This has broken the build on PowerPC: #14138

zhouwg · 2025-06-12T14:42:06Z

firstly, I'd like to emphasize that I still don't think this commit brings performance issue(I think the problem might-be in other place if there is a real performance/regression issue in llama.cpp since June 1 2025).
secondly, I'd like to re-post an interesting comment in my forked llama.cpp project: case-study: NPU performance comparison of GGML_OP_ADD between HWACCEL_CDSP and HWACCEL_QNN zhouwg/ggml-hexagon#34 (comment)

benchmark data of llama-bench on the default branch in that project (ggml-backend.cppv1.10 + ggmldspv0.97 + other scripts and code changes in llama.cpp):
the default ggml backend on Snapdragon 8Elite:
benchmark data of llama-bench on the branch self-build-backup-20250531 in that project ( ggml-hexagon(ggml-backend.cppv1.10 + ggmldspv0.97 + other scripts and code changes in llama.cpp, 100% same as the default branch, backported by me manually and carefully):
the default ggml backend on Snapdragon 8Elite

hope this weird problem will be suddenly disappeared tomorrow.

+whr819987540, @whr819987540, sorry to bother you, pls take a look if you have time, thanks!

there is so much duplicate code in each cpu arch, i expect upstream will prune it eventually arch detection has no fallback if all the arches are not found, by right we should set GGML_CPU_GENERIC i should be relaxing its the weekend

…org#13892" This reverts commit 33809c9.

xctan force-pushed the split-arch branch from 4e0db43 to 2599c59 Compare May 29, 2025 13:59

github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label May 29, 2025

xctan force-pushed the split-arch branch from 272b86e to 87ed654 Compare May 30, 2025 19:30

xctan marked this pull request as ready for review May 30, 2025 21:06

ggerganov self-requested a review June 4, 2025 12:19

ggerganov reviewed Jun 4, 2025

View reviewed changes

ggml/src/ggml-cpu/quants.c Outdated Show resolved Hide resolved

ggml/src/ggml-cpu/quants.c Outdated Show resolved Hide resolved

github-actions bot added Nvidia GPU Issues specific to Nvidia GPUs SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels Jun 4, 2025

ggerganov reviewed Jun 5, 2025

View reviewed changes

ggml/src/ggml-cpu/arch/arm/quants.c Show resolved Hide resolved

xctan added 16 commits June 6, 2025 00:34

move ggml-cpu-aarch64 to repack

6814bd4

split quantize_row_q8_0/1

a07340a

split helper functions

82d7410

split ggml_vec_dot_q4_0_q8_0

ead5762

split ggml_vec_dot_q4_1_q8_1

627e1ec

split ggml_vec_dot_q5_0_q8_0

9582518

split ggml_vec_dot_q5_1_q8_1

beca219

split ggml_vec_dot_q8_0_q8_0

a32715a

split ggml_vec_dot_tq1_0_q8_K

a46eca7

split ggml_vec_dot_tq2_0_q8_K

96a7f51

split ggml_vec_dot_q2_K_q8_K

5f881c9

split ggml_vec_dot_q3_K_q8_K

91fbf27

split ggml_vec_dot_q4_K_q8_K

58b6c62

split ggml_vec_dot_q5_K_q8_K

6272e0c

split ggml_vec_dot_q6_K_q8_K

7c7223f

split ggml_vec_dot_iq2_xxs_q8_K

9671c0e

xctan and others added 12 commits June 6, 2025 00:34

try to fix clang compilation errors again

8713f87

try to fix clang compilation errors, 3rd attempt

df27810

try to fix clang compilation errors, 4th attempt

553d8ca

try to fix clang compilation errors, 5th attempt

9bfcd7e

try to fix clang compilation errors, 6th attempt

08ebdd9

try to fix clang compilation errors, 7th attempt

67eceec

try to fix clang compilation errors, 8th attempt

01a1c5c

try to fix clang compilation errors, 9th attempt

bef5b8d

more cleanup

47701d5

fix compilation errors

e5b6fdb

fix apple targets

2573662

fix a typo in arm version of ggml_vec_dot_q4_K_q8_K

93e6718

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

xctan force-pushed the split-arch branch from 9fa5247 to 93e6718 Compare June 5, 2025 16:35

ggerganov mentioned this pull request Jun 6, 2025

get_rows & dequantize function implementation for repacked weights of type q4_0 (q4_0x8) ggml-org/whisper.cpp#3223

Open

ggerganov approved these changes Jun 6, 2025

View reviewed changes

ggerganov requested a review from slaren June 6, 2025 14:38

slaren approved these changes Jun 9, 2025

View reviewed changes

slaren merged commit f470bc3 into ggml-org:master Jun 9, 2025
45 checks passed

zhouwg mentioned this pull request Jun 14, 2025

Misc. bug: Performance regression on aarch64 q4_0 #14134

Open

Nexesenex added a commit to Nexesenex/croco.cpp that referenced this pull request Jun 15, 2025

Revert "doing what i must because i can, after the mess that is ggml-…

0d729a1

…org#13892" This reverts commit 33809c9.

ggml-cpu : split arch-specific implementations #13892

ggml-cpu : split arch-specific implementations #13892

Conversation

xctan commented May 29, 2025

Uh oh!

xctan commented May 29, 2025

Uh oh!

xctan commented May 31, 2025

Uh oh!

ggerganov Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

xctan Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

ggerganov Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

xctan Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ggerganov commented Jun 5, 2025

Uh oh!

Uh oh!

zhouwg commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l15y commented Jun 10, 2025

Uh oh!

ggerganov commented Jun 10, 2025

Uh oh!

zhouwg commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l15y commented Jun 10, 2025

Uh oh!

xctan commented Jun 10, 2025

Uh oh!

zhouwg commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

barracuda156 commented Jun 12, 2025

Uh oh!

zhouwg commented Jun 12, 2025

Uh oh!

Uh oh!

zhouwg commented Jun 10, 2025 •

edited

Loading

zhouwg commented Jun 10, 2025 •

edited

Loading

zhouwg commented Jun 10, 2025 •

edited

Loading