[Issue]: calling function min with (int, unsigned int) causing ambiguity
Problem Description
when we try to fix a compiler bug about function min by https://github.com/llvm/llvm-project/pull/82956 we encountered build failure in hipCUB. Then we found the line that causes the compilation failure is
https://github.com/ROCm/hipCUB/blob/develop/hipcub/include/hipcub/backend/rocprim/device/device_spmv.hpp#L142
Basically this line is calling function min defined in clang header with (int, unsigned int). Previously the compiler only defined min(int, int). As we introduce min(unsigned, unsigned), min(long, long), min(unsigned long, unsigned long). The compiler could not choose among these candidates since there is no exact match.
We do not want to define min(int, unsigned int) because that would do implicit conversion from int to unsigned int then compare. This may cause unexpected results, e.g. min(-1, 1U) will return 1U. We want users to do explicit cast to indicate their intention. We think it is better than do silent implicit conversion.
Therefore we would like hipCUB to the line https://github.com/ROCm/hipCUB/blob/develop/hipcub/include/hipcub/backend/rocprim/device/device_spmv.hpp#L142
by modifying it to be
size_t block_size = min(num_cols, static_cast<int>(DeviceSpmv::CsrMVKernel_MaxThreads));
which will keep its original behavior, or otherwise as it suits.
Thanks.
Operating System
Ubuntu 22.04
CPU
any
GPU
AMD Radeon RX 7900 XTX
ROCm Version
ROCm 6.0.0
ROCm Component
hipCUB
Steps to Reproduce
No response
(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support
No response
Additional Information
No response
Hi @yxsamliu, thanks for the information. I've created a pull request with the requested change here.
it seems the fix was not working. see comment https://github.com/ROCm/hipCUB/pull/344/files#r1621185464
The fix needs to make the two arguments having the same type. Currently, only min(int, int) is defined.
This issue has been migrated to: https://github.com/ROCm/rocm-libraries/issues/93
Closing the issue in this repo. Please refer to the migrated issue for updates.