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

<cmath>: New intrinsics broke CUDA #1885

Closed
StephanTLavavej opened this issue Apr 29, 2021 · 6 comments · Fixed by #1886
Closed

<cmath>: New intrinsics broke CUDA #1885

StephanTLavavej opened this issue Apr 29, 2021 · 6 comments · Fixed by #1886
Labels
bug Something isn't working fixed Something works now, yay! high priority Important!

Comments

@StephanTLavavej
Copy link
Member

When #1336 added usage of new intrinsics to <cmath>, we forgot about CUDA. 🙀 (Long ago, we broke CUDA by adding new type traits intrinsics in C++14 mode, but these math functions were just different enough that I didn't remember the interaction. Oops!)

I'm not exactly sure why our CUDA unit test didn't catch this, given that the affected overloads aren't templates:

STL/stl/inc/cmath

Lines 62 to 70 in f675d68

_NODISCARD _Check_return_ inline float ceil(_In_ float _Xx) noexcept /* strengthened */ {
#ifdef _M_CEE
return _CSTD ceilf(_Xx);
#elif defined(__clang__)
return __builtin_ceilf(_Xx);
#else // ^^^ __clang__ / !__clang__ vvv
return __ceilf(_Xx);
#endif // __clang__
}

We are including <cmath>, so there's probably some detail of the CUDA compilation process that I don't understand:

#include <cmath>

#define _MSVC_TESTING_NVCC
#include <__msvc_all_public_headers.hpp>

In any event, fixing this should be easy, we just need to backport it to VS 2019 16.9.

Originally encountered in pytorch/pytorch#54382 and tracked by Microsoft-internal VSO-1314894 / AB#1314894 .

@sylveon
Copy link
Contributor

sylveon commented Apr 29, 2021

I believe the CI did not catch this because it only seems to happen when building device code, not host code (not familiar with CUDA but the error message seems to imply this)

@EricAtORS
Copy link

Is this related as well?
https://devtalk.blender.org/t/cuda-compile-error-windows-10/17886

I experienced that issue with other cuda as well, but the suggestion to switch from floor to floorf made it compile.

@sylveon
Copy link
Contributor

sylveon commented Apr 29, 2021

Yes, floorf is unaffected because it comes from the UCRT, which the PR couldn't update since it isn't open source.

@StephanTLavavej StephanTLavavej added fixed Something works now, yay! and removed work in progress labels Apr 30, 2021
@IngmarVoigt2
Copy link

Hi @StephanTLavavej,

is this already on the latest Visual Studio releases? You mentioned this should be backported to v16.9, no? If not do you know when we could expect this to be released? I neither saw this on the release notes for v16.9 nor for v16.10.

Thx a lot!

@IngmarVoigt2
Copy link

Nevermind, I realized it's already in 16.10.2 at least

@StephanTLavavej
Copy link
Member Author

@IngmarVoigt2 Yep, this shipped in 16.10.0 and was backported to 16.9.7. The VS and MSVC release notes don't contain this level of detail, but the STL's Changelog does - see https://github.com/microsoft/STL/wiki/Changelog#vs-2019-1610 and search for the PR number #1886.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay! high priority Important!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants