Repository navigation
fix(mir-importer): normalize pointer address spaces in tuple construction - #1454
Merged
Merged
Conversation
Signed-off-by: Raul Estrada <raulestradaa@gmail.com>
uurl
force-pushed
the
fix/as7-tuple-normalization
branch
from
October 8, 2026 08:55
ff2b278 to
95133c9
Compare
…README Run rustfmt over the cluster example so the per-example formatting check passes: the bounce_cluster_pair signature, the `let pair` binding and the `let neighbor_value` if-expression each move back onto one line. No semantic change. Extend the test_dsmem_ring_exchange README bullet: the mapped pointer is also round-tripped through a (ptr, rank) tuple and a device fn and compared against the direct read. Signed-off-by: nihalpasham <nihalp@nvidia.com>
The tuple round-trip comment attributed the missing call in the PTX to the PTX optimizer. It is LLVM opt -O2 that inlines the private #[inline(never)] helper, because noinline is not propagated to the IR, so the call survives only in the pre-opt cluster.ll. Say so. Add a require_ll_shape assertion that the mapped addrspace(7) pointer is cast to a generic pointer before the tuple is constructed. The call check only proves the helper was reached; this one pins the normalization itself, which is what the round-trip exists to cover. Signed-off-by: nihalpasham <nihalp@nvidia.com>
Collaborator
|
/ok to test 8abc330 |
Collaborator
|
Checked at
Closes #1453. |
nihalpasham
approved these changes
Oct 15, 2026
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
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.
What this adds
A tuple holding a cluster-shared pointer from
cluster::map_shared_ranknow compiles. Tuple construction casts each element to the tuple's declared element type before building the aggregate, the same normalization that struct, enum, array, union and slice construction already apply, so theaddrspace(7)pointer enters the generic slot through anaddrspacecastinstead of failing MIR verification.What we fixed
AggregateKind::Tuplewas the only aggregate arm that builtMirConstructTupleOpstraight from the translated operands. The verifier requires operand types equal to the element types, so any tuple holding a pointer outside the generic address space (cluster-sharedaddrspace(7), CTA-sharedaddrspace(3)) failed.MirTupleTypeelement types and runscast_to_expected_pointer_type_if_neededper element, keeping the pointer kind (RawConststaysRawConst) and declining kind changes as before.(ptr, rank)tuple and a device fn, and its IR check requires the tuple-ABI call and theaddrspacecastin the pre-opt IR; the compile-only example lane runs that check.Address-space contract
map_shared_rankreturns. A later deref through the aggregate is a generic access unless LLVM can prove the space; at -O2 it restoredld/st.shared::clusterin every form tested, and a true mixed cluster/CTA join produced a correct generic load.ld/st.shared::cluster.Implementation
mir-importertranslator/rvalue/aggregate.rs, tuple arm: downcast the translated tuple type toMirTupleType, zipget_types()with the element values, cast each through the shared helper and thread the previous op. The downcast cannot fail:translate_typealways yieldsMirTupleTypefor a Rust tuple.translator/rvalue/coerce.rs: a helper-contract test that anaddrspace(7)RawConstpointer normalizes to the genericRawConsttype throughmir.cast. It documents the helper; the regression for the arm is the example plus its IR check.mir.cast <PtrToPtr>;mir-loweremit_pointer_castalready emitsllvm.addrspacecastwhen the spaces differ, so no lowering change.bounce_cluster_pair, the tuple round trip and comparison against the direct read;verify-code-shape.shasserts the call and theaddrspacecastincluster.ll, read beforeopt -O2because the private helper is inlined there.Verification
Checked at
95133c9cmerged withmain(bc46b1ff, nightly-2026-10-11):MirConstructTupleOpmismatch; the same source builds at this head.cargo test -p mir-importer: 1164 passed at the PR head, 1169 on the merged tree, 0 failed.cargo clippy -p mir-importer --all-targets -- -D warningsand rustdoc with warnings denied pass; the example's own clippy passes.cargo oxide run clusteron an RTX 5060 Laptop (author) and an RTX 5090 (sm_120): ring exchange 1001, 1002, 1003, 1000 and mapped store 3003, 3000, 3001, 3002; memcheck and synccheck 0 errors.CUDA_OXIDE_NO_OPT=1 cargo oxide run cluster: same values with the PTX keepingcvta.shared::cluster.u64,call.uni bounce_cluster_pairand a genericld.b32through the tuple; memcheck 0 errors.cargo oxide run cluster --emit-nvvm-ir --arch sm_120(libNVVM): passes.cargo oxide build cluster --arch sm_90plusverify-code-shape.sh: PASS, the compile-only lane's form.Option<*const u32>, destructured return,*mutstore through a tuple, layout-reordered(u8, *const u32, u16), control-flow join, pointer equality across the cast, mixedaddrspace(7)/addrspace(3)join) compile and run correctly at -O2 and NO_OPT; memcheck and synccheck 0 errors. A by-value closure capture of the mapped pointer already compiled onmain.main.git diff --checkand the per-example rustfmt check pass; DCO sign-off present.Caveats
bounced == directcomparison folds to true because LLVM inlines the helper and merges the two loads; the enforced gate is the IR check, and the generic load through the tuple executes only in the NO_OPT build.pair.0 = p), or[p, p]/[p; 2]into an addressable local, failsMirStoreOpverification onmainas well; the store path lacks the same normalization. Tracked separately.cvta.shared::clusterfor sm_90+ and the runs were on sm_120. The legacy CUDA 12 NVVM route at sm_90 fails on this host formaintoo (PTX JIT error) and was not validated.Closes #1453.