Conversation
suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>
ericcurtin
force-pushed
the
vulkan-compiler-warning
branch
from
6cc3e8b to
7cff721
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a compiler warning about operator precedence in Vulkan code by adding parentheses around the && operation within an || expression to clarify the intended order of operations.
- Adds parentheses to disambiguate logical operator precedence in a boolean expression
github-actions
bot
added
Vulkan
| (!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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]