Repository navigation
cuda::clamp - #5516
cuda::clamp#5516fbusato wants to merge 1 commit into
cuda::clamp#5516Conversation
🟩 CI finished in 1h 16m: Pass: 100%/210 | Total: 1d 17h | Avg: 11m 54s | Max: 51m 47s | Hits: 95%/322859
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| CCCL Packaging | |
| +/- | libcu++ |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| CCCL C Parallel Library | |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| +/- | CCCL Packaging |
| +/- | libcu++ |
| +/- | CUB |
| +/- | Thrust |
| +/- | CUDA Experimental |
| +/- | stdpar |
| +/- | python |
| +/- | CCCL C Parallel Library |
| +/- | Catch2Helper |
🏃 Runner counts (total jobs: 210)
| # | Runner |
|---|---|
| 128 | linux-amd64-cpu16 |
| 23 | windows-amd64-cpu16 |
| 17 | linux-amd64-gpu-l4-latest-1 |
| 12 | linux-arm64-cpu16 |
| 11 | linux-amd64-gpu-rtx2080-latest-1 |
| 10 | linux-amd64-gpu-h100-latest-1 |
| 6 | linux-amd64-gpu-rtxa6000-latest-1 |
| 3 | linux-amd64-gpu-rtx4090-latest-1 |
| { | ||
| if (!_CUDA_VSTD::__cccl_default_is_constant_evaluated()) | ||
| { | ||
| NV_IF_TARGET(NV_IS_DEVICE, (return ::min(::max(__v, __lo), __hi);)) |
There was a problem hiding this comment.
I am wondering how this is an improvement at runtime?
Honestly this is more of an compiler bug if it generates bad code
There was a problem hiding this comment.
@fbusato do you have a reproducer where bad code was produced? I think it would be worthwhile to file this to the compiler team.
There was a problem hiding this comment.
I wrote a reproducer here: https://godbolt.org/z/GTEqznbTa
I was able to achieve the min/max implementation by introducing a temporary:
const auto __tmp = (__v < __lo) ? __lo : __v;
return (__hi < __tmp) ? __hi : __tmp;Unfortunately the min/max instructions are not emited when T is a floating point.
There was a problem hiding this comment.
I wrote a reproducer here: https://godbolt.org/z/WGbrzM9TT
Thanks! This is really unfortunate. I reported this to the team in charge of the compiler optimizer.
There was a problem hiding this comment.
thanks @bernhardmgruber for following up with the compiler team, and thanks @davebayer for further investigating the issue.
Unfortunately the min/max instructions are not emited when T is a floating point.
the problem with floating point is with NaN, but we have a solution for that 🙂 #5034
| { | ||
| NV_IF_TARGET(NV_IS_DEVICE, (return ::min(::max(__v, __lo), __hi);)) | ||
| } | ||
| return __v < __lo ? __lo : __hi < __v ? __hi : __v; |
There was a problem hiding this comment.
Note, we once saw that suboptimal code is generated because the compiler does not see through the nested ternary.
Could you please verify that there is change when writing this as:
| return __v < __lo ? __lo : __hi < __v ? __hi : __v; | |
| if (__v < __lo) | |
| { | |
| return __lo; | |
| } | |
| else if(__hi < __v) | |
| { | |
| return __hi; | |
| } | |
| else | |
| { | |
| return __v; | |
| } |
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef _CUDA___ALGORITHM_CLAMP_H |
There was a problem hiding this comment.
The file needs to be included in <cuda/algorithm>
|
The biggest issue I see with this new API is that users will be confused when to use |
|
@bernhardmgruber entirely agree. Let see if we are able to fix the issue from the compiler side. If yes, I will close the PR |
|
closing in favor of #7325 |
Description
cuda::std::clampis a widely used utility in CCCL and in general code. Unfortunately, it doesn't produce the optimal code for device code for the most common use cases: integer and floating-point types. The main reason is thatcuda::std::clampreturns a reference instead of a value, which is not needed for primitive types.The PR provides
cuda::clampto generate optimal code for integer and floating-point types.Requires
constexprfmin/fmax.