bugfix generic-k code in top-k with softmax #1993
Conversation
|
@t4c1 Thank you for submitting this patch. Just a note on the assertion, it's there more as a warning to users that the generic sort comes with serious performance implications. Due to the control flow associated with it, it introduces somewhat heavy branching and register spilling, resulting in a smaller improvement over the baseline. I'll let @hwu36 chime in on whether we want to get rid of the assertion. |
|
@alihassanijr , maybe leave it there but make the text clearer about the consequences? |
|
@hwu36 leave the assert there with a better message, or remove and make the doc clearer about the consequences? |
|
Leave it there and make the message better so people willing to try can try it. |
examples/61_hopper_gemm_with_topk_and_softmax/61_hopper_gemm_with_topk_and_softmax.cu
Show resolved
Hide resolved
Co-authored-by: Ali Hassani <[email protected]>
…ith_topk_and_softmax.cu Co-authored-by: Ali Hassani <[email protected]>
|
I addressed the review comments some time ago. Is something else required to get this merged? |
|
@alihassanijr anything else here to be done from my side? |
|
@hwu36 to approve the merge. |
Fixes a bug in generic top-k softmax EVT implementation that resulted in wrong results when k != 2 and k != 4.
This allows removal of the static assert requiring k to be either 2 or 4. Comment in example 61 is also fixed to reflect that any k value is now supported.