Fix radix sort regression - #9816
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesRadix sort kernel parameters
Assessment against linked issues
Suggested reviewers: Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cub/cub/device/dispatch/kernels/kernel_radix_sort.cuh (1)
86-92: 🚀 Performance & Scalability | 🔵 Trivialsuggestion: Validate this code-generation change on both
_CCCL_HAS_GRID_CONSTANT()branches before merging. The supplied configuration makes_CCCL_GRID_CONSTANTconditional, and CUDA documents that__grid_constant__changes kernel-parameter handling; run targeted radix-sort tests and benchmarks with supported and fallback compiler/architecture configurations, and verify any forward declarations or explicit instantiations remain consistent. (docs.nvidia.com)As per coding guidelines and the
cub/**/*path instructions, validate performance-sensitive changes with targeted builds/tests.Also applies to: 147-147, 245-254, 338-345, 454-459, 477-477, 508-519, 564-564
Sources: Coding guidelines, Path instructions, MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8f9a93fd-04fc-49b6-8664-c70362076849
📒 Files selected for processing (1)
cub/cub/device/dispatch/kernels/kernel_radix_sort.cuh
This comment has been minimized.
This comment has been minimized.
|
@bernhardmgruber in your "impact of this pr" table it shows some good speedups and minor regressions in the scale of no more than 3%, yet the regression report says they see 40% regression which is not justified by just removing the |
|
I don't understand why nvtx3 dependency fails for this pr and makes CI red, did sth leak in main? |
|
/ok to test e1c903a |
NVTX3 had an upstream issue, that should be resolved now. |
This comment has been minimized.
This comment has been minimized.
😬 CI Workflow Results🟥 Finished in 6h 04m: Pass: 99%/287 | Total: 10d 11h | Max: 2h 21m | Hits: 36%/821310See results here. |
|
Closed in favor of: #9858 |
Those were introduced in #6642.
Fixes: #9768
CCCL 3.3 vs 3.4 with this change:
Impact of this PR: