-
Notifications
You must be signed in to change notification settings - Fork 73
Jitify2 Windows/Visual Studio 2022 fixes #146
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
base: jitify2
Are you sure you want to change the base?
Jitify2 Windows/Visual Studio 2022 fixes #146
Conversation
I have it at home, not sure about in the office. Could try it later this week/next |
|
Thanks so much for this! I'll need to find time to take a closer look, but I quickly ran the tests on Linux and the only problem is the Regarding the trivially-copyable thing, it seems that trivial copyability of classes with deleted copy constructors is complicated, which may explain why the compilers have different opinions. I don't really even remember why the |
5f57534 to
ef0b040
Compare
aedd6c5 to
132540f
Compare
|
I've now resolved all but one of the test failures under windows (Visual Studio 2022 with CUDA 12.6), which originally were: with equivalent failures for Most failures were related to the location of the cuda home / include directory and/or nvcc, and the presence of the host compiler on the path and could be mitigated with These were caused by / addressed by:
I've resolved this by making
And where Does this seem like an OK solution?
Finally, I'm unsure on why this doesn't work under windows / am not familiar enough with nvjitlink to know if this should work under windows or not, or how to fix it. |
fc48a6d to
e5c59d6
Compare
…arget generators. Provides ctest with the --build-config only if $<CONFIG> genex is not empty
Fixed by splitting the string literal into two adjacent string literals.
…:nvvm in the test suite
This should not be neccessary, but (void)pch_verbose; does not prevent this being raised
…pyable under MSVC
… in guess_cuda_home
This allows for cases where the path to nvcc includes spaces, such as the default CUDA toolkit installation location on Windows
… path (no modified usernames)
…ws in remove_all This enables removal of the temporary directory, suppressing a warning
…st 0 on usage for success Always reads from the pipe even when not capturing output, to ensure that a sigpipe is not raised which prevents the exit code of the underlying command from being accessed
…c++ standard std::result_of was deprecated in c++17 and removed in c++20. This was identified under MSVC in c++20
e5c59d6 to
2c90d02
Compare
…uired by some parts of the CTK (curand)
|
With CUDA 13 (after rebasing on When attempting to build the test suite(s), an error occurs in the MSVC customisations From the .vcxproj files(s), This can be worked around by passing them separately (splitting in cmake and passing as separate variables), but libcudacxx / Passing the
This PR under Windows now has 2 errors under windows, which I'm not sure how to (best) resolve:
Any suggestions on the 2 outstanding test failures @benbarsdell? |
|
@benbarsdell Any chance that you could give @ptheywood some insight into the two failing tests? We are wanting to get a release of FLAME GPU out very soon but are stuck needing this PR to be merged. Could the tests even be skipped on windows to get these changes merged in? |
Compiling the current state of
jitify2(70783a3) under windows / MSVC 2022 was resulting in a number of compilation errors and compilation warnings when included in a separate project.getenvandfopenvia a compiler definitionstd::invoke_result_tfor >= C++17 in place ofstd::result_ofstd::result_ofwas deprecated in C++17 and removed in C++20size_ttointwarning with an explicit castjtiify2::CompilerProgramData::nvvmin the test suite#550-Dset but never used variablepch_verboseon windows(void)pch_verbose;does not prevent this being raised on windowsAdditionally, a few fixes are included for the CMakeLists.txt project in this repository for use under Windows / with Visual Studio 2022 as the generator.
-C/--build-configwith the target genex.I no longer / don't currently have the ability to check with VS2019 as github actions no longer provides an image with it, and I do not have a local install at the moment. We may also be dropping support as we cannot trivally use it via CI anymore.
There are still 2 outstanding issues I have not yet resolved, and some input would be helpful:
The trivially copyable kernel launch argument static assertion is failing for
TEST(Jitify2Test, ClassKernelArg)injitify2_test.cu, whereArgis used. I'm unsure on the correct fix for this as the static assertion looks sound to me.The test could be built with
JITIFY_IGNORE_NOT_TRIVIALLY_COPYABLE_ARGS=1to avoid this, but that feels wrong.With the above test disabled, a number of tests are failing in the jitify2 test suite. I've not yet attempted to resolve these, but will when I have time (ctest log).
I have also only tested this on Windows, so need to ensure I have not caused issues under linux either.
Todo
fd04575with GCC 12.3.0 and CUDA 12.9.86