[Tile] Add new tile test mode - #10540
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe build adds Tile test configuration
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
CMakeLists.txt (1)
71-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Add an end-to-end forced-tile configuration test. This contract spans CMake, generated Lit configuration, compiler definitions, and
__tile_global__selection; test both forced and non-forced configurations. As per coding guidelines, changes should be validated with targeted builds and tests.Source: Coding guidelines
libcudacxx/test/libcudacxx/force_include.h (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Keep preprocessor annotations synchronized with their conditions. The comments omit
defined(...)and/or theCCCL_FORCE_TILE_TESTSconjunct, so they describe different conditions than the code. Based on learnings, annotated#else/#endifbranches must repeat the exact condition text from the corresponding#if.Also applies to: 52-64, 138-141
Source: Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 58f934a5-5218-4773-8e1d-6814b3bf114d
📒 Files selected for processing (7)
CMakeLists.txtcmake/CCCLBuildCompilerTargets.cmakelibcudacxx/cmake/LibcudacxxBuildCompilerTargets.cmakelibcudacxx/test/libcudacxx/CMakeLists.txtlibcudacxx/test/libcudacxx/force_include.hlibcudacxx/test/libcudacxx/lit.site.cfg.inlibcudacxx/test/utils/libcudacxx/test/config.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libcudacxx/test/libcudacxx/force_include.h (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Keep preprocessor branch annotations exact.
Line 22 and Lines 142-144 omit parts of their corresponding
#ifconditions, includingdefined(...)andCCCL_FORCE_TILE_TESTS. Update each#else/#endifannotation to repeat the exact condition text.Based on learnings, annotated preprocessor branch comments must repeat the corresponding condition verbatim.
Also applies to: 140-144
Source: Learnings
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e9ed72c-dd3c-4d38-8634-2b8a19fe684c
📒 Files selected for processing (7)
CMakeLists.txtcmake/CCCLBuildCompilerTargets.cmakelibcudacxx/cmake/LibcudacxxBuildCompilerTargets.cmakelibcudacxx/test/libcudacxx/CMakeLists.txtlibcudacxx/test/libcudacxx/force_include.hlibcudacxx/test/libcudacxx/lit.site.cfg.inlibcudacxx/test/utils/libcudacxx/test/config.py
🚧 Files skipped from review as they are similar to previous changes (2)
- libcudacxx/test/libcudacxx/CMakeLists.txt
- libcudacxx/test/utils/libcudacxx/test/config.py
|
pre-commit.ci autofix |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f6b2b5d to
7337bcd
Compare
This comment has been minimized.
This comment has been minimized.
The current testing is not sufficient. We need to test classical SIMT with `--enable-tile` and also `--enable-tile` within a tile program
This comment has been minimized.
This comment has been minimized.
⏱️ CCCL compile-time benchmark comparison: Public headers compile-time benchResult: 2 regression row(s), 4 improvement row(s) above threshold.
Artifacts: reports and traces Direct file processing
🔴 Direct file processing — Regressions
🟢 Direct file processing — Improvements
|
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 3h 33m: Pass: 100%/504 | Total: 5d 02h | Max: 2h 05m | Hits: 89%/1670405See results here. |
The current testing is not sufficient. We need to test classical SIMT with `--enable-tile` and also `--enable-tile` within a tile program
The current testing is not sufficient. We need to test classical SIMT with
--enable-tileand also--enable-tilewithin a tile programFor the latter there is now
CCCL_FORCE_TILE_TESTSwhich changes lit to build a__tile_global__kernelAlso universally add
--enable-tileto the cccl compile options so that we can ensure that all projects are build with it