Add .devcontainer Docker for users#5
Open
jamesETsmith wants to merge 8 commits into
Open
Conversation
…pip and address some of the issues with the build system that expected things in /opt/rocm
…tages for installing prereqs and building warp to speed up iterative development on the image
…n get it to match the rockprof compute
Collaborator
Author
|
@rtmadduri I'm seeing now that the installation of A warning about this is buried in the rock docs here |
Collaborator
|
@jamesETsmith, I think these are good changes and I have been thinking about it and I realize we should probably have 2 different Dockerfiles. This one was designed for CI and we should keep it as lightweight as possible and installing only those dependencies that are absolutely necessary for unit tests to run. (So we probably dont need rocprof in the CI container) I think we should keep the versioning changes and the parameterization of the ROCk URL and revert the rest. Those changes can go into a separate Dockerfile as part of the |
…nd installing rocm via uv pip (not working 100% yet)
rtmadduri
changed the title
rtmadduri
pushed a commit
to rtmadduri/warp
that referenced
this pull request
Jul 6, 2026
* Fix duplicate changelog entries after rebase (NVIDIAGH-1236) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Fix comment inaccuracies in tile load/store paths (NVIDIAGH-1236) - copy_to_global coalesced comment: "loads" → "stores" - tile_shape_f4 comment: describe actual multiply-then-divide operation - carry_delta comment: clarify "tile spans full extent" vs "array contiguous" - tiles.rst: add missing in-bounds requirement for coalesced path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Fix copyright year to 2026 in new benchmark files (NVIDIAGH-1236) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Use Layout::Dense on load-side vectorized path for consistency (NVIDIAGH-1236) Make copy_from_global guards identical to copy_to_global: replace contiguous_shared with Layout::Dense in both the vectorized path guard and the aligned=True static_assert. Removes all contiguous_shared references from tile.h. Not currently reachable (tile_load creates fresh dense tiles), but eliminates the inconsistency that reviewers keep flagging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Use Layout::Dense on load-side coalesced path for consistency (NVIDIAGH-1236) The load-side coalesced guard only checked last-dim stride == 1, while the store side already uses Layout::Dense. Not currently reachable (tile_load creates fresh dense tiles), but aligning the guards prevents future regressions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Fix address alignment check to be unconditional in release builds (NVIDIAGH-1236) The previous commit replaced __trap() with assert(), but assert is compiled out with NDEBUG. Restore __trap() on GPU for unconditional safety in release builds. Keep assert() before it for diagnostic messages in debug builds. CPU path uses assert only (the vectorized call sites are GPU-only). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Address MR 2185 review: Dense guard, aligned fixes, docs (NVIDIAGH-1236) - Add Layout::Dense to tile_layout_strided_t (B3): replaces contiguous_shared && Unique on store-side fast paths. Dense checks full row-major packing, catching non-dense tile_view of larger parent. - Restore shared-pointer alignment asserts on both load/store (B5). - Warn on aligned=True for 1D shared tiles (B4). - Replace __trap() with assert() for misaligned address diagnostic (S3). - Remove NDEBUG guard discussion — kept as-is per API contract (S2). - Fix docstrings: address traps are unconditional, bounds/contiguity are debug-only (S1). Fix CPU alignment claim (S6), N-D offset formula (S7). Revert stray copy() refactor (S8). Fix warp-divergence comment (S5). - Add test_tile_view_non_dense_store regression test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Skip vectorized/coalesced store for broadcast tiles (NVIDIAGH-1236) Broadcast tiles have zero strides on repeated dimensions, so their logical Layout::Size exceeds the physical shared memory. The flat float4 and byte-copy store paths assumed contiguous backing and would overread shared memory. Gate both paths on Layout::Unique (which already checks for zero strides). The scalar fallback correctly handles broadcast via index_from_coord. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Rename can_vectorize/can_coalesce to use tile_ prefix Adds tile_ prefix for consistency with other functions in tile.h, per MR !2185 review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * formatting * Add coalesced byte-copy tests, fix aligned=True silent fallback - Add test_tile_shared_coalesced_mat33 and test_tile_shared_coalesced_mat44 to verify the coalesced byte-copy load/store path for large element types - Hoist aligned=True validation above the size_aligned gate in both copy_from_global and copy_to_global. Previously, aligned=True with a non-float4-aligned tile shape silently fell through to the scalar path. Now produces a compile-time static_assert error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Address review feedback: docs, static_assert, comment fix - Add static_assert(sizeof(T) % sizeof(float) == 0) in coalesced byte-copy load and store paths for self-documenting per-element alignment - Document the three-tier shared tile load cascade (vectorized float4, coalesced byte-copy, scalar fallback) in tiles.rst - Fix comment: "branch prediction" → "warp divergence" (GPU concept) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Address MR review: remove dead code, fix docs, add test coverage - Remove unused 1D tile_shape_f4 specialization (dead code, vectorized path gates on N >= 2) - Use reinterpret_cast<uint64_t> instead of C-style (uint64_t) cast - Document Aligned template parameter contract on tile_global_t - Remove redundant first iteration from can_vectorize() check 4 (already covered by check 1) - Add test_tile_load_3d_shared_vectorized (3D float4 indexing loop) - Add test_tile_load_coalesced_mat33 (coalesced byte-copy correctness) - Force aligned=False in dispatch when storage is register - Expand aligned docstring with full contract on tile_load and tile_store - Add 2D+ gate and outer-stride float4-alignment to tiles.rst docs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Add outer stride float4-alignment check to can_vectorize() The vectorized path computes row offsets as strides[d] / sizeof(float4). When the array's last dimension is not float4-aligned (e.g., array (10,6) of float32, stride=24, 24 % 16 != 0), this integer division truncates and produces wrong global memory indices. Add check ROCm#5 verifying strides[d] % sizeof(float4) == 0 for all outer dimensions. This restores the semantic of the old 2D aligned_stride check that was dropped during the N-D generalization. Also add the same assertion to validate_aligned_runtime for debug builds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Enable vec3 vectorization, fix docs, add regression tests Remove overly conservative elem_packs_cleanly guard that prevented vec3 tiles from using the vectorized float4 path. The size_aligned check alone is sufficient: it ensures last_dim * sizeof(T) divides evenly into float4 chunks (e.g., 8 vec3s = 96 bytes = 6 float4s). Add test_tile_load_vec3_vectorized for 2D vec3 shared tile correctness. Add test_tile_load_partial_outer_dim regression test for tiles where the array outer dim is not a multiple of the tile dim (was incorrectly vectorized on main). Fix CHANGELOG: remove "1D" vectorized claim (gated on N >= 2), add coalesced byte-copy mention. Update aligned parameter docstring to note it only affects 2D+ shared tiles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Extract can_coalesce(), document path cascade, add partial tile test Add can_coalesce() free function with labeled checks explaining each requirement (dense packing, bounds, inner-dim span). Replace inline checks in copy_from_global/copy_to_global. Add longform comment to copy_from_global documenting the three-tier path cascade: vectorized float4 → coalesced byte-copy → scalar fallback. Add test_tile_load_partial_outer_dim regression test for float4-aligned tiles where the array outer dimension is not a multiple of the tile dimension. On main, partial tiles incorrectly hit the vectorized path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Extract can_coalesce() and document tile load/store path cascade Add can_coalesce() free function with labeled checks explaining each requirement (dense packing, bounds, inner-dim span). Replace inline checks in copy_from_global/copy_to_global. Add longform comment to copy_from_global documenting the three-tier path cascade: vectorized float4 → coalesced byte-copy → scalar fallback, with conditions for each. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Add dedicated ASV benchmarks for each tile load/store code path Add four new benchmark files, each targeting one specific code path: - load_store_register.py: Register tiles across 1D/2D/3D and dtypes - load_store_vectorized.py: Shared vectorized float4 (2D/3D, aligned) - load_store_coalesced.py: Shared coalesced byte-copy (1D, large types) - load_store_scalar.py: Shared scalar fallback (non-aligned tile sizes) Rename vectorized_load.py → load_store_vectorized.py for consistency. Existing benchmarks kept for now; removal deferred to follow-up MR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Add coalesced byte-copy path for shared tile load/store For large element types (mat33, mat44, mat66), the scalar tile load/store path issues per-element loads with large strides between consecutive threads in a warp, causing poor memory coalescing. A mat66 tile (144 bytes/element) has threads accessing addresses 144 bytes apart — spanning 36 cache lines per warp instead of 1. Add a coalesced byte-copy path that reinterprets tile data as flat float arrays and copies with thread-striped access. Consecutive threads access consecutive 4-byte words, achieving perfect coalescing regardless of element size. The path activates when the tile's raw bytes are contiguous in global memory (densely packed array, in-bounds, tile spans full inner dims) and the shared layout is not transposed. Falls through to the scalar path for non-contiguous arrays, partial tiles, or transposed layouts. Benchmarked on RTX 5090 (1D shared tiles, 16 MB): mat66: 608 → 2180 GB/s (3.6x) mat44: 1022 → 2075 GB/s (2.0x) mat33: 1871 → 2153 GB/s (1.15x) float32: 481 → 481 GB/s (no regression) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Optimize vectorized path runtime checks for lower-end GPUs Add contiguous_shared guard to skip vectorized path for transposed tiles (non-unit last-dim stride in shared memory layout). Optimize can_vectorize() to use flat evaluation (no early returns) with WP_PRAGMA_UNROLL, reducing branch prediction overhead on GPUs like the L40 where instruction overhead is proportionally significant. Simplify the vectorized path entry from two-branch if/else to single expression. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> * Add N-D vectorized tile load/store with aligned parameter (NVIDIAGH-1236) Extend the vectorized float4 load/store path from 2D-only to all dimensions (1D, 3D, 4D) via tile_shape_f4 and tile_vectorized_check_t. This gives up to 1.3x bandwidth improvement for shared-memory tile loads on aligned, contiguous arrays. Add `aligned` parameter to tile_load() and tile_store() that skips runtime eligibility checks when the caller guarantees 16-byte alignment, contiguity, and in-bounds access. Add documentation for vectorized tile loads (conditions, padding tips, aligned usage) and software pipelining with register tiles (1.2-1.6x speedup from overlapping loads with compute). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Zach Corse <zcorse@nvidia.com> Approved-by: Alain Denzler <adenzler@nvidia.com> Approved-by: Zach Corse <zcorse@nvidia.com> Approved-by: Eric Shi <ershi@nvidia.com> See merge request omniverse/warp!2185
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes the following changes to
Dockerfile.theRock_ci:Before your PR is "Ready for review"
__init__.pyi,docs/api_reference/,docs/language_reference/)pre-commit run -a