-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[GPU] Fixes compile error on MSVC by disabling Whole Program Optimization #4197
[GPU] Fixes compile error on MSVC by disabling Whole Program Optimization #4197
Conversation
I'm not sure what to say here exactly. This is silently hiding an issue. Should we not provide instead an explicit work around that users need to be aware and activate, e.g. through a cache variable? |
Can also print a warning when its get disabled due to people actively selecting to compile CUDA/GPU. Since its difficult to know, that you have to set the WITH_GLOBALOPTIMIZATION_DISABLED, when compiling for CUDA/GPU on windows? So one will probably end up compiling everything without the flag. If we just print a warning, the user does get a notice about Global optimization being disabled (can be hard to filter it between everything else), but at least things will work. |
Sound argument. From my side proceed with adding the message then. |
Yes, seems like a double unstable CI. But we can try it again :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge if CI green. 3rd time I'm running it.
- Failed with a (potentially) flaky test
- Failed because it ran out of storage space
Temporarily avoids errors like:
#3951
#1113
presumable also this one:
#1418 (I have run the example with this patch)
According to this, it can possible impose a penality of 3-4% performance in normal libraries.
An alternative approach might be to parse different compilerflags to NVCC, instead of all Host flags - however I haven't looked into that just yet.