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

chore : Fix vulkan related compiler warnings, add help text, improve CLI options #8477

Merged
merged 8 commits into from
Jul 28, 2024

Conversation

teleprint-me
Copy link
Contributor

@teleprint-me teleprint-me commented Jul 14, 2024

Fix vulkan related compiler warnings, add help text, improve CLI options

Overview

  • Add prototypes for function definitions
  • Invert logic of --no-clean option to be more intuitive
  • Provide a new help prompt with clear instructions

Details

This PR addresses several issues related to the CLI tool for generating Vulkan shaders. The main changes include adding prototypes for function definitions, inverting the logic of the --no-clean option, and providing a new help prompt with clear instructions on how to use the tool effectively.

Notes

The addition of prototypes ensures that there are no undefined references during compilation. Inverting the logic of the --no-clean option makes it more intuitive for users to specify whether or not they want to remove temporary files after building. The new help prompt provides clear instructions on how to use the tool, making it easier for others to adopt and understand its capabilities.

Summary

I've tested this PR locally and ensured that all changes are functional and do not introduce any regressions in existing functionality.

$ ctest --rerun-failed --output-on-failure
Test project /mnt/valerie/forked/ggerganov/llama.cpp/build
# ...
100% tests passed, 0 tests failed out of 25

Label Time Summary:
curl             =   0.61 sec*proc (1 test)
eval-callback    =   0.61 sec*proc (1 test)
main             =  48.64 sec*proc (22 tests)
model            =   0.01 sec*proc (2 tests)

Total Test time (real) =  49.28 sec

Please review my changes and let me know if there are any questions or concerns before merging!

* Add prototypes for function definitions
* Invert logic of --no-clean option to be more intuitive
* Provide a new help prompt with clear instructions
@teleprint-me teleprint-me changed the title chore: Fix compiler warnings, add help text, improve CLI options chore: Fix vulkan related compiler warnings, add help text, improve CLI options Jul 14, 2024
@teleprint-me teleprint-me changed the title chore: Fix vulkan related compiler warnings, add help text, improve CLI options Fix vulkan related compiler warnings, add help text, improve CLI options Jul 14, 2024
@teleprint-me teleprint-me changed the title Fix vulkan related compiler warnings, add help text, improve CLI options chore : Fix vulkan related compiler warnings, add help text, improve CLI options Jul 14, 2024
Signed-off-by: teleprint-me <77757836+teleprint-me@users.noreply.github.com>
@teleprint-me
Copy link
Contributor Author

I didn't notice that the binary wasn't ignored until afterwards. It's a minor quality of life change. Hopefully it's okay to bulk it in with this PR.

@mofosyne mofosyne added the Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix label Jul 14, 2024
@0cc4m 0cc4m self-requested a review July 27, 2024 10:36
Copy link
Collaborator

@0cc4m 0cc4m left a comment

Choose a reason for hiding this comment

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

Just minor comments, looks good and works fine on Linux.

ggml/src/vulkan-shaders/vulkan-shaders-gen.cpp Outdated Show resolved Hide resolved
ggml/src/vulkan-shaders/vulkan-shaders-gen.cpp Outdated Show resolved Hide resolved
@0cc4m 0cc4m merged commit 4730fac into ggerganov:master Jul 28, 2024
53 checks passed
@teleprint-me teleprint-me deleted the fix-vulkan-shader-warnings branch July 28, 2024 07:53
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Aug 2, 2024
…CLI options (ggerganov#8477)

* chore: Fix compiler warnings, add help text, improve CLI options

* Add prototypes for function definitions
* Invert logic of --no-clean option to be more intuitive
* Provide a new help prompt with clear instructions

* chore : Add ignore rule for vulkan shader generator

Signed-off-by: teleprint-me <77757836+teleprint-me@users.noreply.github.com>

* Update ggml/src/vulkan-shaders/vulkan-shaders-gen.cpp

Co-authored-by: 0cc4m <picard12@live.de>

* chore : Remove void and apply C++ style empty parameters

* chore : Remove void and apply C++ style empty parameters

---------

Signed-off-by: teleprint-me <77757836+teleprint-me@users.noreply.github.com>
Co-authored-by: 0cc4m <picard12@live.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants