Skip to content

pytorch clamp fix for fedora 40 #37

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

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

lamikr
Copy link
Owner

@lamikr lamikr commented Jun 2, 2024

fedora 40/gcc 14 build failure. Replace clamp with min and max.

pytorch/pytorch#127666
#12

fedora 40/gcc 14 build failure. Replace clamp with min and max.

pytorch/pytorch#127666
#12

Signed-off-by: Mika Laitio <[email protected]>
@lamikr lamikr merged commit c9f6a00 into master Jun 3, 2024
jeffdaily added a commit to ROCm/pytorch that referenced this pull request Jun 3, 2024
Fixes pytorch#127666.

Other std math functions are replaced with those in the global namespace
during hipify. HIP does not claim to support every function in the C++
standard library. std::clamp is not yet supported and we have been
relying on the std implementation. For Fedora 40 + gcc 14, a host-side
assert is used which is not supported. Work-around this by replacing
std::clamp with min and max for USE_ROCM builds.

Patch comes from @lamikr. Modified to use #ifndef USE_ROCM.

lamikr/rocm_sdk_builder#37
@lamikr lamikr deleted the wip/rocm_sdk_builder_611_bg12_pytorch_clamp branch June 8, 2024 19:34
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Jul 1, 2024
Fixes #127666.

Other std math functions are replaced with those in the global namespace during hipify. HIP does not claim to support every function in the C++ standard library. std::clamp is not yet supported and we have been relying on the std implementation. For Fedora 40 + gcc 14, a host-side assert is used which is not supported. Work-around this by replacing std::clamp with min and max for USE_ROCM builds.

Patch comes from @lamikr. Modified to use #ifndef USE_ROCM.

lamikr/rocm_sdk_builder#37

Pull Request resolved: #127812
Approved by: https://github.com/hongxiayang, https://github.com/malfet
pytorchmergebot pushed a commit to khushi-411/pytorch that referenced this pull request Jul 2, 2024
Fixes pytorch#127666.

Other std math functions are replaced with those in the global namespace during hipify. HIP does not claim to support every function in the C++ standard library. std::clamp is not yet supported and we have been relying on the std implementation. For Fedora 40 + gcc 14, a host-side assert is used which is not supported. Work-around this by replacing std::clamp with min and max for USE_ROCM builds.

Patch comes from @lamikr. Modified to use #ifndef USE_ROCM.

lamikr/rocm_sdk_builder#37

Pull Request resolved: pytorch#127812
Approved by: https://github.com/hongxiayang, https://github.com/malfet
pytorchmergebot pushed a commit to ROCm/pytorch that referenced this pull request Jul 16, 2024
Fixes pytorch#127666.

Other std math functions are replaced with those in the global namespace
during hipify. HIP does not claim to support every function in the C++
standard library. std::clamp is not yet supported and we have been
relying on the std implementation. For Fedora 40 + gcc 14, a host-side
assert is used which is not supported. Work-around this by replacing
std::clamp with min and max for USE_ROCM builds.

Patch comes from @lamikr. Modified to use #ifndef USE_ROCM.

lamikr/rocm_sdk_builder#37
pytorch-bot bot pushed a commit to pytorch/pytorch that referenced this pull request Jul 17, 2024
Fixes #127666.

Other std math functions are replaced with those in the global namespace
during hipify. HIP does not claim to support every function in the C++
standard library. std::clamp is not yet supported and we have been
relying on the std implementation. For Fedora 40 + gcc 14, a host-side
assert is used which is not supported. Work-around this by replacing
std::clamp with min and max for USE_ROCM builds.

Patch comes from @lamikr. Modified to use #ifndef USE_ROCM.

lamikr/rocm_sdk_builder#37
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Jul 17, 2024
Fixes #127666.

Other std math functions are replaced with those in the global namespace during hipify. HIP does not claim to support every function in the C++ standard library. std::clamp is not yet supported and we have been relying on the std implementation. For Fedora 40 + gcc 14, a host-side assert is used which is not supported. Work-around this by replacing std::clamp with min and max.  Using #ifndef USE_ROCM to differentiate between CUDA using std::clamp and the ROCm replacement broke Windows builds. The replacement generates the same PTX as std::clamp, so using the replacement unconditionally. The replacement generates the same PTX as std::clamp. See https://godbolt.org/z/Wde9KW3v4 for a sample.

Original patch comes from @lamikr. Modified to improve efficiency.

lamikr/rocm_sdk_builder#37

Co-authored-by: Nikita Shulga <[email protected]>
Pull Request resolved: #127812
Approved by: https://github.com/hongxiayang, https://github.com/malfet
DiweiSun pushed a commit to DiweiSun/pytorch that referenced this pull request Jul 22, 2024
Fixes pytorch#127666.

Other std math functions are replaced with those in the global namespace during hipify. HIP does not claim to support every function in the C++ standard library. std::clamp is not yet supported and we have been relying on the std implementation. For Fedora 40 + gcc 14, a host-side assert is used which is not supported. Work-around this by replacing std::clamp with min and max.  Using #ifndef USE_ROCM to differentiate between CUDA using std::clamp and the ROCm replacement broke Windows builds. The replacement generates the same PTX as std::clamp, so using the replacement unconditionally. The replacement generates the same PTX as std::clamp. See https://godbolt.org/z/Wde9KW3v4 for a sample.

Original patch comes from @lamikr. Modified to improve efficiency.

lamikr/rocm_sdk_builder#37

Co-authored-by: Nikita Shulga <[email protected]>
Pull Request resolved: pytorch#127812
Approved by: https://github.com/hongxiayang, https://github.com/malfet
xuhancn pushed a commit to xuhancn/pytorch that referenced this pull request Jul 25, 2024
Fixes pytorch#127666.

Other std math functions are replaced with those in the global namespace during hipify. HIP does not claim to support every function in the C++ standard library. std::clamp is not yet supported and we have been relying on the std implementation. For Fedora 40 + gcc 14, a host-side assert is used which is not supported. Work-around this by replacing std::clamp with min and max.  Using #ifndef USE_ROCM to differentiate between CUDA using std::clamp and the ROCm replacement broke Windows builds. The replacement generates the same PTX as std::clamp, so using the replacement unconditionally. The replacement generates the same PTX as std::clamp. See https://godbolt.org/z/Wde9KW3v4 for a sample.

Original patch comes from @lamikr. Modified to improve efficiency.

lamikr/rocm_sdk_builder#37

Co-authored-by: Nikita Shulga <[email protected]>
Pull Request resolved: pytorch#127812
Approved by: https://github.com/hongxiayang, https://github.com/malfet
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.

1 participant