From d2a19e0ad1542ca44da912bc83261c58bf7252c0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 1 Nov 2024 11:01:29 -0700 Subject: [PATCH] Remove `offset_guard_size` from compiler `Heap` structs Push reading `Tunables` down further into where it's needed instead of having duplicate storage per-heap. --- crates/cranelift/src/func_environ.rs | 10 ++++++---- .../src/translate/code_translator/bounds_checks.rs | 9 +++++---- crates/cranelift/src/translate/environ/spec.rs | 7 +++++-- crates/cranelift/src/translate/heap.rs | 3 --- winch/codegen/src/codegen/env.rs | 3 --- winch/codegen/src/codegen/mod.rs | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs index 26b4a1a47ae7..2b20db9983cc 100644 --- a/crates/cranelift/src/func_environ.rs +++ b/crates/cranelift/src/func_environ.rs @@ -1695,6 +1695,10 @@ impl<'module_environment> TargetEnvironment for FuncEnvironment<'module_environm fn proof_carrying_code(&self) -> bool { self.isa.flags().enable_pcc() } + + fn tunables(&self) -> &Tunables { + self.tunables + } } impl<'module_environment> crate::translate::FuncEnvironment @@ -2374,7 +2378,6 @@ impl<'module_environment> crate::translate::FuncEnvironment // If we have a declared maximum, we can make this a "static" heap, which is // allocated up front and never moved. let style = MemoryStyle::for_memory(memory, self.tunables); - let offset_guard_size = self.tunables.memory_guard_size; let (heap_style, readonly_base, base_fact, memory_type) = match style { MemoryStyle::Dynamic { .. } => { let heap_bound = func.create_global_value(ir::GlobalValueData::Load { @@ -2388,7 +2391,7 @@ impl<'module_environment> crate::translate::FuncEnvironment // Create a memtype representing the untyped memory region. let data_mt = func.create_memory_type(ir::MemoryTypeData::DynamicMemory { gv: heap_bound, - size: offset_guard_size, + size: self.tunables.memory_guard_size, }); // This fact applies to any pointer to the start of the memory. let base_fact = ir::Fact::dynamic_base_ptr(data_mt); @@ -2455,7 +2458,7 @@ impl<'module_environment> crate::translate::FuncEnvironment // Create a memtype representing the untyped memory region. let data_mt = func.create_memory_type(ir::MemoryTypeData::Memory { size: bound_bytes - .checked_add(offset_guard_size) + .checked_add(self.tunables.memory_guard_size) .expect("Memory plan has overflowing size plus guard"), }); // This fact applies to any pointer to the start of the memory. @@ -2519,7 +2522,6 @@ impl<'module_environment> crate::translate::FuncEnvironment base: heap_base, min_size, max_size, - offset_guard_size, style: heap_style, index_type: index_type_to_ir_type(self.memory(index).idx_type), memory_type, diff --git a/crates/cranelift/src/translate/code_translator/bounds_checks.rs b/crates/cranelift/src/translate/code_translator/bounds_checks.rs index b49bf1ce2083..47c168bb6708 100644 --- a/crates/cranelift/src/translate/code_translator/bounds_checks.rs +++ b/crates/cranelift/src/translate/code_translator/bounds_checks.rs @@ -65,6 +65,7 @@ where let host_page_size_log2 = env.target_config().page_size_align_log2; let can_use_virtual_memory = heap.page_size_log2 >= host_page_size_log2 && env.signals_based_traps(); + let memory_guard_size = env.tunables().memory_guard_size; let make_compare = |builder: &mut FunctionBuilder, compare_kind: IntCC, @@ -193,7 +194,7 @@ where // multiple fields in the same struct that is in linear memory -- // will all emit the same `index > bound` check, which we can GVN. HeapStyle::Dynamic { bound_gv } - if can_use_virtual_memory && offset_and_size <= heap.offset_guard_size => + if can_use_virtual_memory && offset_and_size <= memory_guard_size => { let bound = get_dynamic_heap_bound(builder, env, heap); let oob = make_compare( @@ -371,7 +372,7 @@ where if can_use_virtual_memory && heap.index_type == ir::types::I32 && u64::from(u32::MAX) - <= u64::from(bound) + u64::from(heap.offset_guard_size) - offset_and_size => + <= u64::from(bound) + u64::from(memory_guard_size) - offset_and_size => { assert!( can_use_virtual_memory, @@ -385,7 +386,7 @@ where offset, AddrPcc::static32( heap.memory_type, - u64::from(bound) + u64::from(heap.offset_guard_size), + u64::from(bound) + u64::from(memory_guard_size), ), )) } @@ -604,7 +605,7 @@ fn explicit_check_oob_condition_and_compute_addr( min: Expr::constant(0), max: Expr::offset( &Expr::global_value(gv), - i64::try_from(heap.offset_guard_size) + i64::try_from(env.tunables().memory_guard_size) .unwrap() .checked_sub(i64::from(access_size)) .unwrap(), diff --git a/crates/cranelift/src/translate/environ/spec.rs b/crates/cranelift/src/translate/environ/spec.rs index 1d1418d342f4..1646630678e4 100644 --- a/crates/cranelift/src/translate/environ/spec.rs +++ b/crates/cranelift/src/translate/environ/spec.rs @@ -17,8 +17,8 @@ use cranelift_frontend::FunctionBuilder; use smallvec::SmallVec; use wasmparser::{Operator, WasmFeatures}; use wasmtime_environ::{ - DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex, TypeConvert, TypeIndex, - WasmHeapType, WasmRefType, WasmResult, + DataIndex, ElemIndex, FuncIndex, GlobalIndex, MemoryIndex, TableIndex, Tunables, TypeConvert, + TypeIndex, WasmHeapType, WasmRefType, WasmResult, }; /// The value of a WebAssembly global variable. @@ -62,6 +62,9 @@ pub trait TargetEnvironment: TypeConvert { /// Returns a pair of the CLIF reference type to use and a boolean that /// describes whether the value should be included in GC stack maps or not. fn reference_type(&self, ty: WasmHeapType) -> (ir::Type, bool); + + /// Returns the compilation knobs that are in effect. + fn tunables(&self) -> &Tunables; } /// A smallvec that holds the IR values for a struct's fields. diff --git a/crates/cranelift/src/translate/heap.rs b/crates/cranelift/src/translate/heap.rs index eff81c1e78f2..1c51133de092 100644 --- a/crates/cranelift/src/translate/heap.rs +++ b/crates/cranelift/src/translate/heap.rs @@ -71,9 +71,6 @@ pub struct HeapData { /// Heap accesses larger than this will always trap. pub max_size: Option, - /// Size in bytes of the offset-guard pages following the heap. - pub offset_guard_size: u64, - /// Heap style, with additional style-specific info. pub style: HeapStyle, diff --git a/winch/codegen/src/codegen/env.rs b/winch/codegen/src/codegen/env.rs index 7315890c68de..a3f38340c0f8 100644 --- a/winch/codegen/src/codegen/env.rs +++ b/winch/codegen/src/codegen/env.rs @@ -84,8 +84,6 @@ pub struct HeapData { /// By default the page size is 64KiB (0x10000; 2**16; 1<<16; 65536) but the /// custom-page-sizes proposal allows opting into a page size of `1`. pub page_size_log2: u8, - /// Size in bytes of the offset guard pages, located after the heap bounds. - pub offset_guard_size: u64, } /// A function callee. @@ -309,7 +307,6 @@ impl<'a, 'translation, 'data, P: PtrSize> FuncEnv<'a, 'translation, 'data, P> { min_size, max_size, page_size_log2: memory.page_size_log2, - offset_guard_size: self.tunables.memory_guard_size, }) } } diff --git a/winch/codegen/src/codegen/mod.rs b/winch/codegen/src/codegen/mod.rs index 8c735557f6e4..ecf83d39778b 100644 --- a/winch/codegen/src/codegen/mod.rs +++ b/winch/codegen/src/codegen/mod.rs @@ -711,7 +711,7 @@ where HeapStyle::Static { bound } if heap.ty == WasmValType::I32 && u64::from(u32::MAX) - <= u64::from(bound) + u64::from(heap.offset_guard_size) + <= u64::from(bound) + u64::from(self.tunables.memory_guard_size) - offset_with_access_size => { let addr = self.context.any_gpr(self.masm);