Skip to content

Commit d0d3245

Browse files
authored
Cranelift: Add heap_load and heap_store instructions (#5300)
* Cranelift: Define `heap_load` and `heap_store` instructions * Cranelift: Implement interpreter support for `heap_load` and `heap_store` * Cranelift: Add a suite runtests for `heap_{load,store}` There are so many knobs we can twist for heaps and I wanted to exhaustively test all of them, so I wrote a script to generate the tests. I've checked in the script in case we want to make any changes in the future, but I don't think it is worth adding this to CI to check that scripts are up to date or anything like that. * Review feedback
1 parent b305f25 commit d0d3245

24 files changed

+693
-13
lines changed

cranelift/codegen/meta/src/shared/formats.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ pub(crate) struct Formats {
1616
pub(crate) float_compare: Rc<InstructionFormat>,
1717
pub(crate) func_addr: Rc<InstructionFormat>,
1818
pub(crate) heap_addr: Rc<InstructionFormat>,
19+
pub(crate) heap_load: Rc<InstructionFormat>,
20+
pub(crate) heap_store: Rc<InstructionFormat>,
1921
pub(crate) int_compare: Rc<InstructionFormat>,
2022
pub(crate) int_compare_imm: Rc<InstructionFormat>,
2123
pub(crate) int_add_trap: Rc<InstructionFormat>,
@@ -206,6 +208,17 @@ impl Formats {
206208
.imm_with_name("size", &imm.uimm8)
207209
.build(),
208210

211+
heap_load: Builder::new("HeapLoad").imm(&imm.heap_imm).value().build(),
212+
213+
heap_store: Builder::new("HeapStore")
214+
// We have more fields for this instruction than
215+
// `InstructionData` can hold without growing in size, so we
216+
// push the immediates out into a side table.
217+
.imm(&imm.heap_imm)
218+
.value()
219+
.value()
220+
.build(),
221+
209222
// Accessing a WebAssembly table.
210223
table_addr: Builder::new("TableAddr")
211224
.imm(&entities.table)

cranelift/codegen/meta/src/shared/immediates.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ pub(crate) struct Immediates {
5959
/// Flags for memory operations like `load` and `store`.
6060
pub memflags: OperandKind,
6161

62+
/// A reference to out-of-line immediates for heap accesses.
63+
pub heap_imm: OperandKind,
64+
6265
/// A trap code indicating the reason for trapping.
6366
///
6467
/// The Rust enum type also has a `User(u16)` variant for user-provided trap codes.
@@ -182,6 +185,13 @@ impl Immediates {
182185
},
183186

184187
memflags: new_imm("flags", "ir::MemFlags", "Memory operation flags"),
188+
189+
heap_imm: new_imm(
190+
"heap_imm",
191+
"ir::HeapImm",
192+
"Reference to out-of-line heap access immediates",
193+
),
194+
185195
trapcode: {
186196
let mut trapcode_values = HashMap::new();
187197
trapcode_values.insert("stk_ovf", "StackOverflow");

cranelift/codegen/meta/src/shared/instructions.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,55 @@ pub(crate) fn define(
11551155
.operands_out(vec![addr]),
11561156
);
11571157

1158+
let heap_imm = &Operand::new("heap_imm", &imm.heap_imm);
1159+
let index =
1160+
&Operand::new("index", HeapOffset).with_doc("Dynamic index (in bytes) into the heap");
1161+
let a = &Operand::new("a", Mem).with_doc("The value loaded from the heap");
1162+
1163+
ig.push(
1164+
Inst::new(
1165+
"heap_load",
1166+
r#"
1167+
Load a value from the given heap at address ``index + offset``,
1168+
trapping on out-of-bounds accesses.
1169+
1170+
Checks that ``index + offset .. index + offset + sizeof(a)`` is
1171+
within the heap's bounds, trapping if it is not. Otherwise, when
1172+
that range is in bounds, loads the value from the heap.
1173+
1174+
Traps on ``index + offset + sizeof(a)`` overflow.
1175+
"#,
1176+
&formats.heap_load,
1177+
)
1178+
.operands_in(vec![heap_imm, index])
1179+
.operands_out(vec![a])
1180+
.can_load(true)
1181+
.can_trap(true),
1182+
);
1183+
1184+
let a = &Operand::new("a", Mem).with_doc("The value stored into the heap");
1185+
1186+
ig.push(
1187+
Inst::new(
1188+
"heap_store",
1189+
r#"
1190+
Store ``a`` into the given heap at address ``index + offset``,
1191+
trapping on out-of-bounds accesses.
1192+
1193+
Checks that ``index + offset .. index + offset + sizeof(a)`` is
1194+
within the heap's bounds, trapping if it is not. Otherwise, when
1195+
that range is in bounds, stores the value into the heap.
1196+
1197+
Traps on ``index + offset + sizeof(a)`` overflow.
1198+
"#,
1199+
&formats.heap_store,
1200+
)
1201+
.operands_in(vec![heap_imm, index, a])
1202+
.operands_out(vec![])
1203+
.can_store(true)
1204+
.can_trap(true),
1205+
);
1206+
11581207
// Note this instruction is marked as having other side-effects, so GVN won't try to hoist it,
11591208
// which would result in it being subject to spilling. While not hoisting would generally hurt
11601209
// performance, since a computed value used many times may need to be regenerated before each

cranelift/codegen/src/ir/dfg.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ use crate::entity::{self, PrimaryMap, SecondaryMap};
44
use crate::ir;
55
use crate::ir::builder::ReplaceBuilder;
66
use crate::ir::dynamic_type::{DynamicTypeData, DynamicTypes};
7+
use crate::ir::immediates::HeapImmData;
78
use crate::ir::instructions::{BranchInfo, CallInfo, InstructionData};
89
use crate::ir::{types, ConstantData, ConstantPool, Immediate};
910
use crate::ir::{
10-
Block, DynamicType, FuncRef, Inst, SigRef, Signature, Type, Value, ValueLabelAssignments,
11-
ValueList, ValueListPool,
11+
Block, DynamicType, FuncRef, HeapImm, Inst, SigRef, Signature, Type, Value,
12+
ValueLabelAssignments, ValueList, ValueListPool,
1213
};
1314
use crate::ir::{ExtFuncData, RelSourceLoc};
1415
use crate::packed_option::ReservedValue;
@@ -83,6 +84,9 @@ pub struct DataFlowGraph {
8384

8485
/// Stores large immediates that otherwise will not fit on InstructionData
8586
pub immediates: PrimaryMap<Immediate, ConstantData>,
87+
88+
/// Out-of-line heap access immediates that don't fit in `InstructionData`.
89+
pub heap_imms: PrimaryMap<HeapImm, HeapImmData>,
8690
}
8791

8892
impl DataFlowGraph {
@@ -101,6 +105,7 @@ impl DataFlowGraph {
101105
values_labels: None,
102106
constants: ConstantPool::new(),
103107
immediates: PrimaryMap::new(),
108+
heap_imms: PrimaryMap::new(),
104109
}
105110
}
106111

cranelift/codegen/src/ir/entities.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,34 @@ impl Heap {
394394
}
395395
}
396396

397+
/// An opaque reference to some out-of-line immediates for `heap_{load,store}`
398+
/// instructions.
399+
///
400+
/// These immediates are too large to store in
401+
/// [`InstructionData`](super::instructions::InstructionData) and therefore must
402+
/// be tracked separately in
403+
/// [`DataFlowGraph::heap_imms`](super::dfg::DataFlowGraph). `HeapImm` provides
404+
/// a way to reference values stored there.
405+
///
406+
/// While the order is stable, it is arbitrary.
407+
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
408+
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
409+
pub struct HeapImm(u32);
410+
entity_impl!(HeapImm, "heap_imm");
411+
412+
impl HeapImm {
413+
/// Create a new `HeapImm` reference from its number.
414+
///
415+
/// This method is for use by the parser.
416+
pub fn with_number(n: u32) -> Option<Self> {
417+
if n < u32::MAX {
418+
Some(Self(n))
419+
} else {
420+
None
421+
}
422+
}
423+
}
424+
397425
/// An opaque reference to a [WebAssembly
398426
/// table](https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format#WebAssembly_tables).
399427
///

cranelift/codegen/src/ir/immediates.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! Each type here should have a corresponding definition in the
55
//! `cranelift-codegen/meta/src/shared/immediates` crate in the meta language.
66
7+
use crate::ir;
78
use alloc::vec::Vec;
89
use core::cmp::Ordering;
910
use core::convert::TryFrom;
@@ -1177,6 +1178,18 @@ impl Not for Ieee64 {
11771178
}
11781179
}
11791180

1181+
/// Out-of-line heap access immediates.
1182+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
1183+
#[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
1184+
pub struct HeapImmData {
1185+
/// The memory flags for the heap access.
1186+
pub flags: ir::MemFlags,
1187+
/// The heap being accessed.
1188+
pub heap: ir::Heap,
1189+
/// The static offset added to the heap access's index.
1190+
pub offset: Uimm32,
1191+
}
1192+
11801193
#[cfg(test)]
11811194
mod tests {
11821195
use super::*;

cranelift/codegen/src/ir/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ pub use crate::ir::constant::{ConstantData, ConstantPool};
3737
pub use crate::ir::dfg::{DataFlowGraph, ValueDef};
3838
pub use crate::ir::dynamic_type::{dynamic_to_fixed, DynamicTypeData, DynamicTypes};
3939
pub use crate::ir::entities::{
40-
Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Heap, Immediate, Inst,
41-
JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value,
40+
Block, Constant, DynamicStackSlot, DynamicType, FuncRef, GlobalValue, Heap, HeapImm, Immediate,
41+
Inst, JumpTable, SigRef, StackSlot, Table, UserExternalNameRef, Value,
4242
};
4343
pub use crate::ir::extfunc::{
4444
AbiParam, ArgumentExtension, ArgumentPurpose, ExtFuncData, Signature,

cranelift/codegen/src/isa/aarch64/lower_inst.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ pub(crate) fn lower_insn_to_regs(
139139
panic!("Direct stack memory access not supported; should not be used by Wasm");
140140
}
141141

142-
Opcode::HeapAddr => {
143-
panic!("heap_addr should have been removed by legalization!");
142+
Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => {
143+
panic!("heap access instructions should have been removed by legalization!");
144144
}
145145

146146
Opcode::TableAddr => {

cranelift/codegen/src/isa/s390x/lower.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ impl LowerBackend for S390xBackend {
212212
Opcode::StackLoad | Opcode::StackStore => {
213213
panic!("Direct stack memory access not supported; should not be used by Wasm");
214214
}
215-
Opcode::HeapAddr => {
216-
panic!("heap_addr should have been removed by legalization!");
215+
Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => {
216+
panic!("heap access instructions should have been removed by legalization!");
217217
}
218218
Opcode::TableAddr => {
219219
panic!("table_addr should have been removed by legalization!");

cranelift/codegen/src/isa/x64/lower.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,8 @@ fn lower_insn_to_regs(
549549
panic!("global_value should have been removed by legalization!");
550550
}
551551

552-
Opcode::HeapAddr => {
553-
panic!("heap_addr should have been removed by legalization!");
552+
Opcode::HeapLoad | Opcode::HeapStore | Opcode::HeapAddr => {
553+
panic!("heap access instructions should have been removed by legalization!");
554554
}
555555

556556
Opcode::TableAddr => {

0 commit comments

Comments
 (0)