More refactoring to remove MemoryStyle (#9543)

* Remove tuple return value of `MemorysStyle::for_memory`

The second option is always `tunables.memory_guard_size`, so propagate
this to the various locations reading it.

* 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.

* Plumb wasm memory types further down in Winch

This commit plumbs `wasmtime_environ::Memory` further down the stack in
Winch to where heaps are processed. This avoids an extra layer of
indirection through a `Heap` type which peels apart a `Memory` to pick
out a few fields. In the future this'll be used with more helpers on
`Memory` to simplify the static/dynamic memory cases.

* Plumb memory type further down in Cranelift

This commit removes the `HeapStyle` structure from Cranelift and instead
plumbs the `wasmtime_environ::Memory` type further down the stack
through in `HeapData` (same as Winch before this commit). This removes
redundant fields in `MemoryType` and continues to push down the
`MemoryStyle` structure even further.

This commit additionally and unconditionally defines a `GlobalValue` for
the heap limit of memory. This is unused most of the time for 32-bit
wasm and is conditionally used depending on how bounds checks are
generated. This is a small amount of bloat to each function since
previously functions that didn't need this `GlobalValue` elided it. A
future refactoring, however, will make it a bit more clear how this is
used even for "static" memories.

* Update test expectations

crates/cranelift/src/func_environ.rs

@@ -1,6 +1,6 @@
 use crate::translate::{
-    FuncEnvironment as _, FuncTranslationState, GlobalVariable, Heap, HeapData, HeapStyle,
-    StructFieldsVec, TableData, TableSize, TargetEnvironment,
+    FuncEnvironment as _, FuncTranslationState, GlobalVariable, Heap, HeapData, StructFieldsVec,
+    TableData, TableSize, TargetEnvironment,
 };
 use crate::{gc, BuiltinFunctionSignatures, TRAP_INTERNAL_ASSERT};
 use cranelift_codegen::cursor::FuncCursor;
@@ -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
@@ -2305,18 +2309,6 @@ impl<'module_environment> crate::translate::FuncEnvironment
         let memory = self.module.memories[index];
         let is_shared = memory.shared;
 
-        let min_size = memory.minimum_byte_size().unwrap_or_else(|_| {
-            // The only valid Wasm memory size that won't fit in a 64-bit
-            // integer is the maximum memory64 size (2^64) which is one
-            // larger than `u64::MAX` (2^64 - 1). In this case, just say the
-            // minimum heap size is `u64::MAX`.
-            debug_assert_eq!(memory.limits.min, 1 << 48);
-            debug_assert_eq!(memory.page_size(), 1 << 16);
-            u64::MAX
-        });
-
-        let max_size = memory.maximum_byte_size().ok();
-
         let (ptr, base_offset, current_length_offset, ptr_memtype) = {
             let vmctx = self.vmctx(func);
             if let Some(def_index) = self.module.defined_memory_index(index) {
@@ -2369,25 +2361,23 @@ impl<'module_environment> crate::translate::FuncEnvironment
             }
         };
 
-        let page_size_log2 = memory.page_size_log2;
+        let heap_bound = func.create_global_value(ir::GlobalValueData::Load {
+            base: ptr,
+            offset: Offset32::new(current_length_offset),
+            global_type: pointer_type,
+            flags: MemFlags::trusted(),
+        });
 
         // If we have a declared maximum, we can make this a "static" heap, which is
         // allocated up front and never moved.
-        let (style, offset_guard_size) = MemoryStyle::for_memory(memory, self.tunables);
-        let (heap_style, readonly_base, base_fact, memory_type) = match style {
+        let style = MemoryStyle::for_memory(memory, self.tunables);
+        let (readonly_base, base_fact, memory_type) = match style {
             MemoryStyle::Dynamic { .. } => {
-                let heap_bound = func.create_global_value(ir::GlobalValueData::Load {
-                    base: ptr,
-                    offset: Offset32::new(current_length_offset),
-                    global_type: pointer_type,
-                    flags: MemFlags::trusted(),
-                });
-
                 let (base_fact, data_mt) = if let Some(ptr_memtype) = ptr_memtype {
                     // 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);
@@ -2438,14 +2428,7 @@ impl<'module_environment> crate::translate::FuncEnvironment
                     (None, None)
                 };
 
-                (
-                    HeapStyle::Dynamic {
-                        bound_gv: heap_bound,
-                    },
-                    false,
-                    base_fact,
-                    data_mt,
-                )
+                (false, base_fact, data_mt)
             }
             MemoryStyle::Static {
                 byte_reservation: bound_bytes,
@@ -2454,7 +2437,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.
@@ -2493,12 +2476,7 @@ impl<'module_environment> crate::translate::FuncEnvironment
                 } else {
                     (None, None)
                 };
-                (
-                    HeapStyle::Static { bound: bound_bytes },
-                    true,
-                    base_fact,
-                    data_mt,
-                )
+                (true, base_fact, data_mt)
             }
         };
 
@@ -2516,13 +2494,9 @@ impl<'module_environment> crate::translate::FuncEnvironment
 
         Ok(self.heaps.push(HeapData {
             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,
-            page_size_log2,
+            bound: heap_bound,
+            pcc_memory_type: memory_type,
+            memory,
         }))
     }
 

crates/cranelift/src/translate/code_translator.rs

@@ -1254,7 +1254,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let effective_addr = if memarg.offset == 0 {
                 addr
             } else {
-                let index_type = environ.heaps()[heap].index_type;
+                let index_type = environ.heaps()[heap].index_type();
                 let offset = builder.ins().iconst(index_type, memarg.offset as i64);
                 environ.uadd_overflow_trap(builder, addr, offset, ir::TrapCode::HEAP_OUT_OF_BOUNDS)
             };
@@ -1278,7 +1278,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
             let effective_addr = if memarg.offset == 0 {
                 addr
             } else {
-                let index_type = environ.heaps()[heap].index_type;
+                let index_type = environ.heaps()[heap].index_type();
                 let offset = builder.ins().iconst(index_type, memarg.offset as i64);
                 environ.uadd_overflow_trap(builder, addr, offset, ir::TrapCode::HEAP_OUT_OF_BOUNDS)
             };
@@ -3222,7 +3222,9 @@ where
         // relatively odd/rare. In the future if needed we can look into
         // optimizing this more.
         Err(_) => {
-            let offset = builder.ins().iconst(heap.index_type, memarg.offset as i64);
+            let offset = builder
+                .ins()
+                .iconst(heap.index_type(), memarg.offset.signed());
             let adjusted_index = environ.uadd_overflow_trap(
                 builder,
                 index,
@@ -3251,7 +3253,7 @@ where
     let mut flags = MemFlags::new();
     flags.set_endianness(ir::Endianness::Little);
 
-    if heap.memory_type.is_some() {
+    if heap.pcc_memory_type.is_some() {
         // Proof-carrying code is enabled; check this memory access.
         flags.set_checked();
     }