[STF] Add exec places from externally-owned CUDA contexts#9779
Conversation
Allow an exec_place to wrap a CUcontext created outside CUDASTF, so SM partitioning can be owned by one library (e.g. cuda.core in Python) and shared across frameworks: the same green context can back a CUDASTF exec place and, for instance, a Warp device mapped from the same handle. - cudax: new exec_place_cuda_ctx_impl generalizing the green-context place; exec_place::cuda_context(ctx, devid = -1) factory (devid derived via cuCtxGetDevice when omitted). exec_place::green_ctx now builds the same impl, converting the CUgreenCtx once at construction instead of on every activate(). Identity (hash/cmp) is keyed on the converted CUcontext, which is stable per green context, so places built from a green_ctx_view and from the converted context compare equal. - C API: stf_exec_place_cuda_context(CUcontext, int dev_id). - Header unittests for equality, device-ordinal derivation, the activate/deactivate round trip, and cross-route equality with the green-context place; C API test running a task on a place built from the primary context. Places are non-owning: the caller must keep the context alive while the place is in use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/ok to test 4e54fc8 |
A null context with an explicit dev_id would construct a place that only misbehaves at activation time (cuCtxSetCurrent(nullptr) succeeds and unbinds the current context). Reject it eagerly instead: - exec_place::cuda_context throws std::invalid_argument (works in release builds, matching the logic_error throws elsewhere in places). - The C API asserts in debug builds; in release the exception is mapped to a null handle with a stderr trace by stf_try_allocate, like every other failure on this surface. - Header unittest for the rejection; the C API test already covers the null-handle return. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/ok to test 94c2461 |
@caugonnet, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
/ok to test 94d2461 |
This comment has been minimized.
This comment has been minimized.
|
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:
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
WalkthroughChangesAdds non-owning CUDA CUDA context place core
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: 4
🧹 Nitpick comments (1)
c/experimental/stf/test/test_places.cpp (1)
46-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Rename
Nto a snake_case descriptive name and declare itconstexprwith uniform initialization, for exampleconstexpr size_t element_count{1024}. Also make handles and pointers that are never reassignedconst, includingplace_derived,place,ctx,lX,t,stream, and thedXpointer itself.As per coding guidelines, “All variables that are not modified must be declared
const” and compile-time values must beconstexpr.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 02ec1c61-707d-4bda-9cf3-c13c30836dd0
📒 Files selected for processing (9)
c/experimental/stf/include/cccl/c/experimental/stf/stf.hc/experimental/stf/src/stf.cuc/experimental/stf/test/test_places.cppcudax/include/cuda/experimental/__places/exec/cuda_context.cuhcudax/include/cuda/experimental/__places/exec/green_context.cuhcudax/include/cuda/experimental/__places/places.cuhcudax/include/cuda/experimental/places.cuhcudax/include/cuda/experimental/stf.cuhcudax/test/places/CMakeLists.txt
Use the existing context guard so device lookup restores the caller's context when querying the device fails.
Keep context identity and device affinity consistent, preserve C API error translation, and use portable context ordering.
|
Also addressed the review-summary style suggestion in 55a49cc: the element count is now constexpr with a descriptive name, and unchanged handles/pointers are const-qualified. |
|
/ok to test ea1e817 |
List the context factory with the other execution-place factories and clarify its non-owning lifetime contract.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 120dcbc3-9d6d-4640-8844-577caaa69e82
📒 Files selected for processing (1)
docs/cudax/places.rst
| - ``exec_place::cuda_context(ctx, devid)`` -- an externally-owned CUDA driver | ||
| context; the device ordinal is derived from the context when ``devid`` is | ||
| omitted | ||
|
|
||
| When an execution place is activated, it sets the appropriate CUDA context | ||
| (e.g. calls ``cudaSetDevice``). Each execution place also has an *affine* | ||
| data place: the memory location naturally associated with it. For a device | ||
| execution place the affine data place is the device's global memory; for | ||
| the host it is pinned host memory (RAM). | ||
|
|
||
| A CUDA-context execution place is non-owning. The caller must keep the | ||
| ``CUcontext`` alive while the place and any streams obtained from it are in use. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
important: Document CUDA-context activation and stream ownership consistently.
The new factory is listed here, but the later “Behavior by execution place type” and stream-management sections omit CUDA-context places. Add that they activate via the driver context and use their own embedded stream pool, so readers do not infer device-place registry semantics.
As per path instructions, documentation changes must focus on technical accuracy, buildable examples, API/version consistency, and migration impact.
Source: Path instructions
Keep only the configuration header before the system-header pragma block, matching cudax header conventions.
|
/ok to test a669f4d |
🥳 CI Workflow Results🟩 Finished in 1h 05m: Pass: 100%/61 | Total: 1d 02h | Max: 1h 05m | Hits: 15%/178068See results here. |
3 participants
Allow an exec_place to wrap a CUcontext created outside CUDASTF, so SM partitioning can be owned by one library (e.g. cuda.core in Python) and shared across frameworks: the same green context can back a CUDASTF exec place and, for instance, a Warp device mapped from the same handle.
Places are non-owning: the caller must keep the context alive while the place is in use.
Description
closes
Checklist