diff --git a/cranelift/docs/langref.rst b/cranelift/docs/langref.rst index fb62111d0a..fb716257bd 100644 --- a/cranelift/docs/langref.rst +++ b/cranelift/docs/langref.rst @@ -654,6 +654,9 @@ trap when accessed. address space reserved for the heap, not including the guard pages. :arg GuardBytes: Size of the guard pages in bytes. +When the base is a global variable, it must be :term:`accessible` and naturally +aligned for a pointer value. + The ``reserved_reg`` option is not yet implemented. Dynamic heaps @@ -674,6 +677,9 @@ is resized. The bound of a dynamic heap is stored in a global variable. :arg BoundGV: Global variable containing the current heap bound in bytes. :arg GuardBytes: Size of the guard pages in bytes. +When the base is a global variable, it must be :term:`accessible` and naturally +aligned for a pointer value. + The ``reserved_reg`` option is not yet implemented. Heap examples diff --git a/cranelift/filetests/isa/intel/legalize-memory.cton b/cranelift/filetests/isa/intel/legalize-memory.cton index 5b7246a2e3..1ba979deb1 100644 --- a/cranelift/filetests/isa/intel/legalize-memory.cton +++ b/cranelift/filetests/isa/intel/legalize-memory.cton @@ -23,7 +23,7 @@ function %deref(i64 vmctx) -> i64 { ebb1(v1: i64): v2 = global_addr.i64 gv2 ; check: $(a1=$V) = iadd_imm v1, -16 - ; check: $(p1=$V) = load.i64 $a1 + ; check: $(p1=$V) = load.i64 notrap aligned $a1 ; check: v2 = iadd_imm $p1, 32 return v2 ; check: return v2 @@ -55,7 +55,7 @@ ebb0(v0: i32, v999: i64): ; Checks here are assuming that no pipehole opts fold the load offsets. ; nextln: $(xoff=$V) = uextend.i64 v0 ; nextln: $(haddr=$V) = iadd_imm v999, 64 - ; nextln: $(hbase=$V) = load.i64 $haddr + ; nextln: $(hbase=$V) = load.i64 notrap aligned $haddr ; nextln: v1 = iadd $hbase, $xoff v2 = load.f32 v1+16 ; nextln: v2 = load.f32 v1+16 @@ -103,7 +103,7 @@ ebb0(v0: i32, v999: i64): ; Checks here are assuming that no pipehole opts fold the load offsets. ; nextln: $(xoff=$V) = uextend.i64 v0 ; nextln: $(haddr=$V) = iadd_imm.i64 v999, 64 - ; nextln: $(hbase=$V) = load.i64 $haddr + ; nextln: $(hbase=$V) = load.i64 notrap aligned $haddr ; nextln: v1 = iadd $hbase, $xoff v2 = load.f32 v1+0x7fff_ffff ; nextln: v2 = load.f32 v1+0x7fff_ffff diff --git a/lib/cretonne/meta/base/instructions.py b/lib/cretonne/meta/base/instructions.py index 8ba1aa65fa..c54f3ef570 100644 --- a/lib/cretonne/meta/base/instructions.py +++ b/lib/cretonne/meta/base/instructions.py @@ -588,6 +588,9 @@ stack_check = Instruction( Read the stack limit from ``GV`` and compare it to the stack pointer. If the stack pointer has reached or exceeded the limit, generate a trap with a ``stk_ovf`` code. + + The global variable must be accessible and naturally aligned for a + pointer-sized value. """, ins=GV, can_trap=True) diff --git a/lib/cretonne/src/ir/globalvar.rs b/lib/cretonne/src/ir/globalvar.rs index 78e1c6cb98..c8fc3b8ce1 100644 --- a/lib/cretonne/src/ir/globalvar.rs +++ b/lib/cretonne/src/ir/globalvar.rs @@ -17,7 +17,8 @@ pub enum GlobalVarData { /// Variable is part of a struct pointed to by another global variable. /// /// The `base` global variable is assumed to contain a pointer to a struct. This global - /// variable lives at an offset into the struct. + /// variable lives at an offset into the struct. The memory must be accessible, and + /// naturally aligned to hold a pointer value. Deref { /// The base pointer global variable. base: GlobalVar, diff --git a/lib/cretonne/src/ir/heap.rs b/lib/cretonne/src/ir/heap.rs index e2a5366a1a..b41565b25b 100644 --- a/lib/cretonne/src/ir/heap.rs +++ b/lib/cretonne/src/ir/heap.rs @@ -29,7 +29,8 @@ pub enum HeapBase { /// This feature is not yet implemented. ReservedReg, - /// The heap base is in a global variable. + /// The heap base is in a global variable. The variable must be accessible and naturally + /// aligned for a pointer. GlobalVar(GlobalVar), } @@ -38,7 +39,8 @@ pub enum HeapBase { pub enum HeapStyle { /// A dynamic heap can be relocated to a different base address when it is grown. Dynamic { - /// Global variable holding the current bound of the heap in bytes. + /// Global variable holding the current bound of the heap in bytes. It is + /// required to be accessible and naturally aligned for a pointer-sized integer. bound_gv: GlobalVar, }, diff --git a/lib/cretonne/src/legalizer/globalvar.rs b/lib/cretonne/src/legalizer/globalvar.rs index 5f6427822b..901b0da416 100644 --- a/lib/cretonne/src/legalizer/globalvar.rs +++ b/lib/cretonne/src/legalizer/globalvar.rs @@ -52,8 +52,11 @@ fn deref_addr(inst: ir::Inst, func: &mut ir::Function, base: ir::GlobalVar, offs pos.use_srcloc(inst); let base_addr = pos.ins().global_addr(ptr_ty, base); - // TODO: We could probably set both `notrap` and `aligned` on this load instruction. - let base_ptr = pos.ins().load(ptr_ty, ir::MemFlags::new(), base_addr, 0); + let mut mflags = ir::MemFlags::new(); + // Deref globals are required to be accessible and aligned. + mflags.set_notrap(); + mflags.set_aligned(); + let base_ptr = pos.ins().load(ptr_ty, mflags, base_addr, 0); pos.func.dfg.replace(inst).iadd_imm(base_ptr, offset); } diff --git a/lib/cretonne/src/legalizer/heap.rs b/lib/cretonne/src/legalizer/heap.rs index 37ce226351..0d6e7d385b 100644 --- a/lib/cretonne/src/legalizer/heap.rs +++ b/lib/cretonne/src/legalizer/heap.rs @@ -58,7 +58,11 @@ fn dynamic_addr( // Start with the bounds check. Trap if `offset + size > bound`. let bound_addr = pos.ins().global_addr(addr_ty, bound_gv); - let bound = pos.ins().load(offset_ty, MemFlags::new(), bound_addr, 0); + let mut mflags = MemFlags::new(); + // The bound variable is requied to be accessible and aligned. + mflags.set_notrap(); + mflags.set_aligned(); + let bound = pos.ins().load(offset_ty, mflags, bound_addr, 0); let oob; if size == 1 { @@ -175,7 +179,11 @@ fn offset_addr( ir::HeapBase::ReservedReg => unimplemented!(), ir::HeapBase::GlobalVar(base_gv) => { let base_addr = pos.ins().global_addr(addr_ty, base_gv); - let base = pos.ins().load(addr_ty, MemFlags::new(), base_addr, 0); + let mut mflags = MemFlags::new(); + // The base address variable is requied to be accessible and aligned. + mflags.set_notrap(); + mflags.set_aligned(); + let base = pos.ins().load(addr_ty, mflags, base_addr, 0); pos.func.dfg.replace(inst).iadd(base, offset); } } diff --git a/lib/wasm/src/code_translator.rs b/lib/wasm/src/code_translator.rs index 42d232e6f5..69111dc8e8 100644 --- a/lib/wasm/src/code_translator.rs +++ b/lib/wasm/src/code_translator.rs @@ -75,8 +75,9 @@ pub fn translate_operator( GlobalValue::Const(val) => val, GlobalValue::Memory { gv, ty } => { let addr = builder.ins().global_addr(environ.native_pointer(), gv); - // TODO: It is likely safe to set `aligned notrap` flags on a global load. - let flags = ir::MemFlags::new(); + let mut flags = ir::MemFlags::new(); + flags.set_notrap(); + flags.set_aligned(); builder.ins().load(ty, flags, addr, 0) } }; @@ -87,8 +88,9 @@ pub fn translate_operator( GlobalValue::Const(_) => panic!("global #{} is a constant", global_index), GlobalValue::Memory { gv, .. } => { let addr = builder.ins().global_addr(environ.native_pointer(), gv); - // TODO: It is likely safe to set `aligned notrap` flags on a global store. - let flags = ir::MemFlags::new(); + let mut flags = ir::MemFlags::new(); + flags.set_notrap(); + flags.set_aligned(); let val = state.pop1(); builder.ins().store(flags, val, addr, 0); } @@ -992,6 +994,9 @@ fn translate_load( // We don't yet support multiple linear memories. let heap = state.get_heap(builder.func, 0, environ); let (base, offset) = get_heap_addr(heap, addr32, offset, environ.native_pointer(), builder); + // Note that we don't set `is_aligned` here, even if the load instruction's + // alignment immediate says it's aligned, because WebAssembly's immediate + // field is just a hint, while Cretonne's aligned flag needs a guarantee. let flags = MemFlags::new(); let (load, dfg) = builder.ins().Load( opcode, @@ -1017,6 +1022,7 @@ fn translate_store( // We don't yet support multiple linear memories. let heap = state.get_heap(builder.func, 0, environ); let (base, offset) = get_heap_addr(heap, addr32, offset, environ.native_pointer(), builder); + // See the comments in `translate_load` about the flags. let flags = MemFlags::new(); builder.ins().Store( opcode, diff --git a/lib/wasm/src/environ/dummy.rs b/lib/wasm/src/environ/dummy.rs index c99afd32d9..bb21b7536a 100644 --- a/lib/wasm/src/environ/dummy.rs +++ b/lib/wasm/src/environ/dummy.rs @@ -209,7 +209,10 @@ impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environ let ext = pos.ins().uextend(I64, callee); pos.ins().imul_imm(ext, 4) }; - let func_ptr = pos.ins().load(ptr, ir::MemFlags::new(), callee_offset, 0); + let mut mflags = ir::MemFlags::new(); + mflags.set_notrap(); + mflags.set_aligned(); + let func_ptr = pos.ins().load(ptr, mflags, callee_offset, 0); // Build a value list for the indirect call instruction containing the callee, call_args, // and the vmctx parameter.