Repository navigation
cuda-core: safe element casts require AnyBitPattern; bool is DeviceCopy - #1516
Conversation
DeviceBuffer::cast_elem, DeviceBuffer::cast_chunks, and Tensor::reinterpret reinterpreted device bytes as any element type of the right size and alignment, which let a u8 buffer become a DeviceBuffer<bool> or Tensor<bool> holding values other than 0 and 1 (NVIDIA#1506). The casts now require bytemuck::AnyBitPattern of their target, so bool is rejected at compile time, and bool implements DeviceCopy: a typed buffer holds valid values by construction, so bool buffers can be allocated, filled, launched, and read back like any other element type. cuda-core re-exports bytemuck, implements Pod for the DType newtypes, and enables half's bytemuck feature for f16 and bf16. The atomic wrappers in cuda-device are not Copy, so they cannot be AnyBitPattern. DeviceBuffer gains wrap_elem and peel_elem, bounded on bytemuck::TransparentWrapper, to view a scalar buffer as its atomic wrapper and back. cuda-device implements TransparentWrapper for its atomic types and Pod for its vector types, and the examples that cast into atomics use the new methods. Every example lock gains the bytemuck dependency of cuda-device. Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
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. |
|
So, as a first step I think the But we need to address two safety issues that popped up from trying to break the above setup via adversarial examples. 1. The first one is the atomic The intended operation is converting an owned integer buffer into an atomic buffer. But implementing use bytemuck::TransparentWrapper;
// Device code:
fn overwrite(value: &u32) {
let atomic = DeviceAtomicU32::wrap_ref(value);
atomic.store(7, AtomicOrdering::Relaxed);
}The conversion aliases the same memory: The original Proposed fix: remove the atomic pub unsafe trait DeviceRepr<Inner>: Sized {}Its documented contract would require matching layout, valid initialized representations in both directions, and no additional ownership or destruction obligations. It would provide no reference-conversion methods. The buffer’s let atomics = integers.wrap_elem::<DeviceAtomicU32>();
// Run the kernel and complete its accesses.
let integers = atomics.peel_elem::<u32>();
|
|
2. Suppose an allocation still contains old byte // writer and reader are independent GPU streams.
let flags = DeviceBuffer::<bool>::zeroed(&writer, 1)?;
let values = flags.to_host_vec(&reader)?;Both calls are safe Rust. However,
Proposed fix: the simplest repair is to finish initialization before safe // After queuing the zero-fill:
stream.synchronize()?;
Ok(buf)To preserve asynchronous initialization, retain its completion dependency and make every safe consumer honor it, including readback and kernel launches. Asking callers to synchronize in documentation does not repair the safe API. Our GPU probe observed byte I think the initialization-ordering code predates this PR and it only surfaced after admitting |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
@nihalpasham Agreed on both points. Fixed in f42ed4f (and brought the branch up to date with main).
Added compile-fail coverage for integer/float atomic The previously documented Tile UI snapshot-path mismatch remains the only failing case in that suite; |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
Added the two remaining host-side audit fixes in 2788c6c:
Both regressions were checked before and after the fixes. Full cuda-core tests passed (105); selected Tile library/GPU tests passed (168), as did all 9 submission-lifetime tests. Clippy, formatting, and SPDX checks passed. Tile rustdoc builds with 12 existing link warnings; the known unrelated Tile UI snapshot mismatch remains. The remaining duplicate-runtime findings are covered by #1460/#1461 and are excluded from this PR's follow-up scope. |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
…ybitpattern-casts Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com> # Conflicts: # cutile-rs/CHANGELOG.md
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
@nihalpasham Both of your requests are addressed:
The compile-fail and gated-stream regressions pass. The previously reported Tile UI snapshot mismatch is also fixed. The follow-up audit added the host-side argument-contract and pinned-buffer alignment fixes, and the latest changes fix tensor alias accounting during completion-lease creation and retirement. All 63 active CI checks now pass at |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
nihalpasham
left a comment
There was a problem hiding this comment.
Looks good from the cuda-oxide side, approving!
One small non-blocking follow-up I noticed: DeviceBuffer::cast_elem and cast_chunks bound the target A: AnyBitPattern but leave the source T unconstrained (missing T: bytemuck::NoUninit). Since MaybeUninit<T> and padded structs implement DeviceCopy, DeviceBuffer<MaybeUninit<u32>> can currently be cast to DeviceBuffer<u32> in safe code. Fine to tighten in a follow-up PR.
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
Addressed the source-validity follow-up in c1e2df5: both safe casts now require Regression coverage rejects both cast methods for GitHub dismissed the prior approval after this code update. @nihalpasham, please reapprove when you've checked the follow-up. |
|
/ok to test c1e2df5 |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
/ok to test 04d17ef |
|
Head 04d17ef also fixes the refreshed-main math_special lockfile; the full example dependency-policy guard passes. The combined host stack passes the complete CPU/assembler and GPU suites, examples and benchmark smoke tests (details on #1604). The source-validity fix is ready for renewed approval; fresh CI is running. |
Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
|
/ok to test 117205c |
…ries Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com>
Conflicts in the workflow triggers and path filters from NVIDIA#1516, which added cuda-repr/** to the filters and pull-request/[0-9]+ to the push branches of the old workflow-level filters. This branch already triggers on push to main and to pull-request/[0-9]+ and gates its jobs with a changes job instead of a workflow-level paths filter, so keep this branch's workflows and add cuda-repr/** to the changes-job filters of ci.yml, docs.yml and examples-compile.yml. cutile-rs.yml took the same entry from main without a conflict. Signed-off-by: Roman Ivanov <rivanov@nvidia.com>
…nd SIMT (#1603) Tile and SIMT currently expose separate `cuda-core` contexts, streams, launch configurations, and allocation owners. A buffer moving between the models consequently needs raw-pointer adaptation, and the separate runtimes cannot coordinate lifetime or conflicting accesses. This removes `cuda_core::simt` and gives both models the same host handles and allocation owner. `Device` identifies a physical GPU; `Context` owns or borrows the actual CUDA context. Typed buffers, contiguous sliced views, foreign owners, and Tile tensors share allocation identity and access tracking. `Tensor::from_buffer`, `into_buffer`, and `as_buffer` transfer or borrow storage without copying device memory. Ownership transfer rejects live typed aliases, preserving the boolean representation guarantees in #1516. Safe submissions retain allocations and access leases until GPU completion, including after a borrowed launch future is dropped or forgotten. Shared references to device atomics are conservatively tracked as writable. Buffer and stream destructors retire work without waiting on the dropping thread; unknown completion retains resources instead of freeing active memory. The existing SIMT async launcher receives the small retention bridge needed to make this core migration safe before its separate consolidation. Allocation uniqueness counts user-visible byte views independently of completion leases. This removes the retirement window where a decremented lease count still had a live Arc and made a unique buffer appear aliased. The same separation at the Tensor layer comes from #1516; both checks remain required for zero-copy ownership transfer. Nihal's fixes preserve ordinary shared-read permissions on empty and peeled buffers and wait for retirement before checking reclaimed VRAM. Consumer code, generated launchers, examples, templates, documentation, tests, and CI paths use the shared API. Tile numeric storage wrappers also implement `DeviceCopy`; `DType` and `DeviceCopy` remain independent traits. Stack: #1516 merged into `main` as `dfea11698aa63b9a42d9f8181a711d27774d0722`. This PR now targets `main` and contains only the core migration. Merge this before the follow-up #1604, which removes `cuda_async::simt`. Validation: - Full `cuda-core` suite: 111 passed; full `cuda-async` suite: 151 passed. - Selected Tile library/GPU/interop/submission-lifetime suites: 156 passed, including graph replay, offset preservation, alias rejection, and forgotten futures. - SIMT host/macro suites: 261 passed; cargo-oxide: 268 passed. Three additional GPU launch-lifetime regressions pass explicitly with `--ignored`. - Clippy with warnings denied for host workspace, Tile, and affected SIMT packages; strict cuda-core rustdoc; formatting, host-path and SPDX guards. The Tile workspace is also checked with every example feature enabled, including Candle CPU references. - Dependency checks pass for the host, SIMT, backend, device-only fixture, and Tile workspaces. GPU validation used a single RTX 5090. Multi-GPU/NVLink cases self-skip on this machine; this does not claim multi-GPU coverage. Existing deliberately ignored tests remain ignored. Closes #1460. Closes #1504. The global-context re-entry/state-machine follow-up remains tracked separately in #1403. Earlier follow-up validation: all 50 cuda-core unit tests and all nine targeted shared-allocation/retirement GPU tests pass, as does cuda-core Clippy with warnings denied. The new concurrency regression failed 3,308 uniqueness checks on the previous implementation and passes with the fix. Earlier main integration: Merged the updated #1516 prerequisite and migrated the new CuTe host consumers to shared LaunchConfig and the explicit event-flags API. Affected-package Clippy and all 16 CuTe example/model workspace Clippy checks pass. CI passed before this refresh; current-head CI is tracked below. Current update (2026-10-30), head `98d0820debc6f3994cb45c041ac182e174bff364`: Plain completion leases now retain allocation access independently of user-view tokens, so tensor-to-buffer conversion preserves unique ownership without weakening cross-stream exclusion or recorded graph ownership. The combined stack passes 1,427 CPU/assembler tests, 750 GPU tests, 27 supported examples and eight benchmark smoke tests. Initial disk exhaustion was resolved and the affected suites rerun successfully. The expanded atomic example is migrated to owned conversions; all 24 atomic GPU cases also pass on the final shared-runtime head. Fresh CI is running; #1516 still needs renewed approval before this stack can merge into main. Merge status (2026-10-30): current head `ba76442278ad1d6c67070ef5fde3dd63efcddb85` records the accepted #1516 squash merge. The squash-merge tree exactly matches the accepted #1516 head; the ancestry-only refresh leaves this PR's entire file tree unchanged from its reviewed predecessor. After copy-pr-bot deleted the old temporary bases and automatically closed the stacked PRs, they were reopened against `main`; temporary restoration refs were removed. Merge order is #1603 then #1604. Fresh CI is running, and retargeting caused GitHub to require review again. Issue/release tracking: #1606. --------- Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com> Signed-off-by: nihalpasham <nihalp@nvidia.com> Co-authored-by: Melih Elibol <1884921+elibol@users.noreply.github.com> Co-authored-by: nihalpasham <nihalp@nvidia.com>
Tile operations and SIMT launches currently use separate async runtimes and cannot compose through the same operation types. This change removes `cuda_async::simt` and routes both programming models through the shared `DeviceOp`, `DeviceFuture`, execution context, scheduler, errors and launch API. Closes #1461. Prerequisite #1603 is merged into `main` as `6a6372e4cc2e0f142471cf18f13b9072abe87757`. This PR targets `main` and its diff now contains only the async migration. The development branch remains on the contributor's fork. - One `KernelArgument` blanket implementation uses `DeviceCopy`. Generic Tile scalars require `DType + DeviceCopy`; tensor-only type parameters retain their independent `DType` contract. The shared argument arena preserves inline storage, padded values, over-alignment and typed destruction for the explicit unsafe boxed-argument escape hatch. - An inert `AsyncKernelLaunchBuilder` requires unsafe finalization before becoming a schedulable, immutable launch. Generated typed launchers retain their checked boundaries. Tile PDL and SIMT cluster/cooperative launch paths are preserved. Raw async macros marshal slice pointers and lengths as direct scalar values; optional CPU-reference examples use the corresponding scalar bounds. - Launches retain their function/module, allocation owners and access leases before enqueueing work. Early future drop retires owners on completion without waiting on the GPU. Partial errors and panics retain submitted resources; unknown completion deliberately leaks owners and exclusion. Forgotten futures also retain their submissions indefinitely. Outputs can contain ordinary Rust borrows and must be valid to drop during submission, as required by host combinators. - Graphs retain captured resources and reacquire allocation access on every replay. `DeviceBox` transfers to/from shared typed buffers without copying and preserves ordinary read permissions. Existing `DeviceOperation`, `and_then`, `apply` and `arc` spellings delegate to the shared implementations. - `cutile_inter_kernel` now uses the normal shared launch API to update the Tile-produced allocation in place, checks its pointer and allocation identity, and verifies results. Consumers, generated-project templates, documentation, UI fixtures and GPU test entry points are migrated. `AGENTS.md` records that future branches must originate from the user's fork. Validation: shared async tests and doctests; Tile unit, aggregate GPU, submission-lifetime, interop and UI tests; cuda-host/cuda-macros suites; 268 cargo-oxide tests; four explicit SIMT lifetime/concurrency GPU tests. Both `cargo oxide run cutile_inter_kernel` and `cargo oxide run async_mlp` pass. Miri passes all seven argument-storage tests, and Loom passes the completion-handoff model. Host/Tile/affected SIMT clippy checks, formatting, strict async rustdoc, SPDX/API-path guards and Tile dependency checks pass. GPU execution was validated on one RTX 5090. This does not add multi-GPU/NVLink hardware coverage. API migration and custom-operation retention obligations are documented in `cuda-async/README.md`. Additional validation: the full CPU runner passes (1,381 tests; 94 intentionally ignored), all top-level SIMT example workspaces pass Clippy with warnings denied, and all 144 macro unit tests pass. The `host_closure` GPU example also passes all 13 scenarios, covering the corrected async slice argument marshalling. Earlier follow-up validation after incorporating the updated #1516/#1603 stack: all 1,394 CPU tests pass (95 intentionally ignored), as do 138 selected Tile GPU/interop/graph/view/lifetime tests and all four explicit SIMT lifetime/concurrency tests. The original stress reproducer reports no false negatives in 1,000,000 raw-buffer uniqueness checks and no false positives in 100,000 aliased-tensor partition attempts. Nihal's `DeviceBox::from_raw_parts` shared-read fix remains included. Restored the original cutile-rs `unsafe async_on()` contract: the caller independently keeps all submitted resources alive and orders their accesses. On return, including an error, `async_on()` releases runtime owners and access leases without synchronization or background retention. Safe futures retain their nonblocking completion-based cleanup. The existing four-tensor launch benchmark now measures 1.88 µs/launch, down from 4.95 µs before this fix (earlier main baseline: 1.68 µs). No batching or new launch API is needed for this fix. Earlier main integration: Merged the updated #1603 prerequisite, including the CuTe host-consumer migration. The cuda-host, cuda-macros, ctm-rs and cute-rs suites pass (497 tests, 43 intentionally ignored), together with affected-package Clippy and all 16 CuTe example/model workspace Clippy checks. CI passed before this refresh; current-head CI is tracked below. Validation for `b811b7067`: all 133 cuda-async tests pass (14 intentionally ignored), all 11 Tile submission-lifetime tests and all four SIMT lifetime/concurrency GPU tests pass, and Clippy/formatting/rustdoc checks pass. Both new gated-stream regressions fail on the preceding code and pass here. That revision was validated before the current refresh. Current update (2026-10-30), head `95a51d708cc1dd203a922a28624aa087cb7a2176`: Carries the #1516 source-validity and #1603 completion-retention fixes while preserving the reviewed async launch lifetime/access contracts. The combined stack passes 1,427 CPU/assembler tests, 750 GPU tests, 27 supported examples and eight benchmark smoke tests. Initial disk exhaustion was resolved and the affected suites rerun successfully. The expanded atomic example is migrated to owned conversions; all 24 atomic GPU cases also pass on the final shared-runtime head. Fresh CI is running; #1516 still needs renewed approval before this stack can merge into main. Merge status (2026-10-30): current head `21bc9bb783429ef39da7ee9ac63bbd686a3a5895` records accepted #1603 squash ancestry. The squash commit's entire tree exactly matches the prerequisite head already included here (`ba7644227`). The apparent conflicts were duplicate history; this merge changes zero files relative to previously green `ede42672a`. GitHub reports the PR mergeable. Current-head CI has restarted; the previous identical tree passed all 70 applicable checks, including GPU tests (one expected skip). No test assertions were relaxed. The earlier intermittent drop-timing failure remains tracked as test-hardening follow-up #1680. Issue/release tracking: #1606. --------- Signed-off-by: Melih Elibol <1884921+elibol@users.noreply.github.com> Signed-off-by: nihalpasham <nihalp@nvidia.com> Co-authored-by: Melih Elibol <1884921+elibol@users.noreply.github.com> Co-authored-by: nihalpasham <nihalp@nvidia.com>
Closes #1506. First change under #1460; implements the
booldecision recorded there and in #1461 and #1503.Safe element casts previously checked size and alignment but could turn a byte containing
2into a Rustbool. This PR rejects those casts, supports initializedboolbuffers, and closes the host-side initialization, atomic-reference, kernel-argument, and pinned-alignment holes found during review and follow-up auditing.Element validity
DeviceBuffer::cast_elem,cast_chunks,can_cast_chunks, andTensor::reinterpretrequirebytemuck::AnyBitPatternon the target. Casting intoboolis therefore a compile error.The two safe buffer casts also require
T: bytemuck::NoUniniton their source: padding orMaybeUninitbytes must not become initialized destination values.can_cast_chunksremains a layout query. These bounds do not restrictDeviceCopyitself, and bool-to-byte casts remain supported.boolimplementsDeviceCopy: valid values can be copied between host and device, and its all-zero representation is valid.DeviceCopydoes not promise that every bit pattern is valid. It remains independent ofDType.The numeric
DTypenewtypes implementPod, andhalfenables its bytemuck feature, preserving numeric reinterpretation targets. cuda-core re-exportsbytemuckfor downstream marker implementations. Vector types in cuda-device also implementPod, preservingcast_chunksusage.Owned atomic-buffer conversions
wrap_elemandpeel_elemuse the unsafe markerDeviceRepr<Inner>. Its contract requires identical layout, valid initialized representations in both directions, no element destruction, and no additional ownership obligations. It has no methods and grants no reference conversions.The integer and floating-point atomic macros implement
DeviceRepr, notTransparentWrapper. Consequently, safeDeviceAtomicU32::wrap_ref(&u32)and the reversepeel_refconversion are unavailable. Owned scalar buffers can still be wrapped for atomic kernel access and peeled for readback, retaining their allocation, context, and deallocation stream. Kernel accesses must obey the existing borrowing and synchronization contracts.The marker lives in the dependency-free,
no_stdcuda-repr crate and is re-exported by cuda-core and cuda-device. This lets both sides share one trait without making kernel-only crates depend on the host runtime. The dependent lockfiles add this local package without changing registry or git dependencies. CI path filters include the new crate.The atomic examples use
wrap_elem/peel_elem.KernelSliceArgis not restricted toDeviceCopy, since atomic wrappers are legitimate kernel element types that cannot implementCopy.Completed zero-initialization
DeviceBuffer::zeroedqueues the zero-fill and synchronizes its stream before returning. A safe readback or launch on another stream can therefore rely on initialization having completed, including forbool. Empty allocations retain their fast path.This makes nonempty
zeroeda blocking constructor. Asynchronous initialization with automatically retained completion dependencies belongs to the subsequent runtime work; callers are not asked to repair a safe API through manual synchronization.Host-side argument contracts and pinned alignment
KernelInputStoredandKernelOutputStorednow requireunsafe impl. Their contracts cover truthful dtype/layout/specialization metadata, argument ABI, addressable extent, and storage/access retention through completion and graph replay. Previously an entirely safe custom binding could presentTensor<bool>storage asu8output, so a normal byte kernel could write invalid bool values. The input counterpart could similarly mislabel arbitrary byte storage as booleans. The safeKernelInput/KernelOutputconversion traits remain available and must return stored bindings satisfying those contracts. Existing owned, borrowed, view, and mapped bindings retain their behavior.PinnedHostBufferreserves alignment slack, exposes an aligned interior pointer, and retains the original CUDA pointer for deallocation. It also rejects allocation sizes that exceed Rust slice/pointer-arithmetic limits. Empty and zero-sized buffers keep their aligned dangling pointers and allocation-free behavior.Tensor alias accounting
Tensor aliases and lazy copy operations share a separate ownership token. Completion leases retain the storage itself without contributing to that token. Uniqueness therefore stays correct while a lease is being registered or destroyed: a real alias cannot become a mutable partition, and retiring a lease cannot spuriously reject a unique tensor. Access exclusion and completion retention remain enforced.
The acquisition and destruction regressions fail on the previous implementation and pass with this change. A separate test preserves ownership held by an unsubmitted copy operation.
Compatibility and validation
The new
AnyBitPatterntarget andNoUninitsource bounds are breaking changes for safe casts whose element types do not implement those traits. Numeric targets remain supported; atomics use the owned-buffer conversions. Tuple types remain usable as buffer elements but are not reinterpretation targets. Custom stored Tile argument implementations must now explicitly uphold the documented unsafe contract.Earlier validation of the underlying changes:
legacy_atomic_rmw_cas,legacy_atomic_fadd, andhashmapon an RTX 5090; all six Tile tensor reinterpretation tests.The Tile UI snapshot now uses the normalized external-crate path. Both UI harness tests (all 12 compile-fail cases) pass, and the CPU CI script now runs them. The version-only update to the planned v0.5.0 API boundary has landed on main in #1614; host and Tile semver checks pass. SIMT workflow branch filters now include stacked PRs targeting
pull-request/<n>prerequisite mirrors.Follow-up validation for the host-side argument and pinned-buffer fixes:
unsafe implis required; snapshots were reviewed and rerun without overwrite.Earlier host-argument follow-up validation: the complete CPU runner passes (1,408 tests, 98 intentionally ignored), including the Tile UI suites. All 25 targeted GPU buffer and pinned-host-buffer tests pass, including the cross-stream initialization regression. The branch includes the #1602 debug-source fixes and the same version metadata as #1614.
Current validation (2026-10-30): head
117205c1277a9e6d422df20499342690b412bc16includes main through76ac10216. The new regression fixture compiled before the source-validity fix, then all five trybuild cases passed afterward without snapshot regeneration. The new-mainmath_specialexample lockfile includes the host dependencies at the versions used by other examples; the full example dependency-policy guard passes. Fresh CI is running. GitHub dismissed the previous approval after the source changes, and renewed review has been requested.The expanded atomics example from current main uses owned
wrap_elem/peel_elemconversions, with explicit checked ownership transfer for its standard-library atomic case. Strict example Clippy passes, and all 24 runtime atomic cases pass on both this PR and the combined shared-host stack on RTX 5090 / CUDA 13.3.Merge-readiness refresh (2026-10-30): head
c5ea583c5b5f06b424a9ab0c48174c771e99ff52includes main through the merged #1480 (b025a8b31). The only manual conflict resolution preserved both changelog entries; source and tests from #1480 merged unchanged. The combined release candidate has an identical source/test tree to previously validatedc6c5a860a(only the changelog and merge ancestry changed). New-head CI is running. Merge #1516 into main first, then retarget #1603 to main and merge it, then retarget #1604 to main and merge it. Do not merge the children into the temporarypull-request/*bases. Issue/release inclusion tracking: #1606.