Some portability improvements from trying to build with Visual Studio 2017 #12150
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tried getting llama.cpp to build on a machine that only had Visual Studio 2017 installed. In trying to get my build to work came across a few things that look like potential obstacles for portability, so I am proposing them as a patch, even if Visual Studio 2017 is probably not a major target in itself.
Here is a summary of the changes:
restrict
keyword byGGML_RESTRICT
macro__restrict
keyword instead ofrestrict
in plain C mode (this is the only change that is really useful only for VS 2017)<algorithm>
forstd::min
andstd::max
(as per https://en.cppreference.com/w/cpp/algorithm/min)<cctype>
forstd::toupper
andstd::tolower
(as per https://en.cppreference.com/w/cpp/string/byte/tolower)_USE_MATH_DEFINES
to the top of the source file to ensure includingM_PI
regardless of which header it is inFor completeness, I was eventually able to build and run successfully with Visual Studio 2017, but only after commenting out the check using
codecvt_utf8<char32_t>
infs_validate_filename()
, as this specialization is not available in the runtime library of VS 2017. Fixing this would probably need implementing a different type of check for non-standard UTF-8 encodings, so this would only be worthwhile if support of VS 2017 is desired.