Skip to content
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.

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

fix: Fix a vulkan-shaders-gen arugment parsing error #10484

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

sparkleholic
Copy link
Contributor

The vulkan-shaders-gen was not parsing the --no-clean argument correctly. Because the previous code was parsing the arguments which have a value only and the --no-clean argument does not have a value, it was not being parsed correctly. This commit can now correctly parse arguments that don't have values.

$ vulkan-shaders-gen --glslc <glslc path> --input-dir ggml/src/vulkan-shaders --output-dir <vulkan-shaders.spv directory> --target-hpp <ggml-vulkan-shaders.hpp output path> --target-cpp <ggml-vulkan-shaders.cpp output path> --no-clean

The vulkan-shaders-gen was not parsing the --no-clean argument correctly.
Because the previous code was parsing the arguments which have a value only
and the --no-clean argument does not have a value, it was not being parsed
correctly. This commit can now correctly parse arguments that don't have values.

<Summary Line>

:Release Notes:

:Detailed Notes:

:Testing Performed:

:QA Notes:

:Issues Addressed:
[GF-9999] Summary
@jeffbolznv
Copy link
Collaborator

Is this covered by #10445?

@sparkleholic
Copy link
Contributor Author

sparkleholic commented Nov 25, 2024

@jeffbolznv
#10445 can fix the issue where the --no-clean is not handled. However, this patch seems better to improve the code that avoid bugs when handling arguments and values, not just the problem of --no-clean. With this patch, arguments doesn't have to be used with a value.

@netrunnereve
Copy link
Collaborator

Yeah this is better than my #10445 as it doesn't alter the build commands (in the unlikely case where someone has a custom script for this) and allows us to keep supporting single arguments in the future. I've tested this and it works properly on my end.

@netrunnereve netrunnereve merged commit 0eb4e12 into ggerganov:master Nov 26, 2024
54 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Dec 20, 2024
)

The vulkan-shaders-gen was not parsing the --no-clean argument correctly.
Because the previous code was parsing the arguments which have a value only
and the --no-clean argument does not have a value, it was not being parsed
correctly. This commit can now correctly parse arguments that don't have values.
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