Skip to content

Fixed cooperative matrix shaders on Vulkan builds when CROSS_COMPILE ON #2914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sandrohanea wants to merge 6 commits into from
Closed

Fixed cooperative matrix shaders on Vulkan builds when CROSS_COMPILE ON #2914

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
04ab3d3
Fixed COOPMAT on Vulkan builds with CROSS_COMPILE ON
Mar 20, 2025
e6ab39f
Fixed a warning to retrigger CI (failed transiently last time)
Mar 21, 2025
62e5be8
Simplified the condition to just pass the DGGML_VULKAN_COOPMAT_GLSLC_…
Mar 26, 2025
eaa2808
Add status messages for GL_NV_cooperative_matrix2 support in Vulkan
Mar 26, 2025
3275375
Enhance Vulkan CMake configuration to allow explicit glslc executable…
Mar 26, 2025
3519d2d
WIP Refactor Vulkan CMake configuration to improve glslc executable h…
Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 44 additions & 21 deletions ggml/src/ggml-vulkan/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,54 @@ if (Vulkan_FOUND)
ggml-vulkan.cpp
../../include/ggml-vulkan.h
)
if (CMAKE_CROSSCOMPILING)
# Make this configurable for cross builds

@jeffbolznv jeffbolznv Mar 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand this part. Why does cross compiling skip the checks?

@sandrohanea sandrohanea Mar 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before this change, it was not skipping this check, but it was not forwarding it to the vulkan-shaders-gen as for CMAKE_CROSSCOMPILING it is starting it with ExternalProject_Add so the compile definition added with add_compile_definitions(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT) was not forwarded to that external project (only added to the current project.

For not CMAKE_CROSSCOMPILING it was added with add_subdirectory which maintained the compile definitions:
image

After my change, for CMAKE_CROSSCOMPILING, we don't try to detect the version of the coopmap based on the current system but retrieving it from the options and pass it to the external project as well.

@jeffbolznv jeffbolznv Mar 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it work if you remove this condition? i.e. detect whether it's supported and pass it to the external project?

@sandrohanea sandrohanea Mar 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that should work:

  • if we remove this condition
  • detect in the main project if we have the COOPMAT and which version
  • pass it to the vulkan-shaders-gen external project
  • in the CMakeLists for that one, we should still check the option and call target_compile_definitions

The reason why I didn't use this approach was that I was thinking that it is clearer to not check the build host for this version when CROSS_COMPILING as we're not building for this system => but indeed, if this system won't have that specific version, we won't be able to build anyway.

I'm happy to simplify it like that if you think it would be better :)

@sandrohanea sandrohanea Mar 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Link to CI in Whisper.net after the change: https://github.com/sandrohanea/whisper.net/actions/runs/14087208133/job/39454313299

option(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT "Enable coopmat shaders" OFF)
option(GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT "Enable coopmat2 shaders" OFF)
else()
# Compile a test shader to determine whether GL_KHR_cooperative_matrix is supported.
# If it's not, there will be an error to stderr.
# If it's supported, set a define to indicate that we should compile those shaders
execute_process(COMMAND ${Vulkan_GLSLC_EXECUTABLE} -o - -fshader-stage=compute --target-env=vulkan1.3 "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/test_coopmat_support.comp"
OUTPUT_VARIABLE glslc_output
ERROR_VARIABLE glslc_error)

if (${glslc_error} MATCHES ".*extension not supported: GL_KHR_cooperative_matrix.*")
message(STATUS "GL_KHR_cooperative_matrix not supported by glslc")
set(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT OFF CACHE INTERNAL "Not enable coopmat shaders")
else()
message(STATUS "GL_KHR_cooperative_matrix supported by glslc")
set(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT ON CACHE INTERNAL "Enable coopmat shaders")
endif()

# Compile a test shader to determine whether GL_KHR_cooperative_matrix is supported.
# If it's not, there will be an error to stderr.
# If it's supported, set a define to indicate that we should compile those shaders
execute_process(COMMAND ${Vulkan_GLSLC_EXECUTABLE} -o - -fshader-stage=compute --target-env=vulkan1.3 "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/test_coopmat_support.comp"
OUTPUT_VARIABLE glslc_output
ERROR_VARIABLE glslc_error)
# Compile a test shader to determine whether GL_NV_cooperative_matrix2 is supported.
# If it's not, there will be an error to stderr.
# If it's supported, set a define to indicate that we should compile those shaders
execute_process(COMMAND ${Vulkan_GLSLC_EXECUTABLE} -o - -fshader-stage=compute --target-env=vulkan1.3 "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/test_coopmat2_support.comp"
OUTPUT_VARIABLE glslc_output
ERROR_VARIABLE glslc_error)

if (${glslc_error} MATCHES ".*extension not supported: GL_KHR_cooperative_matrix.*")
message(STATUS "GL_KHR_cooperative_matrix not supported by glslc")
else()
message(STATUS "GL_KHR_cooperative_matrix supported by glslc")
add_compile_definitions(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT)
if (${glslc_error} MATCHES ".*extension not supported: GL_NV_cooperative_matrix2.*")
message(STATUS "GL_NV_cooperative_matrix2 not supported by glslc")
set(GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT OFF CACHE INTERNAL "Not enable coopmat2 shaders")
else()
message(STATUS "GL_NV_cooperative_matrix2 supported by glslc")
set(GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT ON CACHE INTERNAL "Enable coopmat2 shaders")
endif()
endif()

# Compile a test shader to determine whether GL_NV_cooperative_matrix2 is supported.
# If it's not, there will be an error to stderr.
# If it's supported, set a define to indicate that we should compile those shaders
execute_process(COMMAND ${Vulkan_GLSLC_EXECUTABLE} -o - -fshader-stage=compute --target-env=vulkan1.3 "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/test_coopmat2_support.comp"
OUTPUT_VARIABLE glslc_output
ERROR_VARIABLE glslc_error)

if (${glslc_error} MATCHES ".*extension not supported: GL_NV_cooperative_matrix2.*")
message(STATUS "GL_NV_cooperative_matrix2 not supported by glslc")
if (GGML_VULKAN_COOPMAT_GLSLC_SUPPORT)
message(STATUS "GGML_VULKAN_COOPMAT_GLSLC_SUPPORT enabled")
add_compile_definitions(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT)
else()
message(STATUS "GL_NV_cooperative_matrix2 supported by glslc")
message(STATUS "GGML_VULKAN_COOPMAT_GLSLC_SUPPORT disabled")
endif()

if (GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT)
message(STATUS "GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT enabled")
add_compile_definitions(GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT)
else()
message(STATUS "GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT disabled")
endif()

target_link_libraries(ggml-vulkan PRIVATE Vulkan::Vulkan)
Expand Down Expand Up @@ -119,6 +140,8 @@ if (Vulkan_FOUND)
SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders
CMAKE_ARGS -DCMAKE_TOOLCHAIN_FILE=${HOST_CMAKE_TOOLCHAIN_FILE}
-DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}
-DGGML_VULKAN_COOPMAT_GLSLC_SUPPORT=${GGML_VULKAN_COOPMAT_GLSLC_SUPPORT}
-DGGML_VULKAN_COOPMAT2_GLSLC_SUPPORT=${GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT}
BUILD_COMMAND ${CMAKE_COMMAND} --build .
INSTALL_COMMAND ${CMAKE_COMMAND} --install .
INSTALL_DIR ${CMAKE_BINARY_DIR}
Expand Down
2 changes: 1 addition & 1 deletion ggml/src/ggml-vulkan/ggml-vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1394,7 +1394,7 @@ static std::array<uint32_t, 2> fa_rows_cols(uint32_t D, uint32_t clamp, ggml_typ
return {64, 32};
}
return {64, 64};
};
}

