From 6d5f169b075dc17cf01dd5beb4b4cbbd748acfb2 Mon Sep 17 00:00:00 2001 From: Jiyong Ha Date: Wed, 13 Jan 2021 10:25:21 +0900 Subject: [PATCH 1/6] refactor: Optimize local stack initialization assembly --- lib/compiler-singlepass/src/emitter_x64.rs | 4 +++ lib/compiler-singlepass/src/machine.rs | 29 ++++++++++++++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/compiler-singlepass/src/emitter_x64.rs b/lib/compiler-singlepass/src/emitter_x64.rs index a4c0ceecdda..17e1818d6c1 100644 --- a/lib/compiler-singlepass/src/emitter_x64.rs +++ b/lib/compiler-singlepass/src/emitter_x64.rs @@ -117,6 +117,7 @@ pub trait Emitter { fn emit_xchg(&mut self, sz: Size, src: Location, dst: Location); fn emit_lock_xadd(&mut self, sz: Size, src: Location, dst: Location); fn emit_lock_cmpxchg(&mut self, sz: Size, src: Location, dst: Location); + fn emit_rep_stosq(&mut self); fn emit_btc_gpr_imm8_32(&mut self, src: u8, dst: GPR); fn emit_btc_gpr_imm8_64(&mut self, src: u8, dst: GPR); @@ -1176,6 +1177,9 @@ impl Emitter for Assembler { } } + fn emit_rep_stosq(&mut self) { + dynasm!(self ; rep stosq); + } fn emit_btc_gpr_imm8_32(&mut self, src: u8, dst: GPR) { dynasm!(self ; btc Rd(dst as u8), BYTE src as i8); } diff --git a/lib/compiler-singlepass/src/machine.rs b/lib/compiler-singlepass/src/machine.rs index 17374f8184b..0d8b4679a2b 100644 --- a/lib/compiler-singlepass/src/machine.rs +++ b/lib/compiler-singlepass/src/machine.rs @@ -5,6 +5,7 @@ use smallvec::smallvec; use smallvec::SmallVec; use std::collections::HashSet; use wasmer_compiler::wasmparser::Type as WpType; +use std::cmp; struct MachineStackOffset(usize); @@ -447,17 +448,35 @@ impl Machine { } } - // Initialize all normal locals to zero. - for i in n_params..n { - a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); - } - // Load vmctx into R15. a.emit_mov( Size::S64, Self::get_param_location(0), Location::GPR(GPR::R15), ); + + //Initialize all normal locals to zero. + let mut init_stack_loc_cnt = 0; + let mut last_stack_loc = Location::Memory(GPR::RBP, i32::MAX); + for i in n_params..n { + match locations[i] { + Location::Memory(_, _) => { + init_stack_loc_cnt+=1; + last_stack_loc = cmp::min(last_stack_loc, locations[i]); + }, + Location::GPR(_) => { + a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); + }, + _ => unreachable!(), + } + } + if init_stack_loc_cnt > 0 { + // Since this assemblies takes up 24 bytes, If initialize more than 2 slots, These assemblies are smallar. + a.emit_mov(Size::S64, Location::Imm64(init_stack_loc_cnt as u64), Location::GPR(GPR::RCX)); + a.emit_xor(Size::S64, Location::GPR(GPR::RAX), Location::GPR(GPR::RAX)); + a.emit_lea(Size::S64, last_stack_loc, Location::GPR(GPR::RDI)); + a.emit_rep_stosq(); + } // Add the size of all locals allocated to stack. self.stack_offset.0 += static_area_size - callee_saved_regs_size; From 71f4e15ead06c1fc2115427b167f2aed4fc9729c Mon Sep 17 00:00:00 2001 From: Jiyong Ha Date: Wed, 13 Jan 2021 10:44:24 +0900 Subject: [PATCH 2/6] test: Add skip_stack_guard_page to ignores.txt Since the direction of rep stosq is in reverse order (DF=0), The trap handler is judged as a heap oob exception based on the start address. --- tests/ignores.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ignores.txt b/tests/ignores.txt index 2b3fe69d10a..0195bf2e9dd 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,6 +1,7 @@ # Compilers singlepass::spec::multi_value singlepass::spec::simd +singlepass::spec::skip_stack_guard_page ## SIMD in Cranelift 0.67 has a small bug cranelift::spec::simd::simd_f64x2_arith From 87f6ea945a7a7d592c1af092b51fc8edf7885f80 Mon Sep 17 00:00:00 2001 From: losfair Date: Wed, 13 Jan 2021 20:12:52 +0800 Subject: [PATCH 3/6] Add stack probe. --- lib/compiler-singlepass/src/machine.rs | 10 ++++++++++ tests/ignores.txt | 1 - 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/compiler-singlepass/src/machine.rs b/lib/compiler-singlepass/src/machine.rs index 0d8b4679a2b..0f281392c53 100644 --- a/lib/compiler-singlepass/src/machine.rs +++ b/lib/compiler-singlepass/src/machine.rs @@ -7,6 +7,8 @@ use std::collections::HashSet; use wasmer_compiler::wasmparser::Type as WpType; use std::cmp; +const NATIVE_PAGE_SIZE: usize = 4096; + struct MachineStackOffset(usize); pub struct Machine { @@ -454,6 +456,14 @@ impl Machine { Self::get_param_location(0), Location::GPR(GPR::R15), ); + + // Stack probe. + // + // `rep stosq` writes data from low address to high address and may skip the stack guard page. + // so here we probe it explicitly when needed. + for i in (n_params..n).step_by(NATIVE_PAGE_SIZE / 8).skip(1) { + a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); + } //Initialize all normal locals to zero. let mut init_stack_loc_cnt = 0; diff --git a/tests/ignores.txt b/tests/ignores.txt index 0195bf2e9dd..2b3fe69d10a 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -1,7 +1,6 @@ # Compilers singlepass::spec::multi_value singlepass::spec::simd -singlepass::spec::skip_stack_guard_page ## SIMD in Cranelift 0.67 has a small bug cranelift::spec::simd::simd_f64x2_arith From d5b6560bd95f3f8db58f761419aa82b49e72fa66 Mon Sep 17 00:00:00 2001 From: losfair Date: Wed, 13 Jan 2021 20:14:05 +0800 Subject: [PATCH 4/6] Run cargo fmt. --- lib/compiler-singlepass/src/machine.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/compiler-singlepass/src/machine.rs b/lib/compiler-singlepass/src/machine.rs index 0f281392c53..7c9d7e7ab33 100644 --- a/lib/compiler-singlepass/src/machine.rs +++ b/lib/compiler-singlepass/src/machine.rs @@ -3,9 +3,9 @@ use crate::emitter_x64::*; use crate::x64_decl::{new_machine_state, X64Register}; use smallvec::smallvec; use smallvec::SmallVec; +use std::cmp; use std::collections::HashSet; use wasmer_compiler::wasmparser::Type as WpType; -use std::cmp; const NATIVE_PAGE_SIZE: usize = 4096; @@ -458,31 +458,35 @@ impl Machine { ); // Stack probe. - // + // // `rep stosq` writes data from low address to high address and may skip the stack guard page. // so here we probe it explicitly when needed. - for i in (n_params..n).step_by(NATIVE_PAGE_SIZE / 8).skip(1) { + for i in (n_params..n).step_by(NATIVE_PAGE_SIZE / 8).skip(1) { a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); } - + //Initialize all normal locals to zero. let mut init_stack_loc_cnt = 0; let mut last_stack_loc = Location::Memory(GPR::RBP, i32::MAX); for i in n_params..n { match locations[i] { Location::Memory(_, _) => { - init_stack_loc_cnt+=1; + init_stack_loc_cnt += 1; last_stack_loc = cmp::min(last_stack_loc, locations[i]); - }, + } Location::GPR(_) => { a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); - }, + } _ => unreachable!(), } - } + } if init_stack_loc_cnt > 0 { // Since this assemblies takes up 24 bytes, If initialize more than 2 slots, These assemblies are smallar. - a.emit_mov(Size::S64, Location::Imm64(init_stack_loc_cnt as u64), Location::GPR(GPR::RCX)); + a.emit_mov( + Size::S64, + Location::Imm64(init_stack_loc_cnt as u64), + Location::GPR(GPR::RCX), + ); a.emit_xor(Size::S64, Location::GPR(GPR::RAX), Location::GPR(GPR::RAX)); a.emit_lea(Size::S64, last_stack_loc, Location::GPR(GPR::RDI)); a.emit_rep_stosq(); From 60d61a6469eee807734f4590c88c3a4b60f1168b Mon Sep 17 00:00:00 2001 From: Jiyong Ha <70933233+brew0722@users.noreply.github.com> Date: Thu, 14 Jan 2021 17:39:23 +0900 Subject: [PATCH 5/6] Update lib/compiler-singlepass/src/machine.rs Co-authored-by: Ivan Enderlin --- lib/compiler-singlepass/src/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compiler-singlepass/src/machine.rs b/lib/compiler-singlepass/src/machine.rs index 7c9d7e7ab33..6a519d0f68b 100644 --- a/lib/compiler-singlepass/src/machine.rs +++ b/lib/compiler-singlepass/src/machine.rs @@ -465,7 +465,7 @@ impl Machine { a.emit_mov(Size::S64, Location::Imm32(0), locations[i]); } - //Initialize all normal locals to zero. + // Initialize all normal locals to zero. let mut init_stack_loc_cnt = 0; let mut last_stack_loc = Location::Memory(GPR::RBP, i32::MAX); for i in n_params..n { From 7121ebe3b5892ca9d878a169b8e691d6d64dc07e Mon Sep 17 00:00:00 2001 From: Jiyong Ha <70933233+brew0722@users.noreply.github.com> Date: Thu, 14 Jan 2021 17:39:30 +0900 Subject: [PATCH 6/6] Update lib/compiler-singlepass/src/machine.rs Co-authored-by: Ivan Enderlin --- lib/compiler-singlepass/src/machine.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compiler-singlepass/src/machine.rs b/lib/compiler-singlepass/src/machine.rs index 6a519d0f68b..7f446744cd7 100644 --- a/lib/compiler-singlepass/src/machine.rs +++ b/lib/compiler-singlepass/src/machine.rs @@ -481,7 +481,7 @@ impl Machine { } } if init_stack_loc_cnt > 0 { - // Since this assemblies takes up 24 bytes, If initialize more than 2 slots, These assemblies are smallar. + // Since these assemblies take up to 24 bytes, if more than 2 slots are initialized, then they are smaller. a.emit_mov( Size::S64, Location::Imm64(init_stack_loc_cnt as u64),