Remove heaps from core Cranelift, push them into cranelift-wasm (#5386

) * cranelift-wasm: translate Wasm loads into lower-level CLIF operations Rather than using `heap_{load,store,addr}`. * cranelift: Remove the `heap_{addr,load,store}` instructions These are now legalized in the `cranelift-wasm` frontend. * cranelift: Remove the `ir::Heap` entity from CLIF * Port basic memory operation tests to .wat filetests * Remove test for verifying CLIF heaps * Remove `heap_addr` from replace_branching_instructions_and_cfg_predecessors.clif test * Remove `heap_addr` from readonly.clif test * Remove `heap_addr` from `table_addr.clif` test * Remove `heap_addr` from the simd-fvpromote_low.clif test * Remove `heap_addr` from simd-fvdemote.clif test * Remove `heap_addr` from the load-op-store.clif test * Remove the CLIF heap runtest * Remove `heap_addr` from the global_value.clif test * Remove `heap_addr` from fpromote.clif runtests * Remove `heap_addr` from fdemote.clif runtests * Remove `heap_addr` from memory.clif parser test * Remove `heap_addr` from reject_load_readonly.clif test * Remove `heap_addr` from reject_load_notrap.clif test * Remove `heap_addr` from load_readonly_notrap.clif test * Remove `static-heap-without-guard-pages.clif` test Will be subsumed when we port `make-heap-load-store-tests.sh` to generating `.wat` tests. * Remove `static-heap-with-guard-pages.clif` test Will be subsumed when we port `make-heap-load-store-tests.sh` over to `.wat` tests. * Remove more heap tests These will be subsumed by porting `make-heap-load-store-tests.sh` over to `.wat` tests. * Remove `heap_addr` from `simple-alias.clif` test * Remove `heap_addr` from partial-redundancy.clif test * Remove `heap_addr` from multiple-blocks.clif test * Remove `heap_addr` from fence.clif test * Remove `heap_addr` from extends.clif test * Remove runtests that rely on heaps Heaps are not a thing in CLIF or the interpreter anymore * Add generated load/store `.wat` tests * Enable memory-related wasm features in `.wat` tests * Remove CLIF heap from fcmp-mem-bug.clif test * Add a mode for compiling `.wat` all the way to assembly in filetests * Also generate WAT to assembly tests in `make-load-store-tests.sh` * cargo fmt * Reinstate `f{de,pro}mote.clif` tests without the heap bits * Remove undefined doc link * Remove outdated SVG and dot file from docs * Add docs about `None` returns for base address computation helpers * Factor out `env.heap_access_spectre_mitigation()` to a local * Expand docs for `FuncEnvironment::heaps` trait method * Restore f{de,pro}mote+load clif runtests with stack memory
bytecodealliance · Dec 15, 2022 · c0b587a · c0b587a
1 parent e03d65c
commit c0b587a
Show file tree

Hide file tree

Showing 198 changed files with 2,494 additions and 4,232 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml
@@ -35,6 +35,7 @@ sha2 = { version = "0.10.2", optional = true }
 
 [dev-dependencies]
 criterion = "0.3"
+similar = "2.1.0"
 
 [build-dependencies]
 cranelift-codegen-meta = { path = "meta", version = "0.92.0" }

diff --git a/cranelift/codegen/meta/src/cdsl/formats.rs b/cranelift/codegen/meta/src/cdsl/formats.rs
@@ -116,15 +116,6 @@ impl InstructionFormatBuilder {
         self
     }
 
-    pub fn imm_with_name(mut self, name: &'static str, operand_kind: &OperandKind) -> Self {
-        let field = FormatField {
-            kind: operand_kind.clone(),
-            member: name,
-        };
-        self.0.imm_fields.push(field);
-        self
-    }
-
     pub fn typevar_operand(mut self, operand_index: usize) -> Self {
         assert!(self.0.typevar_operand.is_none());
         assert!(operand_index < self.0.num_value_operands);

diff --git a/cranelift/codegen/meta/src/shared/entities.rs b/cranelift/codegen/meta/src/shared/entities.rs
@@ -35,9 +35,6 @@ pub(crate) struct EntityRefs {
     /// A reference to a jump table declared in the function preamble.
     pub(crate) jump_table: OperandKind,
 
-    /// A reference to a heap declared in the function preamble.
-    pub(crate) heap: OperandKind,
-
     /// A reference to a table declared in the function preamble.
     pub(crate) table: OperandKind,
 
@@ -69,8 +66,6 @@ impl EntityRefs {
 
             jump_table: new("table", "ir::JumpTable", "A jump table."),
 
-            heap: new("heap", "ir::Heap", "A heap."),
-
             table: new("table", "ir::Table", "A table."),
 
             varargs: OperandKind::new(

diff --git a/cranelift/codegen/meta/src/shared/formats.rs b/cranelift/codegen/meta/src/shared/formats.rs
@@ -15,9 +15,6 @@ pub(crate) struct Formats {
     pub(crate) cond_trap: Rc<InstructionFormat>,
     pub(crate) float_compare: Rc<InstructionFormat>,
     pub(crate) func_addr: Rc<InstructionFormat>,
-    pub(crate) heap_addr: Rc<InstructionFormat>,
-    pub(crate) heap_load: Rc<InstructionFormat>,
-    pub(crate) heap_store: Rc<InstructionFormat>,
     pub(crate) int_compare: Rc<InstructionFormat>,
     pub(crate) int_compare_imm: Rc<InstructionFormat>,
     pub(crate) int_add_trap: Rc<InstructionFormat>,
@@ -200,25 +197,6 @@ impl Formats {
                 .imm(&entities.dynamic_stack_slot)
                 .build(),
 
-            // Accessing a WebAssembly heap.
-            heap_addr: Builder::new("HeapAddr")
-                .imm(&entities.heap)
-                .value()
-                .imm_with_name("offset", &imm.uimm32)
-                .imm_with_name("size", &imm.uimm8)
-                .build(),
-
-            heap_load: Builder::new("HeapLoad").imm(&imm.heap_imm).value().build(),
-
-            heap_store: Builder::new("HeapStore")
-                // We have more fields for this instruction than
-                // `InstructionData` can hold without growing in size, so we
-                // push the immediates out into a side table.
-                .imm(&imm.heap_imm)
-                .value()
-                .value()
-                .build(),
-
             // Accessing a WebAssembly table.
             table_addr: Builder::new("TableAddr")
                 .imm(&entities.table)

diff --git a/cranelift/codegen/meta/src/shared/immediates.rs b/cranelift/codegen/meta/src/shared/immediates.rs
@@ -14,9 +14,6 @@ pub(crate) struct Immediates {
     /// counts on shift instructions.
     pub uimm8: OperandKind,
 
-    /// An unsigned 32-bit immediate integer operand.
-    pub uimm32: OperandKind,
-
     /// An unsigned 128-bit immediate integer operand.
     ///
     /// This operand is used to pass entire 128-bit vectors as immediates to instructions like
@@ -59,9 +56,6 @@ pub(crate) struct Immediates {
     /// Flags for memory operations like `load` and `store`.
     pub memflags: OperandKind,
 
-    /// A reference to out-of-line immediates for heap accesses.
-    pub heap_imm: OperandKind,
-
     /// A trap code indicating the reason for trapping.
     ///
     /// The Rust enum type also has a `User(u16)` variant for user-provided trap codes.
@@ -110,11 +104,6 @@ impl Immediates {
                 "ir::immediates::Uimm8",
                 "An 8-bit immediate unsigned integer.",
             ),
-            uimm32: new_imm(
-                "imm",
-                "ir::immediates::Uimm32",
-                "A 32-bit immediate unsigned integer.",
-            ),
             uimm128: new_imm(
                 "imm",
                 "ir::Immediate",
@@ -186,12 +175,6 @@ impl Immediates {
 
             memflags: new_imm("flags", "ir::MemFlags", "Memory operation flags"),
 
-            heap_imm: new_imm(
-                "heap_imm",
-                "ir::HeapImm",
-                "Reference to out-of-line heap access immediates",
-            ),
-
             trapcode: {
                 let mut trapcode_values = HashMap::new();
                 trapcode_values.insert("stk_ovf", "StackOverflow");

diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs
@@ -1118,89 +1118,6 @@ pub(crate) fn define(
         .operands_out(vec![a]),
     );
 
-    let HeapOffset = &TypeVar::new(
-        "HeapOffset",
-        "An unsigned heap offset",
-        TypeSetBuilder::new().ints(32..64).build(),
-    );
-
-    let H = &Operand::new("H", &entities.heap);
-    let index = &Operand::new("index", HeapOffset);
-    let Offset = &Operand::new("Offset", &imm.uimm32).with_doc("Static offset immediate in bytes");
-    let Size = &Operand::new("Size", &imm.uimm8).with_doc("Static size immediate in bytes");
-
-    ig.push(
-        Inst::new(
-            "heap_addr",
-            r#"
-        Bounds check and compute absolute address of ``index + Offset`` in heap memory.
-
-        Verify that the range ``index .. index + Offset + Size`` is in bounds for the
-        heap ``H``, and generate an absolute address that is safe to dereference.
-
-        1. If ``index + Offset + Size`` is less than or equal ot the heap bound, return an
-           absolute address corresponding to a byte offset of ``index + Offset`` from the
-           heap's base address.
-
-        2. If ``index + Offset + Size`` is greater than the heap bound, return the
-           ``NULL`` pointer or any other address that is guaranteed to generate a trap
-           when accessed.
-        "#,
-            &formats.heap_addr,
-        )
-        .operands_in(vec![H, index, Offset, Size])
-        .operands_out(vec![addr]),
-    );
-
-    let heap_imm = &Operand::new("heap_imm", &imm.heap_imm);
-    let index =
-        &Operand::new("index", HeapOffset).with_doc("Dynamic index (in bytes) into the heap");
-    let a = &Operand::new("a", Mem).with_doc("The value loaded from the heap");
-
-    ig.push(
-        Inst::new(
-            "heap_load",
-            r#"
-            Load a value from the given heap at address ``index + offset``,
-            trapping on out-of-bounds accesses.
-
-            Checks that ``index + offset .. index + offset + sizeof(a)`` is
-            within the heap's bounds, trapping if it is not. Otherwise, when
-            that range is in bounds, loads the value from the heap.
-
-            Traps on ``index + offset + sizeof(a)`` overflow.
-            "#,
-            &formats.heap_load,
-        )
-        .operands_in(vec![heap_imm, index])
-        .operands_out(vec![a])
-        .can_load(true)
-        .can_trap(true),
-    );
-
-    let a = &Operand::new("a", Mem).with_doc("The value stored into the heap");
-
-    ig.push(
-        Inst::new(
-            "heap_store",
-            r#"
-            Store ``a`` into the given heap at address ``index + offset``,
-            trapping on out-of-bounds accesses.
-
-            Checks that ``index + offset .. index + offset + sizeof(a)`` is
-            within the heap's bounds, trapping if it is not. Otherwise, when
-            that range is in bounds, stores the value into the heap.
-
-            Traps on ``index + offset + sizeof(a)`` overflow.
-            "#,
-            &formats.heap_store,
-        )
-        .operands_in(vec![heap_imm, index, a])
-        .operands_out(vec![])
-        .can_store(true)
-        .can_trap(true),
-    );
-
     // Note this instruction is marked as having other side-effects, so GVN won't try to hoist it,
     // which would result in it being subject to spilling. While not hoisting would generally hurt
     // performance, since a computed value used many times may need to be regenerated before each

diff --git a/cranelift/codegen/meta/src/shared/settings.rs b/cranelift/codegen/meta/src/shared/settings.rs
@@ -139,21 +139,6 @@ pub(crate) fn define() -> SettingGroup {
         false,
     );
 
-    settings.add_bool(
-        "use_pinned_reg_as_heap_base",
-        "Use the pinned register as the heap base.",
-        r#"
-            Enabling this requires the enable_pinned_reg setting to be set to true. It enables a custom
-            legalization of the `heap_addr` instruction so it will use the pinned register as the heap
-            base, instead of fetching it from a global value.
-
-            Warning! Enabling this means that the pinned register *must* be maintained to contain the
-            heap base address at all times, during the lifetime of a function. Using the pinned
-            register for other purposes when this is set is very likely to cause crashes.
-        "#,
-        false,
-    );
-
     settings.add_bool(
         "enable_simd",
         "Enable the use of SIMD instructions.",

diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs
@@ -4,12 +4,11 @@ use crate::entity::{self, PrimaryMap, SecondaryMap};
 use crate::ir;
 use crate::ir::builder::ReplaceBuilder;
 use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes};
-use crate::ir::immediates::HeapImmData;
 use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData};
 use crate::ir::{types, ConstantData, ConstantPool, Immediate};
 use crate::ir::{
-    Block, DynamicType, FuncRef, HeapImm, Inst, SigRef, Signature, Type, Value,
-    ValueLabelAssignments, ValueList, ValueListPool,
+    Block, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments,
+    ValueList, ValueListPool,
 };
 use crate::ir::{ExtFuncData, RelSourceLoc};
 use crate::packed_option::ReservedValue;
@@ -84,9 +83,6 @@ pub struct DataFlowGraph {
 
     /// Stores large immediates that otherwise will not fit on InstructionData
     pub immediates: PrimaryMap<Immediate, ConstantData>,
-
-    /// Out-of-line heap access immediates that don't fit in `InstructionData`.
-    pub heap_imms: PrimaryMap<HeapImm, HeapImmData>,
 }
 
 impl DataFlowGraph {
@@ -105,7 +101,6 @@ impl DataFlowGraph {
             values_labels: None,
             constants: ConstantPool::new(),
             immediates: PrimaryMap::new(),
-            heap_imms: PrimaryMap::new(),
         }
     }
 

diff --git a/cranelift/codegen/src/ir/entities.rs b/cranelift/codegen/src/ir/entities.rs
@@ -368,60 +368,6 @@ impl SigRef {
     }
 }
 
-/// An opaque reference to a [heap](https://en.wikipedia.org/wiki/Memory_management#DYNAMIC).
-///
-/// Heaps are used to access dynamically allocated memory through
-/// [`heap_addr`](super::InstBuilder::heap_addr).
-///
-/// To create a heap, use [`FunctionBuilder::create_heap`](https://docs.rs/cranelift-frontend/*/cranelift_frontend/struct.FunctionBuilder.html#method.create_heap).
-///
-/// While the order is stable, it is arbitrary.
-#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
-#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
-pub struct Heap(u32);
-entity_impl!(Heap, "heap");
-
-impl Heap {
-    /// Create a new heap reference from its number.
-    ///
-    /// This method is for use by the parser.
-    pub fn with_number(n: u32) -> Option<Self> {
-        if n < u32::MAX {
-            Some(Self(n))
-        } else {
-            None
-        }
-    }
-}
-
-/// An opaque reference to some out-of-line immediates for `heap_{load,store}`
-/// instructions.
-///
-/// These immediates are too large to store in
-/// [`InstructionData`](super::instructions::InstructionData) and therefore must
-/// be tracked separately in
-/// [`DataFlowGraph::heap_imms`](super::dfg::DataFlowGraph). `HeapImm` provides
-/// a way to reference values stored there.
-///
-/// While the order is stable, it is arbitrary.
-#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
-#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
-pub struct HeapImm(u32);
-entity_impl!(HeapImm, "heap_imm");
-
-impl HeapImm {
-    /// Create a new `HeapImm` reference from its number.
-    ///
-    /// This method is for use by the parser.
-    pub fn with_number(n: u32) -> Option<Self> {
-        if n < u32::MAX {
-            Some(Self(n))
-        } else {
-            None
-        }
-    }
-}
-
 /// An opaque reference to a [WebAssembly
 /// table](https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format#WebAssembly_tables).
 ///
@@ -477,8 +423,6 @@ pub enum AnyEntity {
     FuncRef(FuncRef),
     /// A function call signature.
     SigRef(SigRef),
-    /// A heap.
-    Heap(Heap),
     /// A table.
     Table(Table),
     /// A function's stack limit
@@ -500,7 +444,6 @@ impl fmt::Display for AnyEntity {
             Self::Constant(r) => r.fmt(f),
             Self::FuncRef(r) => r.fmt(f),
             Self::SigRef(r) => r.fmt(f),
-            Self::Heap(r) => r.fmt(f),
             Self::Table(r) => r.fmt(f),
             Self::StackLimit => write!(f, "stack_limit"),
         }
@@ -579,12 +522,6 @@ impl From<SigRef> for AnyEntity {
     }
 }
 
-impl From<Heap> for AnyEntity {
-    fn from(r: Heap) -> Self {
-        Self::Heap(r)
-    }
-}
-
 impl From<Table> for AnyEntity {
     fn from(r: Table) -> Self {
         Self::Table(r)