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

build : pass all warning flags to nvcc via -Xcompiler #5570

Merged
merged 3 commits into from
Feb 18, 2024
Merged

Conversation

cebtenzzre
Copy link
Collaborator

Ref #5543

Fixes #5542

@cebtenzzre
Copy link
Collaborator Author

Looks like PR #3952 broke the Makefile. I promise, this stuff all worked when I wrote it :)

We intentionally craft a GF_CC for CUDA that will fail but print the
desired version anyway. So instead of checking whether the version
command was successful, we need to check if the command printed
anything.
@cebtenzzre cebtenzzre changed the title build: pass all warning flags to nvcc via -Xcompiler build : pass all warning flags to nvcc via -Xcompiler Feb 18, 2024
@cebtenzzre cebtenzzre requested a review from slaren February 18, 2024 21:10
Copy link
Member

@slaren slaren left a comment

Choose a reason for hiding this comment

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

This seems to fix it.

@cebtenzzre cebtenzzre merged commit a0c2dad into master Feb 18, 2024
40 of 56 checks passed
@cebtenzzre cebtenzzre deleted the fix-nvcc-wall branch February 18, 2024 21:21
@bartowski1182
Copy link
Contributor

For the record this made my builds stop working with this error:

35.96 /usr/include/c++/11/type_traits:79:52: error: redefinition of 'constexpr const _Tp std::integral_constant<_Tp, __v>::value'
35.96    79 |   template<typename _Tp, _Tp __v>
35.96       |                                                    ^                           
35.96 /usr/include/c++/11/type_traits:67:29: note: 'constexpr const _Tp value' previously declared here
35.96    67 |       static constexpr _Tp                  value = __v;
35.96       |                             ^~~~~
36.46 make: *** [Makefile:447: ggml-cuda.o] Error 1

Reverting this change made it go through, though I imagine it's more likely that something else is causing a true error and your change is making it be accurately reported?

@slaren
Copy link
Member

slaren commented Feb 19, 2024

I noticed that with make builds, NDEBUG is no longer defined in CUDA builds, even without LLAMA_DEBUG. I think that passing the preprocessor defines to nvcc with -Xcompiler doesn't work, and should be passed directly to nvcc instead. What would be the best way to do this? From what I understand, the preprocessor defined are already separately defined in CPPFLAGS, so this should be doable without major changes.

@cebtenzzre
Copy link
Collaborator Author

I think that passing the preprocessor defines to nvcc with -Xcompiler doesn't work, and should be passed directly to nvcc instead.

#5598

jordankanter pushed a commit to jordankanter/llama.cpp that referenced this pull request Mar 13, 2024
* build : pass all warning flags to nvcc via -Xcompiler
* make : fix apparent mis-merge from ggml-org#3952
* make : fix incorrect GF_CC_VER for CUDA host compiler
hodlen pushed a commit to hodlen/llama.cpp that referenced this pull request Apr 1, 2024
* build : pass all warning flags to nvcc via -Xcompiler
* make : fix apparent mis-merge from ggml-org#3952
* make : fix incorrect GF_CC_VER for CUDA host compiler
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.

CMAKE error nvcc fatal : Unknown option '-Wall'
3 participants