Skip to content

Conversation

@sandrohanea
Copy link
Contributor

This PR addresses a small issue if GGML_VULKAN_COOPMAT_GLSLC_SUPPORT or GGML_VULKAN_COOPMAT2_GLSLC_SUPPORT is defined as option, after ggml-org/llama.cpp#12272

The current behaviour will forward it to the vulkan-shaders-gen cmake process but it won't add the compile definition for the main cmake process.

If some client will send it as parameter with -DGGML_VULKAN_COOPMAT_GLSLC_SUPPORT=ON will result in runtime issues for some hardware, see #2965 for more details.

CC: @jeffbolznv , @bandoti

Copy link
Contributor

@bandoti bandoti left a comment

Choose a reason for hiding this comment

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

Looks good to me.

set(GGML_VULKAN_COOPMAT_GLSLC_SUPPORT ON CACHE INTERNAL "Whether coopmat is supported by glslc")
endif()
else()
message(STATUS "GL_KHR_cooperative_matrix support already defined: ${GGML_VULKAN_COOPMAT_GLSLC_SUPPORT}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these status messages required? I am okay with keeping it if you think this is useful. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they are good indicators. Not strictly required, but they would have made identifying the issue in #2965 a lot easier. It was strange that no GGML_VULKAN_COOPMAT_GLSLC_SUPPORT indicator existed if it was defined externally.

Let me know if you want to remove it (if you think it is too verbose)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay sounds good.

@ggerganov
Copy link
Member

This is ready to merge? I will then sync it upstream.

@bandoti
Copy link
Contributor

bandoti commented Mar 31, 2025

Yep looks good to merge.

@ggerganov ggerganov merged commit 0008646 into ggml-org:master Mar 31, 2025
47 checks passed
@sandrohanea sandrohanea deleted the sandro/fix-outsidedef-for-vulkan-coopmat branch March 31, 2025 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants