From b6ad9939bb50e30289e366fadf0b1f370fef9a99 Mon Sep 17 00:00:00 2001 From: Yunus Mutlu Date: Sat, 26 Sep 2026 00:46:42 +0300 Subject: [PATCH] fix(mir-lower): stabilize shared pointer union storage Fixes #1334 Signed-off-by: Yunus Mutlu --- .../src/convert/ops/aggregate/fields.rs | 194 +++++++++++++++++- .../crates/mir-lower/src/convert/types/mod.rs | 2 +- .../src/convert/types/union_storage.rs | 145 ++++++++++++- 3 files changed, 335 insertions(+), 6 deletions(-) diff --git a/cuda-oxide/crates/mir-lower/src/convert/ops/aggregate/fields.rs b/cuda-oxide/crates/mir-lower/src/convert/ops/aggregate/fields.rs index 41ca9756cf..e60a2878d1 100644 --- a/cuda-oxide/crates/mir-lower/src/convert/ops/aggregate/fields.rs +++ b/cuda-oxide/crates/mir-lower/src/convert/ops/aggregate/fields.rs @@ -4,9 +4,10 @@ */ use super::common::{anyhow_to_pliron, spill_enum_value}; +use crate::convert::target_stable_storage::coerce_target_stable_value; use crate::convert::types::{ StructLayoutInfo, StructSlotMap, build_struct_slot_map, build_union_storage_type, convert_type, - is_zero_sized_type, make_slice_struct, + is_zero_sized_type, make_slice_struct, union_field_storage_type, }; use dialect_mir::ops::{MirExtractFieldOp, MirInsertFieldOp}; use dialect_mir::types::{ @@ -311,10 +312,15 @@ fn convert_extract_union_field( let storage_ty = build_union_storage_type(ctx, &union_ty).map_err(anyhow_to_pliron)?; let ptr = spill_enum_value(ctx, rewriter, union_value, storage_ty, union_ty.abi_align()); - let load = llvm::LoadOp::new(ctx, ptr, field_llvm_ty); + let field_storage_ty = + union_field_storage_type(ctx, field_llvm_ty).map_err(anyhow_to_pliron)?; + let load = llvm::LoadOp::new(ctx, ptr, field_storage_ty); llvm_export::ops::set_op_alignment(ctx, load.get_operation(), union_ty.abi_align() as u32); rewriter.insert_operation(ctx, load.get_operation()); - rewriter.replace_operation(ctx, op, load.get_operation()); + let stored = load.get_operation().deref(ctx).get_result(0); + let semantic = + coerce_target_stable_value(ctx, rewriter, stored, field_llvm_ty, "union field storage")?; + rewriter.replace_operation_with_values(ctx, op, vec![semantic]); Ok(()) } @@ -344,7 +350,16 @@ fn convert_insert_union_field( let storage_ty = build_union_storage_type(ctx, &union_ty).map_err(anyhow_to_pliron)?; let ptr = spill_enum_value(ctx, rewriter, union_value, storage_ty, union_ty.abi_align()); - let store = llvm::StoreOp::new(ctx, new_value, ptr); + let field_storage_ty = + union_field_storage_type(ctx, field_llvm_ty).map_err(anyhow_to_pliron)?; + let stored_value = coerce_target_stable_value( + ctx, + rewriter, + new_value, + field_storage_ty, + "union field storage", + )?; + let store = llvm::StoreOp::new(ctx, stored_value, ptr); llvm_export::ops::set_op_alignment(ctx, store.get_operation(), union_ty.abi_align() as u32); rewriter.insert_operation(ctx, store.get_operation()); @@ -363,6 +378,7 @@ mod tests { use crate::convert::ops::test_util::*; use dialect_mir::attributes::FieldIndexAttr; use dialect_mir::ops as mir; + use dialect_mir::types::MirPtrType; use llvm_export::types as llvm_types; @@ -431,6 +447,176 @@ mod tests { ); } + /// A shared pointer carried through a union must use generic-pointer + /// physical storage in both directions. Construction converts AS3 -> AS0; + /// extraction converts the stored AS0 pointer back to semantic AS3. + #[test] + fn shared_pointer_union_field_round_trips_through_generic_storage() { + let mut ctx = make_ctx(); + + let pointee: TypeHandle = IntegerType::get(&ctx, 32, Signedness::Unsigned).into(); + let shared: TypeHandle = MirPtrType::get_shared(&mut ctx, pointee, true).into(); + let bits: TypeHandle = IntegerType::get(&ctx, 64, Signedness::Unsigned).into(); + let union_ty: TypeHandle = MirUnionType::get( + &mut ctx, + "SharedPointerBits".into(), + vec!["ptr".into(), "bits".into()], + vec![shared, bits], + 8, + 8, + ) + .into(); + + let (module, block) = build_kernel(&mut ctx, vec![shared], vec![shared]); + let pointer = block.deref(&ctx).get_argument(0); + + let undef = mir::MirUndefOp::new(&mut ctx, union_ty).get_operation(); + undef.insert_at_back(block, &ctx); + let undef_value = undef.deref(&ctx).get_result(0); + + let insert = Operation::new( + &mut ctx, + MirInsertFieldOp::get_concrete_op_info(), + vec![union_ty], + vec![undef_value, pointer], + vec![], + 0, + ); + MirInsertFieldOp::new(insert).set_attr_insert_index(&ctx, FieldIndexAttr(0)); + insert.insert_at_back(block, &ctx); + let union_value = insert.deref(&ctx).get_result(0); + + let extract = Operation::new( + &mut ctx, + MirExtractFieldOp::get_concrete_op_info(), + vec![shared], + vec![union_value], + vec![], + 0, + ); + MirExtractFieldOp::new(extract).set_attr_index(&ctx, FieldIndexAttr(0)); + extract.insert_at_back(block, &ctx); + let result = extract.deref(&ctx).get_result(0); + + append_mir_return(&mut ctx, block, vec![result]); + + crate::lower_mir_to_llvm(&mut ctx, module).expect("lowering failed"); + let body = kernel_blocks(&ctx, module); + + assert_eq!( + count_ops::(&ctx, &body), + 2, + "union insertion must genericize AS3 and extraction must restore AS3" + ); + assert_eq!( + count_ops::(&ctx, &body), + 0, + "MirInsertFieldOp must be fully lowered" + ); + assert_eq!( + count_ops::(&ctx, &body), + 0, + "MirExtractFieldOp must be fully lowered" + ); + } + + #[test] + fn nested_shared_pointer_union_field_round_trips_through_generic_storage() { + let mut ctx = make_ctx(); + + let pointee: TypeHandle = IntegerType::get(&ctx, 32, Signedness::Unsigned).into(); + let shared: TypeHandle = MirPtrType::get_shared(&mut ctx, pointee, true).into(); + let cookie: TypeHandle = IntegerType::get(&ctx, 64, Signedness::Unsigned).into(); + let inner: TypeHandle = MirStructType::get_with_full_layout( + &mut ctx, + "SharedPointerInner".into(), + vec!["pointer".into(), "cookie".into()], + vec![shared, cookie], + vec![0, 1], + vec![0, 8], + 16, + 8, + ) + .into(); + let union_ty: TypeHandle = MirUnionType::get( + &mut ctx, + "NestedSharedPointerUnion".into(), + vec!["inner".into()], + vec![inner], + 16, + 8, + ) + .into(); + + let (module, block) = build_kernel(&mut ctx, vec![shared, cookie], vec![shared]); + let pointer = block.deref(&ctx).get_argument(0); + let cookie_value = block.deref(&ctx).get_argument(1); + + let construct_inner = Operation::new( + &mut ctx, + mir::MirConstructStructOp::get_concrete_op_info(), + vec![inner], + vec![pointer, cookie_value], + vec![], + 0, + ); + construct_inner.insert_at_back(block, &ctx); + let inner_value = construct_inner.deref(&ctx).get_result(0); + + let undef = mir::MirUndefOp::new(&mut ctx, union_ty).get_operation(); + undef.insert_at_back(block, &ctx); + let undef_value = undef.deref(&ctx).get_result(0); + + let insert = Operation::new( + &mut ctx, + MirInsertFieldOp::get_concrete_op_info(), + vec![union_ty], + vec![undef_value, inner_value], + vec![], + 0, + ); + MirInsertFieldOp::new(insert).set_attr_insert_index(&ctx, FieldIndexAttr(0)); + insert.insert_at_back(block, &ctx); + let union_value = insert.deref(&ctx).get_result(0); + + let extract_inner = Operation::new( + &mut ctx, + MirExtractFieldOp::get_concrete_op_info(), + vec![inner], + vec![union_value], + vec![], + 0, + ); + MirExtractFieldOp::new(extract_inner).set_attr_index(&ctx, FieldIndexAttr(0)); + extract_inner.insert_at_back(block, &ctx); + let recovered_inner = extract_inner.deref(&ctx).get_result(0); + + let extract_pointer = Operation::new( + &mut ctx, + MirExtractFieldOp::get_concrete_op_info(), + vec![shared], + vec![recovered_inner], + vec![], + 0, + ); + MirExtractFieldOp::new(extract_pointer).set_attr_index(&ctx, FieldIndexAttr(0)); + extract_pointer.insert_at_back(block, &ctx); + let result = extract_pointer.deref(&ctx).get_result(0); + + append_mir_return(&mut ctx, block, vec![result]); + + crate::lower_mir_to_llvm(&mut ctx, module).expect("lowering failed"); + let body = kernel_blocks(&ctx, module); + + assert_eq!( + count_ops::(&ctx, &body), + 2, + "nested union insertion must genericize AS3 and extraction must restore AS3" + ); + assert_eq!(count_ops::(&ctx, &body), 0); + assert_eq!(count_ops::(&ctx, &body), 0); + } + /// The row-width arm of [`resolve_aggregate_slots`]' no-history fallback /// is defensive symmetry: no current lowering path produces a runtime-width /// slice value with an empty conversion history, so no end-to-end pipeline diff --git a/cuda-oxide/crates/mir-lower/src/convert/types/mod.rs b/cuda-oxide/crates/mir-lower/src/convert/types/mod.rs index 1d112dfa60..fb1181fa60 100644 --- a/cuda-oxide/crates/mir-lower/src/convert/types/mod.rs +++ b/cuda-oxide/crates/mir-lower/src/convert/types/mod.rs @@ -142,4 +142,4 @@ pub(crate) use pointer_storage::{ }; pub use struct_layout::struct_value_lowering_is_byte_faithful; pub(crate) use struct_layout::{StructLayoutInfo, StructSlotMap, build_struct_slot_map}; -pub(crate) use union_storage::build_union_storage_type; +pub(crate) use union_storage::{build_union_storage_type, union_field_storage_type}; diff --git a/cuda-oxide/crates/mir-lower/src/convert/types/union_storage.rs b/cuda-oxide/crates/mir-lower/src/convert/types/union_storage.rs index aa821690ff..6bb1ff85c1 100644 --- a/cuda-oxide/crates/mir-lower/src/convert/types/union_storage.rs +++ b/cuda-oxide/crates/mir-lower/src/convert/types/union_storage.rs @@ -14,6 +14,43 @@ use pliron::r#type::TypeHandle; use super::layout::make_padding_type; use super::pointer_storage::llvm_type_contains_pointer; use super::{convert_type, llvm_type_is_byte_faithful, llvm_type_size_align}; +use crate::convert::target_stable_storage::{StorageRewriteOptions, target_stable_storage_type}; + +/// Maximum number of shared-pointer leaves introduced by fixed-array expansion +/// while rebuilding one union field at its semantic/physical storage boundary. +/// +/// Direct pointers and struct nesting remain proportional to source structure. +/// Arrays can encode arbitrarily many extract/cast/insert sequences compactly, +/// so only array-expanded AS3 leaves count against this code-shape budget. +pub(crate) const MAX_UNION_FIELD_ARRAY_REWRITE_LEAVES: u64 = 16; + +/// Return the target-stable physical type used for one union field. +/// +/// Shared-memory pointers become generic pointers recursively so the union's +/// byte image is independent of whether the eventual backend uses legacy +/// 64-bit AS3 pointers or modern NVVM's 32-bit AS3 representation. +pub(crate) fn union_field_storage_type( + ctx: &mut Context, + semantic_ty: TypeHandle, +) -> Result { + let rewrite = target_stable_storage_type( + ctx, + semantic_ty, + StorageRewriteOptions { + canonicalize_bool: false, + }, + "union field storage", + )?; + + if rewrite.array_shared_pointer_leaves > MAX_UNION_FIELD_ARRAY_REWRITE_LEAVES { + return Err(anyhow::anyhow!( + "union field storage: arrays containing shared-memory pointers are not supported above the bounded rewrite limit; rewrite requires {} pointer conversions, supported bound is {MAX_UNION_FIELD_ARRAY_REWRITE_LEAVES}", + rewrite.array_shared_pointer_leaves + )); + } + + Ok(rewrite.ty) +} /// Build byte-exact LLVM storage for a Rust union. /// @@ -92,7 +129,33 @@ pub(crate) fn build_union_storage_type( } pointer_carrier = Some(llvm_field_ty); } - fields.push((llvm_field_ty, field_size, field_align, contains_pointer)); + let storage_field_ty = union_field_storage_type(ctx, llvm_field_ty)?; + let (storage_size, storage_align) = llvm_type_size_align(ctx, storage_field_ty) + .ok_or_else(|| { + anyhow::anyhow!( + "union `{}` field {} has unsupported target-stable LLVM storage layout", + union_ty.name(), + index + ) + })?; + if storage_size != field_size || storage_align != field_align { + return Err(anyhow::anyhow!( + "union `{}` field {} target-stable storage changes layout from size/alignment {}/{} to {}/{}", + union_ty.name(), + index, + field_size, + field_align, + storage_size, + storage_align + )); + } + + fields.push(( + storage_field_ty, + storage_size, + storage_align, + contains_pointer, + )); } let storage_align = align.min(16); @@ -273,4 +336,84 @@ mod tests { let llvm_array = convert_type(&mut ctx, array).unwrap(); assert_eq!(llvm_type_size_align(&ctx, llvm_array), Some((96, 16))); } + + #[test] + fn union_field_storage_accepts_shared_pointer_array_at_rewrite_bound() { + let mut ctx = make_ctx(); + let u32_ty = mir_uint(&mut ctx, 32); + let shared: TypeHandle = MirPtrType::get_shared(&mut ctx, u32_ty, false).into(); + let pointers: TypeHandle = + MirArrayType::get(&mut ctx, shared, MAX_UNION_FIELD_ARRAY_REWRITE_LEAVES).into(); + + let semantic = convert_type(&mut ctx, pointers) + .expect("shared-pointer array must convert to an LLVM semantic type"); + let storage = union_field_storage_type(&mut ctx, semantic) + .expect("the exact bounded-array rewrite limit must remain supported"); + + let storage_ref = storage.deref(&ctx); + let array = storage_ref + .downcast_ref::() + .expect("rewritten field must remain an LLVM array"); + let element_ref = array.elem_type().deref(&ctx); + let pointer = element_ref + .downcast_ref::() + .expect("rewritten array elements must remain pointer-typed"); + + assert_eq!( + pointer.address_space(), + llvm_types::address_space::GENERIC, + "bounded AS3 array elements must use generic pointer storage" + ); + } + + #[test] + fn union_field_storage_rejects_shared_pointer_array_above_rewrite_bound() { + let mut ctx = make_ctx(); + let u32_ty = mir_uint(&mut ctx, 32); + let shared: TypeHandle = MirPtrType::get_shared(&mut ctx, u32_ty, false).into(); + let pointers: TypeHandle = + MirArrayType::get(&mut ctx, shared, MAX_UNION_FIELD_ARRAY_REWRITE_LEAVES + 1).into(); + + let semantic = convert_type(&mut ctx, pointers) + .expect("shared-pointer array must convert to an LLVM semantic type"); + let error = union_field_storage_type(&mut ctx, semantic) + .expect_err("array-expanded AS3 leaves above the budget must fail closed"); + + assert!( + error.to_string().contains("bounded rewrite limit"), + "{error}" + ); + } + + #[test] + fn union_storage_genericizes_target_dependent_shared_pointer_carrier() { + let mut ctx = make_ctx(); + let u32_ty = mir_uint(&mut ctx, 32); + let u64_ty = mir_uint(&mut ctx, 64); + let shared: TypeHandle = MirPtrType::get_shared(&mut ctx, u32_ty, false).into(); + + let union_ty = MirUnionType::get( + &mut ctx, + "SharedPointerBits".into(), + vec!["ptr".into(), "bits".into()], + vec![shared, u64_ty], + 8, + 8, + ); + let union_data = union_ty.deref(&ctx).clone(); + + let storage = build_union_storage_type(&mut ctx, &union_data) + .expect("direct shared pointer must use target-stable union storage"); + let fields = struct_fields(&ctx, storage); + let carrier_ref = fields[1].deref(&ctx); + let carrier = carrier_ref + .downcast_ref::() + .expect("shared pointer carrier must remain pointer-typed"); + assert_eq!( + carrier.address_space(), + llvm_types::address_space::GENERIC, + "shared-pointer union storage must use a target-stable generic pointer carrier" + ); + assert_eq!(llvm_type_size_align(&ctx, storage), Some((8, 8))); + } }