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

CUDA Compiler Fixes #8355

Merged
merged 6 commits into from
Feb 17, 2021
Merged

CUDA Compiler Fixes #8355

merged 6 commits into from
Feb 17, 2021

Conversation

obilaniu
Copy link
Contributor

This is an extensive rewrite and hopefully hardening of the CUDA NVCC compiler object and its flag handling.

The class method _to_host_flags() is entirely rewritten and gains knowledge of the entire nvcc argument set in the official documentation. The method now translates an array of mixed GCC/NVCC or MVSC/NVCC flags into a legal argument set for nvcc, exploiting -Xcompiler and -Xlinker to pass down foreign arguments into the compiler. It does the best it can to take into account and paper over the peculiarities of NVCC argument parsing.

Builds successfully the entire test cases/cuda testsuite on my machine (with CUDA Toolkit 10.2).

Related to #8337. @andrzejc, would you like to test out this branch?

@obilaniu
Copy link
Contributor Author

CI failure is real - somewhere the DT_RPATH $ORIGIN is getting expanded into nothingness.

@jpakkane jpakkane added this to the 0.57.1 milestone Feb 15, 2021
@andrzejc
Copy link

Related to #8337. @andrzejc, would you like to test out this branch?

Thank you @obilaniu, this is awesome, I'm definitely gonna give it a try.

@obilaniu obilaniu force-pushed the cudafixes branch 2 times, most recently from 01cf8af to 6f3a56d Compare February 16, 2021 11:49
@obilaniu
Copy link
Contributor Author

@andrzejc I have solved the CUDA CI failure. If you have Linux, this should definitely work. If you have Windows, it would be extremely valuable if you tried this out to ensure it does not break Windows CUDA.

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 3 alerts when merging 6f3a56d into 1104575 - view on LGTM.com

new alerts:

  • 3 for Unused local variable

@obilaniu obilaniu force-pushed the cudafixes branch 2 times, most recently from 424b6ec to 064a270 Compare February 16, 2021 17:04
@andrzejc
Copy link

@obilaniu I just finished testing with Linux box, seems to work perfectly, thank you! I'll check on Windows tomorrow.

@obilaniu
Copy link
Contributor Author

@andrzejc With the rebase of an hour ago or yesterday?

@andrzejc
Copy link

@andrzejc With the rebase of an hour ago or yesterday?

with the rebase

@andrzejc
Copy link

@obilaniu so I was curious about Windows and played with building with MSVC a moment ago and it also seems to work without any issues. I was able to compile & link my project with MSVC2017/CUDA 10.0 configuration and all the tests are passing just like they did with the generator approach. Awesome!

@obilaniu obilaniu changed the title [WIP] CUDA Compiler Fixes CUDA Compiler Fixes Feb 16, 2021
@obilaniu
Copy link
Contributor Author

@andrzejc Great! @jpakkane I think this is ready for review.

Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

I've got a few style nits, but nothing serious.

mesonbuild/compilers/cuda.py Outdated Show resolved Hide resolved
mesonbuild/compilers/cuda.py Outdated Show resolved Hide resolved
mesonbuild/compilers/cuda.py Outdated Show resolved Hide resolved
@dcbaker dcbaker merged commit 7812cee into mesonbuild:master Feb 17, 2021
@obilaniu obilaniu deleted the cudafixes branch February 17, 2021 03:04
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.

5 participants