Bug 1626967: Provide Cranelift the size of wasm::Frame from a StaticEnvironment value; r=rhunt
authorBenjamin Bouvier <benj@benj.me>
Tue, 07 Apr 2020 10:37:32 +0000
changeset 522560 985eea91c36c76c8ebdb28183251b3d61291a52b
parent 522559 837dab8ce31cb96546ca90b5703331644961ce94
child 522561 4f1a62010e1f954cb1ff353eb241fc753c8b6a1b
push id37292
push userccoroiu@mozilla.com
push dateTue, 07 Apr 2020 16:05:40 +0000
treeherdermozilla-central@6c6c67870c37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhunt
bugs1626967
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1626967: Provide Cranelift the size of wasm::Frame from a StaticEnvironment value; r=rhunt This avoids one platform-specific number (3 for x86_64, 4 for arm64). Differential Revision: https://phabricator.services.mozilla.com/D69396
js/src/wasm/WasmCraneliftCompile.cpp
js/src/wasm/cranelift/baldrapi.h
js/src/wasm/cranelift/src/compile.rs
js/src/wasm/cranelift/src/isa.rs
js/src/wasm/cranelift/src/wasm2clif.rs
--- a/js/src/wasm/WasmCraneliftCompile.cpp
+++ b/js/src/wasm/WasmCraneliftCompile.cpp
@@ -359,24 +359,26 @@ CraneliftStaticEnvironment::CraneliftSta
       static_memory_bound(0),
       memory_guard_size(0),
       memory_base_tls_offset(offsetof(TlsData, memoryBase)),
       instance_tls_offset(offsetof(TlsData, instance)),
       interrupt_tls_offset(offsetof(TlsData, interrupt)),
       cx_tls_offset(offsetof(TlsData, cx)),
       realm_cx_offset(JSContext::offsetOfRealm()),
       realm_tls_offset(offsetof(TlsData, realm)),
