Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
@coderabbitai full review Please review this PR against its stacked base, focusing on correctness, Expanded API docstrings, user guides, and runnable documentation examples are |
|
important: I will review the PR against its stacked base. I will focus on correctness, compiler integration, synchronization, input/output contracts, and regressions. I will treat the documentation and launch-bounds work in ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved API read-only handling and packaging/export validation gaps remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds portable and Numba-CUDA-MLIR Merge Sort APIs for block and warp groups, including key/pair sorting, partial tiles, scratch storage, and custom comparisons.
Changes:
- Adds backend-neutral semantics, planning, and CUB providers.
- Integrates lowering, exports, typing, and packaging.
- Adds contract, compiler, runtime, GPU, and packaging tests.
Review findings:
- Moderate (2 votes): Allow read-only thread data in the Merge Sort API checks.
- Moderate (1 vote): Add Merge Sort files and headers to the independent wheel validator.
- Nit (1 vote): Require both new exports in the installed-wheel
coop.__all__probe.
| File | Reviewed change |
|---|---|
python/cuda_coop/tests/packaging/typing/portable_consumer.py |
Portable typing coverage |
python/cuda_coop/tests/packaging/typing/numba_consumer.py |
Qualified typing coverage |
python/cuda_coop/tests/packaging/typing/invalid_consumer.py |
Invalid API typing cases |
python/cuda_coop/tests/packaging/test_wheel.py |
Wheel payload coverage |
python/cuda_coop/tests/packaging/test_metadata.py |
Package metadata coverage |
python/cuda_coop/tests/contracts/core/test_core_merge_sort.py |
Core planning contracts |
python/cuda_coop/tests/backends/numba_mlir/unit/test_public_foundation.py |
Backend surface checks |
python/cuda_coop/tests/backends/numba_mlir/unit/test_merge_sort_planning.py |
Planning diagnostics |
python/cuda_coop/tests/backends/numba_mlir/runtime/test_merge_sort.py |
GPU runtime tests |
python/cuda_coop/tests/backends/numba_mlir/compile/test_merge_sort_compile.py |
Compilation tests |
python/cuda_coop/cuda/coop/numba_mlir/_lowering/_merge_sort.py |
Provider generation |
python/cuda_coop/cuda/coop/numba_mlir/_group_merge_sort.pyi |
Qualified type stubs |
python/cuda_coop/cuda/coop/numba_mlir/_group_merge_sort.py |
Qualified entry points |
python/cuda_coop/cuda/coop/numba_mlir/_compiler/_rewrite_merge_sort.py |
Payload inference |
python/cuda_coop/cuda/coop/numba_mlir/_compiler/_group_merge_sort.py |
Backend planning and lowering |
python/cuda_coop/cuda/coop/numba_mlir/__init__.pyi |
Qualified exports |
python/cuda_coop/cuda/coop/numba_mlir/__init__.py |
Lazy qualified exports |
python/cuda_coop/cuda/coop/_core/warp/merge_sort.py |
Warp semantics |
python/cuda_coop/cuda/coop/_core/warp/__init__.py |
Warp exports |
python/cuda_coop/cuda/coop/_core/group/merge_sort.py |
Group planning |
python/cuda_coop/cuda/coop/_core/group/__init__.py |
Group exports |
python/cuda_coop/cuda/coop/_core/block/merge_sort.py |
Block semantics |
python/cuda_coop/cuda/coop/_core/block/__init__.py |
Block exports |
python/cuda_coop/cuda/coop/_core/api/merge_sort.pyi |
Portable type stubs |
python/cuda_coop/cuda/coop/_core/api/merge_sort.py |
Portable entry points |
python/cuda_coop/cuda/coop/_core/api/__init__.pyi |
API stub exports |
python/cuda_coop/cuda/coop/_core/api/__init__.py |
API runtime exports |
python/cuda_coop/cuda/coop/_core/__init__.py |
Core exports |
python/cuda_coop/cuda/coop/__init__.pyi |
Top-level type exports |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/cccl/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughAdded cooperative CUDA merge-sort APIs for keys and key/value pairs. The implementation supports block and warp groups, full and partial tiles, custom comparators, typed payloads, CUB lowering, and Numba-MLIR compilation. ChangesMerge Sort
Priority: ➖ Normal Change: Feature Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established for the added merge-sort API surface. Comment |
Add input-preserving key and key/value Merge Sort operations with blocked results, partial tiles, and independent storage for logical warps. Qualified Numba calls also accept local arrays and stateless comparators. Validate runtime tile counts before narrowing them to the CUB ABI and retain inferred payload dtypes across chained operations. Signed-off-by: Trent Nelson <trent@trent.me>
ab4123b to
e574f2e
Compare
|
@coderabbitai review The read-only input validation finding is fixed in the owning Merge Sort commit, with 12 host regressions and readable-payload typing coverage. Please review the current head incrementally. Expanded documentation remains in #11320, followed by launch-bounds inference in #11392. Focus on credible correctness or contract findings within this PR. |
|
✅ Action performedReview finished.
|
🥳 CI Workflow Results🟩 Finished in 28m 21s: Pass: 100%/8 | Total: 47m 12s | Max: 11m 46sSee results here. |

Why this is needed
Kernels need to order the values already held by a cooperative group and
keep associated data, such as original indices, attached to each key.
This adds common and Numba-CUDA-MLIR Merge Sort APIs:
Behavior
merge_sort_keysandmerge_sort_pairsfor Block, physical Warp,and logical Warp groups. Inputs are preserved and results use blocked
layout. Equal-key order is unspecified. The common API accepts read-only
ThreadData inputs.
and independent scratch for each physical or logical Warp group.
TempStorage; the qualified namespace also acceptsfixed local arrays and custom comparison callbacks.
hooks, with public stubs and host/compiler/GPU coverage.
Stacked on #11217. User guides, expanded API documentation, and executable
documentation examples are added in the downstream docs PR #11320.
Validation
Fresh installed wheels passed 685 host/packaging checks on each of
Python 3.10 and 3.14,
56 focused unit/compiler checks with GPUs hidden, and 57 GPU tests
on CUDA 13.3 with Numba-CUDA-MLIR 0.5.2. Source, wheel, installed module,
and bundled-header bytes were compared. Wheel validation and changed-file
pre-commit passed.
Remote CI is separate from these local results.