From 09f770a8c31e339f10595dd8e6ddec0bab7d7d98 Mon Sep 17 00:00:00 2001 From: nik Date: Fri, 31 Jul 2026 07:59:34 +0000 Subject: [PATCH] fix(mir): preserve wrapping pointer-offset semantics Signed-off-by: nik --- crates/dialect-mir/src/ops/memory.rs | 31 ++++++- crates/dialect-mir/tests/ops_test.rs | 9 ++ crates/llvm-export/src/export/ops.rs | 6 +- crates/llvm-export/src/lib.rs | 22 +++++ crates/llvm-export/tests/export_test.rs | 40 ++++++++ .../terminator/intrinsics/memory.rs | 92 +++++++++++++++++++ .../src/translator/terminator/mod.rs | 15 +++ crates/mir-lower/src/convert/ops/memory.rs | 39 ++++++++ .../examples/ptr_offset_from/Cargo.toml | 1 + .../examples/ptr_offset_from/src/main.rs | 52 +++++++++-- 10 files changed, 296 insertions(+), 11 deletions(-) diff --git a/crates/dialect-mir/src/ops/memory.rs b/crates/dialect-mir/src/ops/memory.rs index 1f5e87338b..a8fdc113a3 100644 --- a/crates/dialect-mir/src/ops/memory.rs +++ b/crates/dialect-mir/src/ops/memory.rs @@ -9,13 +9,14 @@ use pliron::{ builtin::{ - attributes::IntegerAttr, + attributes::{BoolAttr, IntegerAttr}, op_interfaces::{NOpdsInterface, NResultsInterface, OneOpdInterface, OneResultInterface}, types::IntegerType, }, common_traits::Verify, context::{Context, Ptr}, derive::op_interface_impl, + identifier::Identifier, irbuild::{inserter::Inserter, rewriter::Rewriter}, location::Located, op::Op, @@ -732,11 +733,39 @@ impl Verify for MirRefOp { )] pub struct MirPtrOffsetOp; +const PTR_OFFSET_INBOUNDS_KEY: &str = "mir_ptr_offset_inbounds"; + impl MirPtrOffsetOp { /// Create a new MirPtrOffsetOp wrapper. pub fn new(op: Ptr) -> Self { MirPtrOffsetOp { op } } + + /// Set whether this offset carries Rust's in-bounds pointer-arithmetic + /// promise. Wrapping offsets must set this to false. + pub fn set_inbounds(&self, ctx: &mut Context, inbounds: bool) { + let key = Identifier::try_new(PTR_OFFSET_INBOUNDS_KEY.to_string()) + .expect("valid ptr-offset attribute key"); + self.get_operation() + .deref_mut(ctx) + .attributes + .set(key, BoolAttr::new(inbounds)); + } + + /// Return whether this offset may lower to LLVM `getelementptr inbounds`. + /// + /// Pointer offsets without an explicit attribute carry Rust's ordinary + /// in-bounds contract, so absence defaults to true. + pub fn is_inbounds(&self, ctx: &Context) -> bool { + let key = Identifier::try_new(PTR_OFFSET_INBOUNDS_KEY.to_string()) + .expect("valid ptr-offset attribute key"); + self.get_operation() + .deref(ctx) + .attributes + .get::(&key) + .map(|attr| bool::from(attr.clone())) + .unwrap_or(true) + } } impl Verify for MirPtrOffsetOp { diff --git a/crates/dialect-mir/tests/ops_test.rs b/crates/dialect-mir/tests/ops_test.rs index b6bdb4f981..cfe4499e97 100644 --- a/crates/dialect-mir/tests/ops_test.rs +++ b/crates/dialect-mir/tests/ops_test.rs @@ -305,6 +305,15 @@ fn test_mir_ptr_offset_verify() { ); let offset_op = MirPtrOffsetOp::new(op); assert!(offset_op.verify(&ctx).is_ok(), "Valid MirPtrOffsetOp"); + assert!( + offset_op.is_inbounds(&ctx), + "ordinary pointer offsets default to inbounds" + ); + offset_op.set_inbounds(&mut ctx, false); + assert!( + !offset_op.is_inbounds(&ctx), + "wrapping pointer offsets retain their explicit semantics" + ); let block2 = BasicBlock::new(&mut ctx, None, vec![i32_ty.into(), usize_ty.into()]); let i32_val = block2.deref(&ctx).get_argument(0); diff --git a/crates/llvm-export/src/export/ops.rs b/crates/llvm-export/src/export/ops.rs index dbfde5e275..d761d4f5c6 100644 --- a/crates/llvm-export/src/export/ops.rs +++ b/crates/llvm-export/src/export/ops.rs @@ -874,7 +874,11 @@ impl<'a> ModuleExportState<'a> { res_name.clone() }; - write!(output, " {gep_name} = getelementptr inbounds ").unwrap(); + write!(output, " {gep_name} = getelementptr").unwrap(); + if ops::gep_inbounds(self.ctx, op.get_operation()) { + write!(output, " inbounds").unwrap(); + } + write!(output, " ").unwrap(); self.export_type(elem_ty, output)?; write!(output, ", ").unwrap(); if self.legacy_typed_pointers() { diff --git a/crates/llvm-export/src/lib.rs b/crates/llvm-export/src/lib.rs index ebed079c23..0c8f23b75e 100644 --- a/crates/llvm-export/src/lib.rs +++ b/crates/llvm-export/src/lib.rs @@ -271,6 +271,10 @@ pub mod ops { /// and emitted as `align N` during export. const OP_ALIGNMENT_KEY: &str = "cuda_oxide_op_alignment"; + /// Op-attribute key controlling whether a GEP is emitted with LLVM's + /// `inbounds` promise. Absence denotes an in-bounds GEP. + const GEP_INBOUNDS_KEY: &str = "cuda_oxide_gep_inbounds"; + /// Op-attribute key controlling whether an inline asm op is emitted with /// LLVM's `sideeffect` marker. Absent means true, matching the conservative /// default for user-authored inline PTX. @@ -295,6 +299,24 @@ pub mod ops { .unwrap_or(false) } + /// Stamp the source pointer arithmetic contract onto an LLVM GEP. + pub fn set_gep_inbounds(ctx: &mut Context, op: Ptr, inbounds: bool) { + let key = Identifier::try_new(GEP_INBOUNDS_KEY.to_string()).expect("valid identifier"); + op.deref_mut(ctx) + .attributes + .set(key, BoolAttr::new(inbounds)); + } + + /// Return whether an LLVM GEP carries the `inbounds` promise. + pub fn gep_inbounds(ctx: &Context, op: Ptr) -> bool { + let key = Identifier::try_new(GEP_INBOUNDS_KEY.to_string()).expect("valid identifier"); + op.deref(ctx) + .attributes + .get::(&key) + .map(|attr| bool::from(attr.clone())) + .unwrap_or(true) + } + /// Debug type metadata for a local variable described by `llvm.dbg.declare`. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub enum DebugLocalTypeKind { diff --git a/crates/llvm-export/tests/export_test.rs b/crates/llvm-export/tests/export_test.rs index 670213d6e7..232feaebb2 100644 --- a/crates/llvm-export/tests/export_test.rs +++ b/crates/llvm-export/tests/export_test.rs @@ -304,6 +304,46 @@ fn legacy_gep_rejects_a_result_address_space_different_from_its_base() { ); } +#[test] +fn gep_inbounds_marker_controls_exported_pointer_semantics() { + let mut ctx = Context::new(); + let module = ModuleOp::new(&mut ctx, "gep_semantics".try_into().unwrap()); + let module_block = module_top_block(&mut ctx, &module); + + let pointer = PointerType::get(&ctx, 0); + let i32_ty = IntegerType::get(&ctx, 32, Signedness::Signless); + let void_ty = VoidType::get(&ctx); + let func_ty = FuncType::get(&ctx, void_ty.into(), vec![pointer.into()], false); + let func = FuncOp::new(&mut ctx, "offsets".try_into().unwrap(), func_ty); + let entry = func.get_or_create_entry_block(&mut ctx); + let base = entry.deref(&ctx).get_argument(0); + + let ordinary = GetElementPtrOp::new(&mut ctx, base, vec![GepIndex::Constant(1)], i32_ty.into()); + ordinary.get_operation().insert_at_back(entry, &ctx); + + let wrapping = GetElementPtrOp::new(&mut ctx, base, vec![GepIndex::Constant(2)], i32_ty.into()); + llvm_export::ops::set_gep_inbounds(&mut ctx, wrapping.get_operation(), false); + wrapping.get_operation().insert_at_back(entry, &ctx); + + ReturnOp::new(&mut ctx, None) + .get_operation() + .insert_at_back(entry, &ctx); + func.get_operation().insert_at_back(module_block, &ctx); + + let ir = export_module_to_string(&ctx, &module).expect("GEP export succeeds"); + let gep_lines: Vec<_> = ir + .lines() + .filter(|line| line.contains("getelementptr")) + .collect(); + assert_eq!(gep_lines.len(), 2, "{ir}"); + assert!(gep_lines[0].contains("getelementptr inbounds"), "{ir}"); + assert!( + gep_lines[1].contains("getelementptr i32") + && !gep_lines[1].contains("getelementptr inbounds"), + "{ir}" + ); +} + #[test] fn legacy_pointer_select_keeps_one_canonical_type() { let mut ctx = Context::new(); diff --git a/crates/mir-importer/src/translator/terminator/intrinsics/memory.rs b/crates/mir-importer/src/translator/terminator/intrinsics/memory.rs index f93732718a..5389699ac2 100644 --- a/crates/mir-importer/src/translator/terminator/intrinsics/memory.rs +++ b/crates/mir-importer/src/translator/terminator/intrinsics/memory.rs @@ -199,6 +199,98 @@ pub fn emit_volatile_store( } } +/// Emits `core::intrinsics::arith_offset::(ptr, count) -> *const T`. +/// +/// This intrinsic backs the safe wrapping raw-pointer offset methods. The +/// explicit non-inbounds marker preserves wrapping semantics through LLVM +/// lowering while retaining the source pointer's pointee type and address +/// space. +#[allow(clippy::too_many_arguments)] +pub fn emit_arith_offset( + ctx: &mut Context, + body: &mir::Body, + args: &[mir::Operand], + destination: &mir::Place, + target: &Option, + block_ptr: Ptr, + prev_op: Option>, + value_map: &mut ValueMap, + block_map: &[Ptr], + loc: Location, +) -> TranslationResult> { + use dialect_mir::ops::MirPtrOffsetOp; + use dialect_mir::types::MirPtrType; + + if args.len() != 2 { + return input_err!( + loc.clone(), + TranslationErr::unsupported(format!( + "arith_offset expects 2 arguments (ptr, count), got {}", + args.len() + )) + ); + } + + let (ptr, op_after_ptr) = rvalue::translate_operand( + ctx, + body, + &args[0], + value_map, + block_ptr, + prev_op, + loc.clone(), + )?; + let ptr_type = ptr.get_type(ctx); + if ptr_type.deref(ctx).downcast_ref::().is_none() { + return input_err!( + loc.clone(), + TranslationErr::unsupported(format!( + "arith_offset: expected pointer operand, got {:?}", + ptr_type.deref(ctx) + )) + ); + } + + let (count, op_after_count) = rvalue::translate_operand( + ctx, + body, + &args[1], + value_map, + block_ptr, + op_after_ptr, + loc.clone(), + )?; + let offset = Operation::new( + ctx, + MirPtrOffsetOp::get_concrete_op_info(), + vec![ptr_type], + vec![ptr, count], + vec![], + 0, + ); + offset.deref_mut(ctx).set_loc(loc.clone()); + MirPtrOffsetOp::new(offset).set_inbounds(ctx, false); + if let Some(prev) = op_after_count { + offset.insert_after(ctx, prev); + } else { + offset.insert_at_front(block_ptr, ctx); + } + + let result = offset.deref(ctx).get_result(0); + emit_store_result_and_goto( + ctx, + destination, + result, + target, + block_ptr, + offset, + value_map, + block_map, + loc, + "arith_offset call without target block", + ) +} + #[derive(Clone, Copy)] enum PtrOffsetFromResult { Signed, diff --git a/crates/mir-importer/src/translator/terminator/mod.rs b/crates/mir-importer/src/translator/terminator/mod.rs index 39b5261da5..a352e992a3 100644 --- a/crates/mir-importer/src/translator/terminator/mod.rs +++ b/crates/mir-importer/src/translator/terminator/mod.rs @@ -2670,6 +2670,21 @@ fn try_dispatch_intrinsic( )?)) } + "core::intrinsics::arith_offset" | "std::intrinsics::arith_offset" => { + Ok(Some(intrinsics::memory::emit_arith_offset( + ctx, + body, + args, + destination, + target, + block_ptr, + prev_op, + value_map, + block_map, + loc, + )?)) + } + "core::intrinsics::ptr_offset_from" | "std::intrinsics::ptr_offset_from" => { Ok(Some(intrinsics::memory::emit_ptr_offset_from( ctx, diff --git a/crates/mir-lower/src/convert/ops/memory.rs b/crates/mir-lower/src/convert/ops/memory.rs index 54cb3b0ea8..894b600a60 100644 --- a/crates/mir-lower/src/convert/ops/memory.rs +++ b/crates/mir-lower/src/convert/ops/memory.rs @@ -503,6 +503,8 @@ pub(crate) fn convert_ptr_offset( vec![llvm_export::ops::GepIndex::Value(offset)], elem_ty, ); + let inbounds = dialect_mir::ops::MirPtrOffsetOp::new(op).is_inbounds(ctx); + llvm::set_gep_inbounds(ctx, llvm_gep.get_operation(), inbounds); rewriter.insert_operation(ctx, llvm_gep.get_operation()); rewriter.replace_operation(ctx, op, llvm_gep.get_operation()); @@ -1576,6 +1578,43 @@ mod tests { .downcast_ref::() .expect("gep src_elem_type should be IntegerType"); assert_eq!(int_ty.width(), 32, "gep elem type must be i32 (pointee)"); + assert!( + llvm::gep_inbounds(&ctx, gep.get_operation()), + "ordinary pointer offsets retain the in-bounds contract" + ); + } + + #[test] + fn convert_wrapping_ptr_offset_lowers_to_non_inbounds_gep() { + let mut ctx = make_ctx(); + let i32_ty: TypeHandle = IntegerType::get(&ctx, 32, Signedness::Signless).into(); + let i64_ty: TypeHandle = IntegerType::get(&ctx, 64, Signedness::Signed).into(); + let mir_ptr_ty = MirPtrType::get_generic(&mut ctx, i32_ty, false); + + let (module_ptr, block) = build_kernel(&mut ctx, vec![mir_ptr_ty.into(), i64_ty], vec![]); + let ptr_val = block.deref(&ctx).get_argument(0); + let off_val = block.deref(&ctx).get_argument(1); + + let off_op = Operation::new( + &mut ctx, + mir::MirPtrOffsetOp::get_concrete_op_info(), + vec![mir_ptr_ty.into()], + vec![ptr_val, off_val], + vec![], + 0, + ); + mir::MirPtrOffsetOp::new(off_op).set_inbounds(&mut ctx, false); + off_op.insert_at_back(block, &ctx); + append_mir_return(&mut ctx, block, vec![]); + + crate::lower_mir_to_llvm(&mut ctx, module_ptr).expect("lowering failed"); + + let body = kernel_blocks(&ctx, module_ptr); + let gep = find_first::(&ctx, &body).expect("expected one llvm.gep"); + assert!( + !llvm::gep_inbounds(&ctx, gep.get_operation()), + "wrapping pointer offsets must not promise in-bounds arithmetic" + ); } // ========================================================================= diff --git a/crates/rustc-codegen-cuda/examples/ptr_offset_from/Cargo.toml b/crates/rustc-codegen-cuda/examples/ptr_offset_from/Cargo.toml index 4d12083f1b..17bc8c4e1c 100644 --- a/crates/rustc-codegen-cuda/examples/ptr_offset_from/Cargo.toml +++ b/crates/rustc-codegen-cuda/examples/ptr_offset_from/Cargo.toml @@ -11,6 +11,7 @@ license = "Apache-2.0" # - `ptr_offset_from_unsigned::` via `offset_from_unsigned` # - `ptr_offset_from::` via `offset_from` # - byte-oriented wrappers via `byte_offset_from*` +# - `arith_offset::` via safe wrapping pointer offsets # # Build and run with: # cargo oxide run ptr_offset_from diff --git a/crates/rustc-codegen-cuda/examples/ptr_offset_from/src/main.rs b/crates/rustc-codegen-cuda/examples/ptr_offset_from/src/main.rs index ad8e18ce5d..98d9eb05d5 100644 --- a/crates/rustc-codegen-cuda/examples/ptr_offset_from/src/main.rs +++ b/crates/rustc-codegen-cuda/examples/ptr_offset_from/src/main.rs @@ -1,13 +1,14 @@ // Copyright (c) 2024-2026 NVIDIA CORPORATION. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Regression coverage for raw pointer distance intrinsics. +//! Regression coverage for raw pointer distance and wrapping-offset intrinsics. //! //! The stable raw pointer methods lower through rustc intrinsics in MIR: //! `offset_from_unsigned` uses `ptr_offset_from_unsigned`, while //! `offset_from` and the byte-oriented wrappers use `ptr_offset_from`. -//! cuda-oxide must compute the byte address difference and divide by the -//! pointee size, preserving signed results for negative distances. +//! Safe `wrapping_offset`/`wrapping_add` use `arith_offset`. cuda-oxide must +//! preserve signed distances, scale offsets by the pointee type, retain pointer +//! mutability through libcore's cast, and leave zero-sized pointees stationary. use cuda_core::{CudaContext, DeviceBuffer, LaunchConfig}; use cuda_device::{DisjointSlice, cuda_module, kernel, thread}; @@ -16,8 +17,19 @@ use cuda_device::{DisjointSlice, cuda_module, kernel, thread}; mod kernels { use super::*; + #[inline(always)] + fn wrapping_offset(pointer: *const T, count: isize) -> *const T { + pointer.wrapping_offset(count) + } + #[kernel] - pub fn pointer_distances(values: &[u32], lo: usize, hi: usize, mut out: DisjointSlice) { + pub fn pointer_offsets( + values: &[u32], + lo: usize, + hi: usize, + extreme: isize, + mut out: DisjointSlice, + ) { if thread::index_1d().get() == 0 { let base = values.as_ptr(); unsafe { @@ -28,12 +40,25 @@ mod kernels { *out.get_unchecked_mut(1) = lo_ptr.offset_from(hi_ptr) as i64; *out.get_unchecked_mut(2) = hi_ptr.byte_offset_from(lo_ptr) as i64; *out.get_unchecked_mut(3) = hi_ptr.byte_offset_from_unsigned(lo_ptr) as i64; + *out.get_unchecked_mut(4) = *base.wrapping_add(hi) as i64; + *out.get_unchecked_mut(5) = *hi_ptr.wrapping_offset(-((hi - lo) as isize)) as i64; + *out.get_unchecked_mut(6) = + *base.cast::<[u32; 3]>().wrapping_add(2).cast::() as i64; + *out.get_unchecked_mut(7) = *(base as *mut u32).wrapping_add(lo) as i64; + *out.get_unchecked_mut(8) = + (wrapping_offset(base.cast::<()>(), hi as isize) == base.cast::<()>()) as i64; + let wrapped = base.wrapping_offset(extreme).addr(); + let byte_delta = wrapped.wrapping_sub(base.addr()); + let expected_delta = (extreme as usize).wrapping_mul(core::mem::size_of::()); + *out.get_unchecked_mut(9) = (byte_delta == expected_delta) as i64; + *out.get_unchecked_mut(10) = + (wrapping_offset(base.cast::<()>(), extreme) == base.cast::<()>()) as i64; } } } } -const SLOTS: usize = 4; +const SLOTS: usize = 11; fn main() { let ctx = CudaContext::new(0).expect("ctx"); @@ -43,22 +68,24 @@ fn main() { let values: Vec = (0..16).map(|v| v * 11).collect(); let lo = 3usize; let hi = 9usize; + let extreme = isize::MAX; let values_dev = DeviceBuffer::from_host(&stream, &values).unwrap(); let mut out_dev = DeviceBuffer::::zeroed(&stream, SLOTS).unwrap(); // SAFETY: launch shape/resources match the kernel; buffers cover its accesses. unsafe { - module.pointer_distances( + module.pointer_offsets( &stream, LaunchConfig::for_num_elems(1), &values_dev, lo, hi, + extreme, &mut out_dev, ) } - .expect("launch pointer_distances"); + .expect("launch pointer_offsets"); let got = out_dev.to_host_vec(&stream).unwrap(); let element_distance = (hi - lo) as i64; @@ -68,6 +95,13 @@ fn main() { -element_distance, byte_distance, byte_distance, + values[hi] as i64, + values[lo] as i64, + values[6] as i64, + values[lo] as i64, + 1, + 1, + 1, ]; let mut errors = 0; @@ -79,9 +113,9 @@ fn main() { } if errors == 0 { - println!("SUCCESS: pointer distances are correct"); + println!("SUCCESS: pointer distances and wrapping offsets are correct"); } else { - println!("FAILURE: {errors} pointer distance mismatches"); + println!("FAILURE: {errors} pointer offset mismatches"); std::process::exit(1); } }