-      realm_func_import_tls_offset(offsetof(FuncImportTls, realm)) {
+      realm_func_import_tls_offset(offsetof(FuncImportTls, realm)),
+      size_of_wasm_frame(sizeof(wasm::Frame)) {
 }
 
 // Most of BaldrMonkey's data structures refer to a "global offset" which is a
 // byte offset into the `globalArea` field of the  `TlsData` struct.
 //
 // Cranelift represents global variables with their byte offset from the "VM
-// context pointer" which is the `WasmTlsReg` pointing to the `TlsData` struct.
+// context pointer" which is the `WasmTlsReg` pointing to the `TlsData`
+// struct.
 //
 // This function translates between the two.
 
 static size_t globalToTlsOffset(size_t globalOffset) {
   return offsetof(wasm::TlsData, globalArea) + globalOffset;
 }
 
 CraneliftModuleEnvironment::CraneliftModuleEnvironment(
@@ -509,18 +511,18 @@ bool wasm::CraneliftCompileFunctions(con
     uint32_t totalCodeSize = masm.currentOffset();
     uint8_t* codeBuf = (uint8_t*)js_malloc(totalCodeSize);
     if (codeBuf) {
       masm.executableCopy(codeBuf);
 
       const CodeRangeVector& codeRanges = code->codeRanges;
       MOZ_ASSERT(codeRanges.length() >= inputs.length());
 
-      // Within the current batch, functions' code ranges have been added in the
-      // same order as the inputs.
+      // Within the current batch, functions' code ranges have been added in
+      // the same order as the inputs.
       size_t firstCodeRangeIndex = codeRanges.length() - inputs.length();
 
       for (size_t i = 0; i < inputs.length(); i++) {
         int funcIndex = inputs[i].index;
         mozilla::Unused << funcIndex;
 
         JitSpew(JitSpew_Codegen, "# ========================================");
         JitSpew(JitSpew_Codegen, "# Start of wasm cranelift code for index %d",
--- a/js/src/wasm/cranelift/baldrapi.h
+++ b/js/src/wasm/cranelift/baldrapi.h
@@ -72,16 +72,17 @@ struct CraneliftStaticEnvironment {
   size_t memory_guard_size;
   size_t memory_base_tls_offset;
   size_t instance_tls_offset;
   size_t interrupt_tls_offset;
   size_t cx_tls_offset;
   size_t realm_cx_offset;
   size_t realm_tls_offset;
   size_t realm_func_import_tls_offset;
+  size_t size_of_wasm_frame;
 
   // Not bindgen'd because it's inlined.
   inline CraneliftStaticEnvironment();
 };
 
 // This structure proxies the C++ ModuleEnvironment and the information it
 // contains.
 
--- a/js/src/wasm/cranelift/src/compile.rs
+++ b/js/src/wasm/cranelift/src/compile.rs
@@ -30,19 +30,19 @@ use cranelift_codegen::ir::{
     TrapCode,
 };
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::CodegenResult;
 use cranelift_codegen::Context;
 use cranelift_wasm::{FuncIndex, FuncTranslator, ModuleTranslationState, WasmResult};
 
 use crate::bindings;
-use crate::isa::make_isa;
+use crate::isa::{make_isa, POINTER_SIZE};
 use crate::utils::DashResult;
-use crate::wasm2clif::{init_sig, TransEnv, POINTER_SIZE, TRAP_THROW_REPORTED};
+use crate::wasm2clif::{init_sig, TransEnv, TRAP_THROW_REPORTED};
 
 // Namespace for user-defined functions.
 const USER_FUNCTION_NAMESPACE: u32 = 0;
 
 // Namespace for builtins functions that are translated to symbolic accesses in Spidermonkey.
 const SYMBOLIC_FUNCTION_NAMESPACE: u32 = 1;
 
 /// The result of a function's compilation: code + metadata.
--- a/js/src/wasm/cranelift/src/isa.rs
+++ b/js/src/wasm/cranelift/src/isa.rs
@@ -24,16 +24,21 @@
 use std::env;
 
 use cranelift_codegen::isa;
 use cranelift_codegen::settings::{self, Configurable};
 
 use crate::bindings::StaticEnvironment;
 use crate::utils::{BasicError, DashResult};
 
+#[cfg(target_pointer_width = "64")]
+pub const POINTER_SIZE: usize = 8;
+#[cfg(target_pointer_width = "32")]
+pub const POINTER_SIZE: usize = 4;
+
 impl From<isa::LookupError> for BasicError {
     fn from(err: isa::LookupError) -> BasicError {
         BasicError::new(err.to_string())
     }
 }
 
 impl From<settings::SetError> for BasicError {
     fn from(err: settings::SetError) -> BasicError {
@@ -99,23 +104,21 @@ fn make_shared_flags(
     env_flags: &Option<EnvVariableFlags>,
 ) -> settings::SetResult<settings::Flags> {
     let mut sb = settings::builder();
 
     // We don't install SIGFPE handlers, but depend on explicit traps around divisions.
     sb.enable("avoid_div_traps")?;
 
     // Cranelift needs to know how many words are pushed by `GenerateFunctionPrologue` so it can
-    // compute frame pointer offsets accurately.
-    //
-    // 1. Return address (whether explicitly pushed on ARM or implicitly on x86).
-    // 2. TLS register.
-    // 3. Previous frame pointer.
-    //
-    sb.set("baldrdash_prologue_words", "3")?;
+    // compute frame pointer offsets accurately. C++'s "sizeof" gives us the number of bytes, which
+    // we translate to the number of words, as expected by Cranelift.
+    debug_assert_eq!(env.size_of_wasm_frame % POINTER_SIZE, 0);
+    let num_words = env.size_of_wasm_frame / POINTER_SIZE;
+    sb.set("baldrdash_prologue_words", &num_words.to_string())?;
 
     // Make sure that libcalls use the supplementary VMContext argument.
     let libcall_call_conv = if env.platform_is_windows {
         "baldrdash_windows"
     } else {
         "baldrdash_system_v"
     };
     sb.set("libcall_call_conv", libcall_call_conv)?;
--- a/js/src/wasm/cranelift/src/wasm2clif.rs
+++ b/js/src/wasm/cranelift/src/wasm2clif.rs
@@ -31,26 +31,22 @@ use cranelift_codegen::isa::{CallConv, T
 use cranelift_codegen::packed_option::PackedOption;
 use cranelift_wasm::{
     FuncEnvironment, FuncIndex, GlobalIndex, GlobalVariable, MemoryIndex, ReturnMode,
     SignatureIndex, TableIndex, TargetEnvironment, WasmError, WasmResult,
 };
 
 use crate::bindings::{self, SymbolicAddress};
 use crate::compile::{symbolic_function_name, wasm_function_name};
+use crate::isa::POINTER_SIZE;
 
 #[cfg(target_pointer_width = "64")]
-const POINTER_TYPE: ir::Type = ir::types::I64;
+pub const POINTER_TYPE: ir::Type = ir::types::I64;
 #[cfg(target_pointer_width = "32")]
-const POINTER_TYPE: ir::Type = ir::types::I32;
-
-#[cfg(target_pointer_width = "64")]
-pub const POINTER_SIZE: i32 = 8;
-#[cfg(target_pointer_width = "32")]
-pub const POINTER_SIZE: i32 = 4;
+pub const POINTER_TYPE: ir::Type = ir::types::I32;
 
 #[cfg(target_pointer_width = "64")]
 pub const REF_TYPE: ir::Type = ir::types::R64;
 #[cfg(target_pointer_width = "32")]
 pub const REF_TYPE: ir::Type = ir::types::R32;
 
 /// Convert a TlsData offset into a `Offset32` for a global decl.
 fn offset32(offset: usize) -> ir::immediates::Offset32 {
@@ -716,17 +712,17 @@ impl<'a, 'b, 'c> FuncEnvironment for Tra
         let style = if is_static {
             // We have a static heap.
             let bound = bound.into();
             ir::HeapStyle::Static { bound }
         } else {
             // Get the `TlsData::boundsCheckLimit` field.
             let bound_gv = func.create_global_value(ir::GlobalValueData::Load {
                 base: vcmtx,
-                offset: POINTER_SIZE.into(),
+                offset: (POINTER_SIZE as i32).into(),
                 global_type: ir::types::I32,
                 readonly: false,
             });
             ir::HeapStyle::Dynamic { bound_gv }
         };
 
         let min_size = (self.env.min_memory_length() as u64).into();
         let offset_guard_size = (self.static_env.memory_guard_size as u64).into();
@@ -868,19 +864,22 @@ impl<'a, 'b, 'c> FuncEnvironment for Tra
 
         // Check for a null callee.
         pos.ins()
             .trapz(callee_func, ir::TrapCode::IndirectCallToNull);
 
         // Handle external tables, set up environment.
         // A function table call could redirect execution to another module with a different realm,
         // so switch to this realm just in case.
-        let callee_vmctx =
-            pos.ins()
-                .load(POINTER_TYPE, ir::MemFlags::trusted(), entry, POINTER_SIZE);
+        let callee_vmctx = pos.ins().load(
+            POINTER_TYPE,
+            ir::MemFlags::trusted(),
+            entry,
+            POINTER_SIZE as i32,
+        );
         self.switch_to_indirect_callee_realm(&mut pos, callee_vmctx);
         self.load_pinned_reg(&mut pos, callee_vmctx);
 
         // First the wasm args.
         let mut args = ir::ValueList::default();
         args.push(callee_func, &mut pos.func.dfg.value_lists);
         args.extend(call_args.iter().cloned(), &mut pos.func.dfg.value_lists);
         args.push(callee_vmctx, &mut pos.func.dfg.value_lists);
@@ -917,19 +916,22 @@ impl<'a, 'b, 'c> FuncEnvironment for Tra
             let gv = self.func_import_global(pos.func, callee_index);
             let gv_addr = pos.ins().global_value(POINTER_TYPE, gv);
 
             // We need the first two pointer-sized fields from the `FuncImportTls` struct: `code`
             // and `tls`.
             let fit_code = pos
                 .ins()
                 .load(POINTER_TYPE, ir::MemFlags::trusted(), gv_addr, 0);
-            let fit_tls =
-                pos.ins()
-                    .load(POINTER_TYPE, ir::MemFlags::trusted(), gv_addr, POINTER_SIZE);
+            let fit_tls = pos.ins().load(
+                POINTER_TYPE,
+                ir::MemFlags::trusted(),
+                gv_addr,
+                POINTER_SIZE as i32,
+            );
 
             // Switch to the callee's realm.
             self.switch_to_import_realm(&mut pos, fit_tls, gv_addr);
             self.load_pinned_reg(&mut pos, fit_tls);
 
             // The `tls` field is the VM context pointer for the callee.
             args.push(fit_tls, &mut pos.func.dfg.value_lists);
 
@@ -1265,11 +1267,11 @@ impl TableInfo {
 
         TableInfo { global }
     }
 
     /// Get the size in bytes of each table entry.
     pub fn entry_size(&self) -> i64 {
         // Each entry is an `wasm::FunctionTableElem` which consists of the code pointer and a new
         // VM context pointer.
-        i64::from(POINTER_SIZE) * 2
+        (POINTER_SIZE * 2) as i64
     }
 }