Repository navigation
fix(mir-importer): preserve call destination evaluation order - #1229
Merged
nihalpasham merged 4 commits intoSep 6, 2026
Merged
Conversation
Signed-off-by: Raul Estrada <raulestradaa@gmail.com>
uurl
force-pushed
the
fix/projected-call-destination-ordering
branch
from
September 4, 2026 04:33
707b56a to
f02781a
Compare
…stination-ordering
…ture CI's per-example fmt loop wants the five-line signature on one line. Whitespace only, straight output of `cargo fmt --manifest-path .../mir_projected_call_destination/Cargo.toml` under the pinned nightly.
…nder test The renderer now emits prepare_destination_write followed by emit_prepared_result_and_goto; the test still asserted the removed emit_store_result_and_goto and failed. Assert both routes, in order.
Collaborator
|
Reviewed and validated:
Layered three commits:
Verified here: 12/12 host/device cases on the 5090, full smoketest 226/226, fork CI green. Merging. |
nihalpasham
approved these changes
Sep 6, 2026
4 of 6 tasks
nihalpasham
added a commit
that referenced
this pull request
Sep 6, 2026
* feat(wmma): add native shared-address ldmatrix forms
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
* refactor(wmma): name ldmatrix u32 carriers explicitly
Signed-off-by: Zihua Wu <13583761+lucifer1004@users.noreply.github.com>
* book: refresh mlir-dialects.md catalog stamp for the new shared-u32 ldmatrix forms
intrinsics/catalog.json's SHA-256 moved to 3df52944 because six existing
ldmatrix entries gained `ldmatrix_*_shared_u32` compatibility paths (entry
count unchanged at 1025; the overlay digest moved with them); the book
page's stamp still read the pre-PR 20bc41c2. Op/module counts are unchanged
(576 ops / 42 modules), so only the stamp needed updating.
* generated_ldmatrix: rustfmt the shared-address conversion
CI's per-example fmt loop reflows the three-line `unsafe { ... }` block at
src/main.rs:108 onto two lines. Whitespace only.
* merge main (#1229 rewrote the intrinsic destination-write path this PR's
cvta lowering shares; resolved by keeping the prepare step and the widened
u32/u64 result type)
Co-authored-by: nihalpasham <nihalp@nvidia.com>
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.
Closes #1164
Summary
MIR call destinations must be evaluated before the callee executes. This matters for projected destinations such as:
where evaluating the destination after the call can observe state mutated by the callee and store the result at a different location.
This PR introduces a prepared destination-write path that materializes the destination before the producer executes, retains the typed storage information across the call or intrinsic, and performs the final store afterward.
What changed
Added a
PreparedDestinationWriteabstraction for call results:ValueMap::store_localOrdinary function-item, closure, and other call paths now follow:
instead of evaluating the projected destination after the call.
Generated intrinsic importers now prepare their destination before emitting the result-producing NVVM operation and use the same prepared-result epilogue.
Regenerated the affected intrinsic importer sources from
cuda-intrinsics-gen.Migrated hand-written intrinsic result paths to the same destination-write abstraction, including unit-returning paths such as
core::intrinsics::copy,typed_swap_nonoverlapping, and compiler-hint no-op intrinsics.Removed the old result-store helpers and their independent single-projection grammar.
Preserved the existing canonical payload storage path, including storage coercions such as boolean payload representation and pointer storage representation.
Updated the intrinsic contributor documentation to reflect the new prepared-destination flow.
Correctness regression
The original ordering bug is observable when the callee mutates a local used by the destination projection.
For example, conceptually:
where
bumpchanges_1to2.The destination must resolve to
RET[0]before entering the callee. Resolving it afterward incorrectly stores intoRET[2].The regression fixture compares host and device behavior and now covers:
Field -> IndexDeref -> FieldIndex -> FieldsincoscasesIt also includes a negative importer test ensuring that if the general place-address walker cannot represent a destination, translation fails with an unsupported-construct error instead of emitting a semantically incorrect store.
Validation
Runtime host/device comparison: