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][HIP] std::min/max differs from libstdc++/libc++ about NaN #93962

Closed
yxsamliu opened this issue May 31, 2024 · 0 comments · Fixed by #93976
Closed

[CUDA][HIP] std::min/max differs from libstdc++/libc++ about NaN #93962

yxsamliu opened this issue May 31, 2024 · 0 comments · Fixed by #93976
Assignees
Labels
clang:headers Headers provided by Clang, e.g. for intrinsics cuda

Comments

@yxsamliu
Copy link
Collaborator

CUDA/HIP defines overloaded version of host device std::min/max with higher precedence than those in standard C++ headers:

min(const __T &__a, const __T &__b) {

https://github.com/gcc-mirror/gcc/blob/d9c90c82d900fdae95df4499bf5f0a4ecb903b53/libstdc%2B%2B-v3/include/bits/stl_algobase.h#L233

return std::min(__a, __b, __less<>());

However, there is a subtle difference of the CUDA/HIP version compared to libstdc++/libc++ version.

CUDA/HIP version is like __a < __b ? __a : __b. If there is NaN, it chooese __b.

libstdc++/libc++ version is like __b < __a ? __b : __a. If there is NaN, it chooses __a.

This difference causes annoyance for PyTorch

ROCm/HIP#2209 (comment)

according to https://en.cppreference.com/w/cpp/algorithm/min

std::min(a,b) returns the smaller of a and b. If the values are equivalent, returns a. It seems libstdc++/libc++ behavior is correct. CUDA/HIP std::min/max needs to be fixed.

@Artem-B

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label May 31, 2024
@yxsamliu yxsamliu self-assigned this May 31, 2024
@philnik777 philnik777 added clang:headers Headers provided by Clang, e.g. for intrinsics and removed libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels May 31, 2024
yxsamliu added a commit to yxsamliu/llvm-project that referenced this issue May 31, 2024
The std::min behaves like 'a<b?a:b', which does not match libstdc++/libc++
behavior like 'b<a?b:a' when input is NaN.

Make it consistent with libstdc++/libc++.

Fixes: llvm#93962

Fixes: ROCm/HIP#3502
yxsamliu added a commit that referenced this issue Jun 3, 2024
The std::min behaves like 'a<b?a:b', which does not match
libstdc++/libc++ behavior like 'b<a?b:a' when input is NaN.

Make it consistent with libstdc++/libc++.

Fixes: #93962

Fixes: ROCm/HIP#3502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:headers Headers provided by Clang, e.g. for intrinsics cuda
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants