Skip to content

Fix compiler warning in Vulkan code #15768

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
wants to merge 1 commit into from
Closed

Fix compiler warning in Vulkan code #15768

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions ggml/src/ggml-vulkan/ggml-vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2928,8 +2928,8 @@ static void ggml_vk_load_shaders(vk_device& device) {
const bool use_subgroups = device->subgroup_arithmetic && device->architecture != vk_device_architecture::AMD_GCN;
// Ensure a subgroup size >= 16 is available
const bool use_subgroups16 = use_subgroups &&
(!device->subgroup_size_control && device->subgroup_size >= 16 ||
device->subgroup_size_control && device->subgroup_min_size <= 16 && device->subgroup_max_size >= 16);
((!device->subgroup_size_control && device->subgroup_size >= 16) ||
(device->subgroup_size_control && device->subgroup_min_size <= 16 && device->subgroup_max_size >= 16));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I screwed up the condition again, same thing as with mmid.

It should just be

    const bool use_subgroups16 = use_subgroups && subgroup_min_size_16;

Up to you if you want me to open a PR with that fix and close this PR, or you could update this PR. I haven't tested it yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll let you do it, you can poke me for a review if you want. I was tempted to look at refactoring:

ggml_vk_load_shaders

function in general, looks like there are some de-duplication opportunities. Or it could at least be split into smaller functions, it's over 1000 lines long.

Copy link
Collaborator Author

@ericcurtin ericcurtin Sep 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sometimes I don't like lambda expressions because it promotes these long functions. The lambda's could be short static functions with descriptive names (makes for more detailed stack traces too).


const uint32_t subgroup_size = (device->vendor_id == VK_VENDOR_ID_INTEL && device->subgroup_size_control && device->subgroup_min_size <= 16 && device->subgroup_max_size >= 16) ? 16 : device->subgroup_size;
const uint32_t subgroup_size16 = std::max(subgroup_size, 16u);
Expand Down
Loading