static bool ggml_vk_matmul_shmem_support(const vk_device& device, const std::vector<uint32_t>& warptile, bool mul_mat_id, ggml_type src0_type) {

Expand Down
14 changes: 14 additions & 0 deletions ggml/src/ggml-vulkan/vulkan-shaders/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,22 @@ if(NOT GLSLC_EXECUTABLE)
message(FATAL_ERROR "glslc not found.")
endif()

option(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT "Enable coopmat shaders" OFF)
option(GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT "Enable coopmat2 shaders" OFF)

message(STATUS "GGML_VULKAN_COOPMAT_GLSLC_SUPPORT: ${GGML_VULKAN_COOPMAT_GLSLC_SUPPORT}")
message(STATUS "GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT: ${GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT}")

set(TARGET vulkan-shaders-gen)
add_executable(${TARGET} vulkan-shaders-gen.cpp)
if (GGML_VULKAN_COOPMAT_GLSLC_SUPPORT)

@jeffbolznv jeffbolznv Mar 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In ggml-org/llama.cpp#11695 (comment) I had guessed that we may need to pass through a cmake variable telling this makefile which version of glslc to use? Is that not necessary? It's just the #defines that weren't making it through?

@sandrohanea sandrohanea Mar 26, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% sure if we need additional version to be used (and in my case it was working as same version as the host system was used). I can confirm that only providing the compile definitions is working for my build => https://github.com/sandrohanea/whisper.net/actions/runs/13977409227/job/39134498314

target_compile_definitions(vulkan-shaders-gen PRIVATE GGML_VULKAN_COOPMAT_GLSLC_SUPPORT)
endif()

if (GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT)
target_compile_definitions(vulkan-shaders-gen PRIVATE GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT)
endif()

install(TARGETS ${TARGET} RUNTIME)
target_compile_features(${TARGET} PRIVATE cxx_std_17)
target_link_libraries(vulkan-shaders-gen PUBLIC Threads::Threads)