smoke test to verify GPU memory allocation/deallocation#9195
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
important: WalkthroughAdds a fixed ChangesDevice Memory Smoke Test
Possibly related PRs
Suggested reviewers
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c99fe5ab-3d24-479e-9214-fd813e9002a9
📒 Files selected for processing (1)
test/cuda_smoke/cuda_runtime_smoke.cu
|
@bernhardmgruber @alliepiper added a smoke test for GPU memory allocation |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/cuda_smoke/cuda_runtime_smoke.cu (2)
85-109: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winimportant: Qualify CUDA runtime free-function calls from global scope.
In this block, use
::cudaMalloc,::cudaMemcpy,::cudaGetLastError,::cudaDeviceSynchronize, and::cudaFreeto match the repository rule for free-function qualification.As per coding guidelines: “All calls to free functions must be fully qualified starting from the global namespace, e.g., ::cuda::ceil_div, including calls to functions in the same namespace”.
78-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winimportant: Make this test self-contained for device readiness.
Line 85 allocates immediately, but this case does not verify device availability or set a device locally. Add a local
cudaGetDeviceCount/SKIP+cudaSetDevice(0)in this test so it is independent of other test ordering.#!/bin/bash # Verify whether this test case has local device readiness guards. rg -n -C3 'TEST_CASE\("cudaMalloc/cudaFree round-trip works"' test/cuda_smoke/cuda_runtime_smoke.cu rg -n -C2 'cudaGetDeviceCount|cudaSetDevice|SKIP\(' test/cuda_smoke/cuda_runtime_smoke.cu
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3cca54e7-d47f-4bf2-aada-49eb9a5f6f38
📒 Files selected for processing (1)
test/cuda_smoke/cuda_runtime_smoke.cu
|
/ok to test 1691f63 |
This comment has been minimized.
This comment has been minimized.
|
|
||
| // smoke test for GPU memory allocation/deallocation | ||
|
|
||
| TEST_CASE("cudaMalloc/cudaFree round-trip works", "[cuda_smoke][device_memory]") |
There was a problem hiding this comment.
We should also add a test for async memory allocations from pools
There was a problem hiding this comment.
Yes, in a separate PR!
|
/ok to test fd2a4cc |
This comment has been minimized.
This comment has been minimized.
|
/ok to test b79a156 |
This comment has been minimized.
This comment has been minimized.
🥳 CI Workflow Results🟩 Finished in 9h 34m: Pass: 100%/501 | Total: 3d 10h | Max: 49m 36s | Hits: 99%/635766See results here. |
|
@charan-003 thank you for the contribution! This is great! |
3 participants
Following up on #8859.
Add test for GPU memory allocation/deallocation.