Skip to content

Commit

Permalink
Remove offset_guard_size from compiler Heap structs
Browse files Browse the repository at this point in the history
Push reading `Tunables` down further into where it's needed instead of
having duplicate storage per-heap.
  • Loading branch information
alexcrichton committed Nov 1, 2024
1 parent 628630b commit d2a19e0
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 17 deletions.
10 changes: 6 additions & 4 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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),
),
))
}
Expand Down Expand Up @@ -604,7 +605,7 @@ fn explicit_check_oob_condition_and_compute_addr<FE: FuncEnvironment + ?Sized>(
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(),
Expand Down
7 changes: 5 additions & 2 deletions crates/cranelift/src/translate/environ/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions crates/cranelift/src/translate/heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ pub struct HeapData {
/// Heap accesses larger than this will always trap.
pub max_size: Option<u64>,

/// 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,

Expand Down
3 changes: 0 additions & 3 deletions winch/codegen/src/codegen/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion winch/codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d2a19e0

Please sign in to comment.