Skip to content

Conversation

taronaeo
Copy link
Collaborator

Signed-off-by: Aaron Teo <[email protected]>
@taronaeo taronaeo requested a review from CISC October 19, 2025 12:12
@taronaeo taronaeo requested a review from slaren as a code owner October 19, 2025 12:12
@github-actions github-actions bot added the devops improvements to build systems and github actions label Oct 19, 2025
@CISC
Copy link
Collaborator

CISC commented Oct 19, 2025

I think it would be preferable if you updated this for s390x instead, even if it would be just one entry for now:

else()
message(FATAL_ERROR "GGML_CPU_ALL_VARIANTS not yet supported with ${GGML_SYSTEM_ARCH} on ${CMAKE_SYSTEM_NAME}")
endif()

And that requires additional support here:

elseif (GGML_SYSTEM_ARCH STREQUAL "s390x")
message(STATUS "s390x detected")

@CISC
Copy link
Collaborator

CISC commented Oct 19, 2025

What is the exact reason for not being able to make additional builds btw?

Copy link
Collaborator

@CISC CISC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see if GGML_CPU_ALL_VARIANTS is possible, but ok to merge as-is for now.

@taronaeo
Copy link
Collaborator Author

What is the exact reason for not being able to make additional builds btw?

A couple of things:

  1. The current GCC compiler version is GCC 11.4.0, which, if I recall correctly, only recognises the IBM z15 hardware level and maybe IBM z16, I need the compiler and toolchain team to reconfirm this. To build for later hardware levels, we will need to bump the compiler version, but this will affect all other release binaries also.
  2. The CI that we provided via IBM POWER and Z GitHub Runners are running on an IBM z15 machine, which, if memory serves me well, can't build for later hardware levels. Or at least, it will be complaining about not understanding some newer, more optimised instruction sets. But this also depends on how the supplied GCC was built from source in the first place.
  3. All VXE-activated hardware levels (IBM z15 through z17) share the same build flags -mvx -mzvector, but the instruction set is optimised differently for newer hardware levels. So there is no control for the optimisation level except to manually specify the -march= flag, and would lead back to Point 2 if I'm not wrong.

@taronaeo
Copy link
Collaborator Author

I can dig deeper into Point 2 at a later timing to check if Point 2 is still a problem. If it isn't, then I guess we can build for IBM z16 and z17 as well, but it would definitely require a GCC version bump

@taronaeo taronaeo requested a review from ggerganov as a code owner October 19, 2025 13:11
@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Oct 19, 2025
@taronaeo
Copy link
Collaborator Author

I think it would be preferable if you updated this for s390x instead, even if it would be just one entry for now:

else()
message(FATAL_ERROR "GGML_CPU_ALL_VARIANTS not yet supported with ${GGML_SYSTEM_ARCH} on ${CMAKE_SYSTEM_NAME}")
endif()

And that requires additional support here:

elseif (GGML_SYSTEM_ARCH STREQUAL "s390x")
message(STATUS "s390x detected")

Done, looks good here too: https://github.com/ggml-org/llama.cpp/actions/runs/18631002307/job/53115650493. Re-requesting review just in case I missed anything.

@taronaeo taronaeo requested a review from CISC October 19, 2025 14:14
@CISC
Copy link
Collaborator

CISC commented Oct 19, 2025

Ah, you need to implement arch/s390/cpu-feats.cpp and call ggml_add_cpu_backend_features as well.

@CISC
Copy link
Collaborator

CISC commented Oct 19, 2025

Let's merge as-is for now to get release build working again though.

@CISC CISC merged commit 4f73d0a into master Oct 19, 2025
82 of 84 checks passed
@CISC
Copy link
Collaborator

CISC commented Oct 19, 2025

Ah, you need to implement arch/s390/cpu-feats.cpp and call ggml_add_cpu_backend_features as well.

Or since it seems it only built the z15 target perhaps it works just fine...

@taronaeo
Copy link
Collaborator Author

taronaeo commented Oct 20, 2025

Ah, you need to implement arch/s390/cpu-feats.cpp and call ggml_add_cpu_backend_features as well.

Or since it seems it only built the z15 target perhaps it works just fine...

Ehh, you're right. We need to implement arch/s390/cpu-feats.cpp so that GGML knows which backend to load. Otherwise, we'd get this error:

llama_model_load: error loading model: make_cpu_buft_list: no CPU backend found

I'll work on this sometime next week when I get the time. But I guess whoever that downloads the distributable binaries will have to rename libggml-cpu-s390x-z15.so to libggml-cpu.so for it to work properly for now.

@CISC
Copy link
Collaborator

CISC commented Oct 20, 2025

I'll work on this sometime next week when I get the time. But I guess whoever that downloads the distributable binaries will have to rename libggml-cpu-s390x-z15.so to libggml-cpu.so for it to work properly for now.

Good thing I mentioned it might not work yet in the merge then, we'll see if anyone reads that or not. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops improvements to build systems and github actions ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants