c.parallel: device wrappers as code, not format strings - #3439
Conversation
🟩 CI finished in 51m 21s: Pass: 100%/3 | Total: 1h 03m | Avg: 21m 13s | Max: 50m 58s
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-v100-latest-1 |
| 1 | linux-amd64-cpu16 |
shwina
left a comment
There was a problem hiding this comment.
I can't provide feedback on the implementation, but it's definitely a quality-of-life improvement to be able to define the kernels in code rather than as strings.
🟩 CI finished in 28m 38s: Pass: 100%/3 | Total: 32m 36s | Avg: 10m 52s | Max: 25m 09s
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
gevtushenko
left a comment
There was a problem hiding this comment.
That's quite a chain of indirections
🟩 CI finished in 43m 37s: Pass: 100%/3 | Total: 55m 32s | Avg: 18m 30s | Max: 40m 15s | Hits: 98%/310
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
|
@robertmaynard I believe I addressed your comments. |
🟩 CI finished in 1h 00m: Pass: 100%/3 | Total: 1h 17m | Avg: 25m 58s | Max: 58m 36s | Hits: 98%/310
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
|
cc @NVIDIA/cccl-python-codeowners for vis |
🟩 CI finished in 1h 02m: Pass: 100%/3 | Total: 1h 16m | Avg: 25m 24s | Max: 1h 01m | Hits: 98%/310
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
| struct template_id | ||
| {}; | ||
|
|
||
| struct specialization |
There was a problem hiding this comment.
Could this struct be renamed to specialization_t or specialization_st to make it easier to realize that it describes a type. Ease mental burden of parsing the code.
rwgk
left a comment
There was a problem hiding this comment.
I found one trivial grammar error in the Readme.
The code complexity is amazing.
|
@robertmaynard can you please re-review? |
🟩 CI finished in 1h 31m: Pass: 100%/3 | Total: 1h 51m | Avg: 37m 00s | Max: 1h 30m | Hits: 96%/330
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 3)
| # | Runner |
|---|---|
| 2 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
|
@NVIDIA/cccl-cmake-codeowners ping for a re-review |
alliepiper
left a comment
There was a problem hiding this comment.
Only reviewed CMake changes.
The features used here aren't an area of CMake I'm very familiar with, but based on @robertmaynard's comments and my reading of the relevant docs this LGTM.
Approving.
🟩 CI finished in 41m 18s: Pass: 100%/5 | Total: 1h 07m | Avg: 13m 32s | Max: 36m 37s | Hits: 98%/326
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 5)
| # | Runner |
|---|---|
| 4 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
🟩 CI finished in 40m 19s: Pass: 100%/5 | Total: 1h 04m | Avg: 12m 56s | Max: 32m 28s | Hits: 55%/326
|
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| python | |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
Modifications in project or dependencies?
| Project | |
|---|---|
| CCCL Infrastructure | |
| libcu++ | |
| CUB | |
| Thrust | |
| CUDA Experimental | |
| stdpar | |
| +/- | python |
| +/- | CCCL C Parallel Library |
| Catch2Helper |
🏃 Runner counts (total jobs: 5)
| # | Runner |
|---|---|
| 4 | linux-amd64-gpu-rtx2080-latest-1 |
| 1 | linux-amd64-cpu16 |
Description
So far, even with the recent refactoring, the code for the device wrappers lived in format strings. This has a number of downsides, and this PR aims to resolve those.
The wrappers are now actual C++ templates, can be edited with the help of LSP and code formatters, and attempts to obtain their type names will now do some rudimentary type checking in the host code. This is not perfect, as things like
cccl_op_tcarry basically no type information, but is an improvement over the status quo.Also added is a CMake target that bundles all the wrappers into a file that contains their contents, preprocessed to a degree, wrapped into a string, so all the code can be simply added into the NVRTC TU wherever needed.
Resolves #2525
Resolves #2665
Resolves #2918
Checklist