Repository navigation
Improve our WarpReduce implementation - #6814
Conversation
|
Note to reviewers, its simplest to go by commit |
| template <class T> | ||
| inline constexpr bool reduce_max_exists<T, ::cuda::maximum<T>, decltype(__reduce_max_sync(0xFFFFFFFF, T{}))> = | ||
| (::cuda::std::is_same_v<int, T> || ::cuda::std::is_same_v<unsigned int, T>); |
There was a problem hiding this comment.
Note to self:
I believe we can relax that constrain to is_integral<T> && sizeof(T) <= sizeof(int) and then in the optimization pass below cast to using promoted_t = ::cuda::std::conditional_t<::cuda::std::is_signed_v<T>, int, unsigned>;
Then casting back to the original T should be fine. That said, I believe this to be a rather invasiev change so I leave that for a followup
| _CCCL_DEVICE _CCCL_FORCEINLINE _Tp ReduceStep( | ||
| _Tp input, ReductionOp reduction_op, int last_lane, int offset, ::cuda::std::true_type /*is_small_unsigned*/) | ||
| { | ||
| return ReduceStep(input, reduction_op, last_lane, offset); |
There was a problem hiding this comment.
"Dispatch to specialized implementations"
This comment has been minimized.
This comment has been minimized.
|
Per https://docs.nvidia.com/cuda/cuda-c-programming-guide/#warp-reduce-synopsis |
1146770 to
e620a4a
Compare
That is true, but we do not have a function object that does logical |
| * Up-offset to pull from | ||
| */ | ||
| _CCCL_DEVICE _CCCL_FORCEINLINE double | ||
| ReduceStep(double input, ::cuda::std::plus<> /*reduction_op*/, int last_lane, int offset) |
There was a problem hiding this comment.
Note to reviewer, I believe we can also considerably clean those up, but that would make the PR quite unwieldly
This comment has been minimized.
This comment has been minimized.
I think we should map |
bernhardmgruber
left a comment
bernhardmgruber
left a comment
There was a problem hiding this comment.
This LGTM, but we absolutely must have a SASS diff of a fitting test/benchmark before/after SM80 to verify this. I can take that.
|
Unfortunately, this is creating a lot more instructions on SM90 (and 100) for test For example: This also affects |
4167ad8 to
9479fc0
Compare
|
Thanks @bernhardmgruber for checking, I found that I missed a |
This comment has been minimized.
This comment has been minimized.
| _CCCL_DEVICE _CCCL_FORCEINLINE T Reduce(T input, int valid_items, ReductionOp reduction_op) | ||
| { | ||
| return ReduceImpl(bool_constant_v<ALL_LANES_VALID>, input, valid_items, reduction_op); | ||
| if constexpr (ALL_LANES_VALID) |
There was a problem hiding this comment.
Question: Why do we only do the __reduce_meow_sync optimization when having a full lane, it explicitly takes the valid lanes as arguments?
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 10h 16m: Pass: 100%/93 | Total: 5d 05h | Max: 5h 26m | Hits: 63%/96454See results here. |
For the warpspeed scan PR we need a common implementation of
__reduce_or_syncI wanted to add a specialization for that to
WarpReduceand found some ... historical artifacts.This cleanes those up and centralizes the optimizations into a single place. Finally it adds the specializations for
__reduce_and_syncand__reduce_or_syncThere is no standard functor for
logical_xorthough :(