ggml-blas: Fix BLAS Compile Definitions

[DaAwesomeP]

This PR fixes a few issues in encountered while building with BLAS:

1. When providing BLAS_INCLUDE_DIRS, BLAS compile definitions (BLIS, NVPL, etc.) are not set because the vendor cases are not reached
2. When building with AOCL (AMD's optimized FLAME/BLIS implementation), GGML_BLAS_USE_BLIS was not set because the vendor is not FLAME.
3. OPENBLAS_VERSION does not seem to be a stable way to check for OpenBLAS. It was evaluating as true when I was building with AOCL. I changed it to be consistent with the other options (BLIS and NVPL).
4. GGML_BLAS_USE_NVPL was never set in any case.

[DaAwesomeP]

Hey @ggerganov there doesn't seem to be a code owner for ggml-blas. Who should I request a review from?

[Copilot]

Pull request overview

This PR fixes BLAS compile definition issues where vendor-specific definitions (BLIS, NVPL, OpenBLAS, etc.) were not being set when BLAS_INCLUDE_DIRS was provided externally, causing compilation or linking problems.

Key changes:

• Moves BLAS vendor compile definitions from conditional pkg-config blocks to a consolidated section that executes for all configurations
• Adds support for AOCL and AOCL_mt vendors to set GGML_BLAS_USE_BLIS
• Adds missing GGML_BLAS_USE_NVPL and GGML_BLAS_USE_OPENBLAS compile definitions
• Replaces header-based OPENBLAS_VERSION checks with CMake-based GGML_BLAS_USE_OPENBLAS checks
• Converts multiple independent #if blocks to mutually-exclusive #if/#elif/#elif chains for thread setting

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
ggml/src/ggml-blas/CMakeLists.txt	Removes vendor-specific compile definitions from pkg-config block; adds consolidated compile definition logic that runs for all vendors; adds AOCL support and missing NVPL/OpenBLAS definitions
ggml/src/ggml-blas/ggml-blas.cpp	Replaces OPENBLAS_VERSION with GGML_BLAS_USE_OPENBLAS; converts separate #if blocks to #if/#elif/#elif chains for thread setting and vendor identification

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The header inclusion logic (lines 9-19) doesn't explicitly handle GGML_BLAS_USE_OPENBLAS, falling through to the generic cblas.h include. This differs from the previous approach where OPENBLAS_VERSION (defined by OpenBLAS headers) was used to guard OpenBLAS-specific code, providing a guarantee that the declarations were available. With the new CMake-based GGML_BLAS_USE_OPENBLAS definition, there's a potential mismatch if the wrong cblas.h is found. Consider adding an explicit case for OpenBLAS in the header inclusion chain to include the OpenBLAS-specific header and ensure openblas_set_num_threads() and related functions are properly declared.

[DaAwesomeP]

Odd that Copilot annotated the wrong line. Line 18 is intentionally left as an else because I assume that handles the ATLAS and FlexiBLAS backends. I didn't change that logic.

[ggerganov]

Hm, honestly it's a bit difficult for me to trace the full logic of the changes. Are you confident enough this is an improvement / do you expect any potential issues?

[DaAwesomeP]

The main change is that in the original, each add_compile_definitions only runs if BLAS_FOUND is true. So if you provided your own direct path to the BLAS library with BLAS_INCLUDE_DIRS and GGML_BLAS_VENDOR (not relying on FindBLAS), then none of the backend-specific compile definitions were added. With this change add_compile_definitions always runs for the intended backend regardless of how it is provided to CMake.

[AndVinni]

After this change, compilation warnings appeared in my MinGW64 build:

cmake -B build -DGGML_BLAS=ON -DGGML_BLAS_VENDOR=FLAME -DBLAS_LIBRARIES="D:/Project/blis_install/lib/libblis.a" -DBLAS_INCLUDE_DIRS="D:/Project/blis_install/include" -DCMAKE_PREFIX_PATH="D:/Project/blis_install" -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_FLAGS="-march=core2 -O3 -fno-math-errno" -DCMAKE_CXX_FLAGS="-march=core2 -O3 -fno-math-errno" -DBUILD_SHARED_LIBS=ON -DCMAKE_C_FLAGS="-D_WIN32_WINNT=0x0A00" -DCMAKE_CXX_FLAGS="-D_WIN32_WINNT=0x0A00"

Compilation log:

[7/84] Building CXX object ggml/src/ggml-blas/CMakeFiles/ggml-blas.dir/ggml-blas.cpp.obj
In file included from D:/Project/blis_install/include/blis.h:1,
                 from D:/Project/Repos/llama.cpp/ggml/src/ggml-blas/ggml-blas.cpp:14:
./frame/include//bli_param_macro_defs.h: In function 'bool bli_is_row_vector(dim_t, dim_t)':
./frame/include//bli_param_macro_defs.h:484:52: warning: unused parameter 'n' [-Wunused-parameter]
./frame/include//bli_param_macro_defs.h: In function 'bool bli_is_col_vector(dim_t, dim_t)':
./frame/include//bli_param_macro_defs.h:490:43: warning: unused parameter 'm' [-Wunused-parameter]
./frame/include//bli_param_macro_defs.h: In function 'inc_t bli_vector_inc(trans_t, dim_t, dim_t, inc_t, inc_t)':
./frame/include//bli_param_macro_defs.h:535:65: warning: unused parameter 'n' [-Wunused-parameter]
./frame/include//bli_param_macro_defs.h: In function 'bool bli_is_row_stored(inc_t, inc_t)':
./frame/include//bli_param_macro_defs.h:542:43: warning: unused parameter 'rs' [-Wunused-parameter]
./frame/include//bli_param_macro_defs.h: In function 'bool bli_is_col_stored(inc_t, inc_t)':

...and so on.

AI replaced in \llama.cpp\ggml\src\ggml-blas\CMakeLists.txt

96  - target_include_directories(ggml-blas PRIVATE ${BLAS_INCLUDE_DIRS})
    + target_include_directories(ggml-blas SYSTEM PRIVATE ${BLAS_INCLUDE_DIRS})

And the problem was solved.

D:\AI>c:\llama\b\llama-bench.exe   -m d:\AI\nomic-embed-text-v1.Q4_0.gguf   -t 2   -embd 1   -mmp 0
| model                          |       size |     params | backend    | threads | mmap |       embd |            test |                  t/s |
| ------------------------------ | ---------: | ---------: | ---------- | ------: | ---: | ---------: | --------------: | -------------------: |
| nomic-bert 137M Q4_0           |  73.48 MiB |   136.73 M | BLAS       |       2 |    0 |          1 |           pp512 |         54.04 ┬▒ 0.38 |
| nomic-bert 137M Q4_0           |  73.48 MiB |   136.73 M | BLAS       |       2 |    0 |          1 |           tg128 |         31.25 ┬▒ 0.52 |

[DaAwesomeP]

@AndVinni I agree with the AI suggestion to hide those warnings. Would you like to open a pull or should I?

It seems that I fixed a bug for your build: Like me, you are setting GGML_BLAS_VENDOR, BLAS_INCLUDE_DIRS, and BLAS_LIBRARIES and bypassing FindBLAS. Before this change GGML_BLAS_USE_BLIS would not be set for you. If you weren't seeing these warnings before, then it seems like it wasn't running line 14 (include <blis.h>) at all and falling back to the generic CBLAS header. More significantly, that means it would never call bli_thread_set_num_threads() (line 121).

[AndVinni]

@DaAwesomeP

Please proceed as you see fit.
I am not deeply involved in the project, but I am interested in it.
The fixes suggested by the AI are simply a self-check of the root cause of the issue.
There are likely more cross-platform ways to fix the problem.

I will not have access to the industrial computer on which I built it for a long time, so I will not be able to re-verify.