Repository navigation
fix(mir-lower): cap enum alignment anchors - #1357
Merged
Merged
Conversation
Signed-off-by: Yunus Mutlu <mutluyunus708@gmail.com>
YunusMutlu
force-pushed
the
fix/enum-alignment-anchor
branch
from
October 13, 2026 12:00
af0448f to
ffa23db
Compare
…enums Union and enum storage each built their own zero-length alignment anchor and each capped it at a bare 16. One helper, alignment_anchor_type, now builds the anchor for both, and MAX_NATURAL_ALIGN names the cap for the layout model and the test fixtures as well. The helper's doc states the real reason for the cap: the exporter's data layout strings, llvm_type_size_align and the exporter's natural_alignment all stop at i128, while llc's default NVPTX layout has i256:256, so a wider anchor made opt and llc disagree on a type's alignment and raised nothing. Rust alignment is stamped on the memory operations. The three comments that said NVPTX itself stops at 16 bytes now point at the helper. Tests: the enum anchor table gains the 2^21 row (the first width LLVM rejects; 2^29 is the u32 wrap), and a lowering-level regression beside the union one checks that an over-aligned enum array keeps the Rust alignment on the alloca and the store at 32, 64 and 2^29 while the storage keeps rustc's size and stride. Both fail with an unbounded anchor. Signed-off-by: nihalpasham <nihalp@nvidia.com>
No tracked example read an over-aligned enum on a GPU, so the capped alignment anchor had no end-to-end coverage. Tag32 is a u8 tag padded to 32 bytes; the new kernel reads it through *const Tag32 over a byte buffer, so the 32-byte stride and the align 32 on the tag load both have to come from rustc's layout. The host checks tags at offsets 0, 32, 64 and 96. The PASS marker, manifest, lockfile and smoketest category are unchanged. Signed-off-by: nihalpasham <nihalp@nvidia.com>
The debug assertion ran after the clamp, so it could only ever see 1 to 16 and never caught a non-power-of-two ABI alignment above the cap. Signed-off-by: nihalpasham <nihalp@nvidia.com>
A one-page figure for the book and the pull request, in the style of the tile-libraries direct-linkage figure. Signed-off-by: nihalpasham <nihalp@nvidia.com>
Collaborator
|
/ok to test e8e81bc |
Collaborator
|
Thanks, this is the right fix and it matches what union storage has done since #610. On top:
At |
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
When mir-lower mints the LLVM storage type of an enum whose tag and payload bytes under-align it, it inserts a zero-length integer array at slot 0 whose element width encodes the alignment. The width was
align * 8with no bound, so a#[repr(align(1 << 29))]enum wrapped toi0, which LLVM rejects, and anything fromrepr(align(1 << 21))up already exceeded LLVM's integer width limit. The anchor is now capped at 16 bytes,[0 x i128], the cap union storage has used since #610. The Rust alignment still rides on every alloca, load and store of the enum.What we fixed
repr(align(2^20)),2^21,2^29) go from a failed build to building, running and passing memcheck. Enums aligned 32, 64 and 128 produce instruction-identical PTX.i128, while llc's own NVPTX layout givesi256a 32-byte alignment, so a wider anchor made opt and llc disagree and bought nothing.2^21table row, and arepr(align(32))enum case in therepr_u32_enum_strideexample so an over-aligned enum runs on a GPU in CI.Implementation
convert/types/layout.rs:MAX_NATURAL_ALIGNandalignment_anchor_type, used byunion_storage.rs,enum_layout.rs, the layout model and the test fixtures. The size, the stride and the alignment stamped on memory operations do not change; only the anchor's element width for alignments of 32 and up.2^20,2^21,2^29), andconvert_ref_preserves_over_aligned_enum_array_layout_and_alignmentover 32, 64 and2^29.Verification
Checked at
e8e81bc4e(nightly-2026-10-11, RTX 5090):cargo test -p mir-lower --all-targets: 322 unit and 147 integration tests pass; 2440 downstream tests in mir-importer and cuda-oxide-codegen pass. Clippy and rustdoc with warnings as errors, fmt, the example's fmt and clippy gates, the 18 guards and the lock check: clean.2^29(natural alignment 1, expected 16).cargo oxide run repr_u32_enum_stride:align32_enum [0, 1, 1, 0] PASS, memcheck clean. Scratch kernels with enums aligned2^20,2^21and2^29fail to build on main and run here.Caveats
align 8on the global; that is Shared memory alignment can be invalid or weaker than the pointee ABI requirement #1344 and is unchanged here.2^16and up fail closed in NVVM's 16-bit align field. Both are pre-existing and separate.Closes #1356.