Cranellift: remove Baldrdash support and related features. (#4571)

* Cranellift: remove Baldrdash support and related features.

As noted in Mozilla's bugzilla bug 1781425 [1], the SpiderMonkey team
has recently determined that their current form of integration with
Cranelift is too hard to maintain, and they have chosen to remove it
from their codebase. If and when they decide to build updated support
for Cranelift, they will adopt different approaches to several details
of the integration.

In the meantime, after discussion with the SpiderMonkey folks, they
agree that it makes sense to remove the bits of Cranelift that exist
to support the integration ("Baldrdash"), as they will not need
them. Many of these bits are difficult-to-maintain special cases that
are not actually tested in Cranelift proper: for example, the
Baldrdash integration required Cranelift to emit function bodies
without prologues/epilogues, and instead communicate very precise
information about the expected frame size and layout, then stitched
together something post-facto. This was brittle and caused a lot of
incidental complexity ("fallthrough returns", the resulting special
logic in block-ordering); this is just one example. As another
example, one particular Baldrdash ABI variant processed stack args in
reverse order, so our ABI code had to support both traversal
orders. We had a number of other Baldrdash-specific settings as well
that did various special things.

This PR removes Baldrdash ABI support, the `fallthrough_return`
instruction, and pulls some threads to remove now-unused bits as a
result of those two, with the  understanding that the SpiderMonkey folks
will build new functionality as needed in the future and we can perhaps
find cleaner abstractions to make it all work.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1781425

* Review feedback.

* Fix (?) DWARF debug tests: add `--disable-cache` to wasmtime invocations.

The debugger tests invoke `wasmtime` from within each test case under
the control of a debugger (gdb or lldb). Some of these tests started to
inexplicably fail in CI with unrelated changes, and the failures were
only inconsistently reproducible locally. It seems to be cache related:
if we disable cached compilation on the nested `wasmtime` invocations,
the tests consistently pass.

* Review feedback.
This commit is contained in:
Chris Fallin
2022-08-02 12:37:56 -07:00
committed by GitHub
parent ff37c9d8a4
commit 43f1765272
57 changed files with 221 additions and 1134 deletions

View File

@@ -72,7 +72,7 @@
//! ("Relax verification to allow I8X16 to act as a default vector type")
use super::{hash_map, HashMap};
use crate::environ::{FuncEnvironment, GlobalVariable, ReturnMode};
use crate::environ::{FuncEnvironment, GlobalVariable};
use crate::state::{ControlStackFrame, ElseData, FuncTranslationState};
use crate::translation_utils::{
block_with_params, blocktype_params_results, f32_translation, f64_translation,
@@ -531,23 +531,14 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
state.reachable = false;
}
Operator::Return => {
let (return_count, br_destination) = {
let return_count = {
let frame = &mut state.control_stack[0];
if environ.return_mode() == ReturnMode::FallthroughReturn {
frame.set_branched_to_exit();
}
let return_count = frame.num_return_values();
(return_count, frame.br_destination())
frame.num_return_values()
};
{
let return_args = state.peekn_mut(return_count);
bitcast_wasm_returns(environ, return_args, builder);
match environ.return_mode() {
ReturnMode::NormalReturns => builder.ins().return_(return_args),
ReturnMode::FallthroughReturn => {
canonicalise_then_jump(builder, br_destination, return_args)
}
};
builder.ins().return_(return_args);
}
state.popn(return_count);
state.reachable = false;

View File

@@ -5,9 +5,7 @@
//! [wasmtime-environ]: https://crates.io/crates/wasmtime-environ
//! [Wasmtime]: https://github.com/bytecodealliance/wasmtime
use crate::environ::{
FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, TargetEnvironment,
};
use crate::environ::{FuncEnvironment, GlobalVariable, ModuleEnvironment, TargetEnvironment};
use crate::func_translator::FuncTranslator;
use crate::state::FuncTranslationState;
use crate::WasmType;
@@ -150,9 +148,6 @@ pub struct DummyEnvironment {
/// Vector of wasm bytecode size for each function.
pub func_bytecode_sizes: Vec<usize>,
/// How to return from functions.
return_mode: ReturnMode,
/// Instructs to collect debug data during translation.
debug_info: bool,
@@ -168,12 +163,11 @@ pub struct DummyEnvironment {
impl DummyEnvironment {
/// Creates a new `DummyEnvironment` instance.
pub fn new(config: TargetFrontendConfig, return_mode: ReturnMode, debug_info: bool) -> Self {
pub fn new(config: TargetFrontendConfig, debug_info: bool) -> Self {
Self {
info: DummyModuleInfo::new(config),
trans: FuncTranslator::new(),
func_bytecode_sizes: Vec::new(),
return_mode,
debug_info,
module_name: None,
function_names: SecondaryMap::new(),
@@ -184,11 +178,7 @@ impl DummyEnvironment {
/// Return a `DummyFuncEnvironment` for translating functions within this
/// `DummyEnvironment`.
pub fn func_env(&self) -> DummyFuncEnvironment {
DummyFuncEnvironment::new(
&self.info,
self.return_mode,
self.expected_reachability.clone(),
)
DummyFuncEnvironment::new(&self.info, self.expected_reachability.clone())
}
fn get_func_type(&self, func_index: FuncIndex) -> TypeIndex {
@@ -222,8 +212,6 @@ impl DummyEnvironment {
pub struct DummyFuncEnvironment<'dummy_environment> {
pub mod_info: &'dummy_environment DummyModuleInfo,
return_mode: ReturnMode,
/// Expected reachability data (before/after for each op) to assert. This is used for testing.
expected_reachability: Option<ExpectedReachability>,
}
@@ -231,12 +219,10 @@ pub struct DummyFuncEnvironment<'dummy_environment> {
impl<'dummy_environment> DummyFuncEnvironment<'dummy_environment> {
pub fn new(
mod_info: &'dummy_environment DummyModuleInfo,
return_mode: ReturnMode,
expected_reachability: Option<ExpectedReachability>,
) -> Self {
Self {
mod_info,
return_mode,
expected_reachability,
}
}
@@ -268,10 +254,6 @@ impl<'dummy_environment> TargetEnvironment for DummyFuncEnvironment<'dummy_envir
}
impl<'dummy_environment> FuncEnvironment for DummyFuncEnvironment<'dummy_environment> {
fn return_mode(&self) -> ReturnMode {
self.return_mode
}
fn make_global(
&mut self,
func: &mut ir::Function,
@@ -863,11 +845,8 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
self.func_bytecode_sizes
.push(body.get_binary_reader().bytes_remaining());
let func = {
let mut func_environ = DummyFuncEnvironment::new(
&self.info,
self.return_mode,
self.expected_reachability.clone(),
);
let mut func_environ =
DummyFuncEnvironment::new(&self.info, self.expected_reachability.clone());
let func_index =
FuncIndex::new(self.get_num_func_imports() + self.info.function_bodies.len());
let name = get_func_name(func_index);

View File

@@ -6,5 +6,5 @@ mod spec;
pub use crate::environ::dummy::DummyEnvironment;
pub use crate::environ::spec::{
FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode, TargetEnvironment,
FuncEnvironment, GlobalVariable, ModuleEnvironment, TargetEnvironment,
};

View File

@@ -41,15 +41,6 @@ pub enum GlobalVariable {
Custom,
}
/// How to return from functions.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum ReturnMode {
/// Use normal return instructions as needed.
NormalReturns,
/// Use a single fallthrough return at the end of the function.
FallthroughReturn,
}
/// Environment affecting the translation of a WebAssembly.
pub trait TargetEnvironment {
/// Get the information needed to produce Cranelift IR for the given target.
@@ -102,13 +93,6 @@ pub trait FuncEnvironment: TargetEnvironment {
signature.returns[index].purpose == ir::ArgumentPurpose::Normal
}
/// Should the code be structured to use a single `fallthrough_return` instruction at the end
/// of the function body, rather than `return` instructions as needed? This is used by VMs
/// to append custom epilogues.
fn return_mode(&self) -> ReturnMode {
ReturnMode::NormalReturns
}
/// Called after the locals for a function have been parsed, and the number
/// of variables defined by this function is provided.
fn after_locals(&mut self, num_locals_defined: usize) {

View File

@@ -5,7 +5,7 @@
//! WebAssembly module and the runtime environment.
use crate::code_translator::{bitcast_wasm_returns, translate_operator};
use crate::environ::{FuncEnvironment, ReturnMode};
use crate::environ::FuncEnvironment;
use crate::state::FuncTranslationState;
use crate::translation_utils::get_vmctx_value_label;
use crate::WasmResult;
@@ -253,13 +253,8 @@ fn parse_function_body<FE: FuncEnvironment + ?Sized>(
// generate a return instruction that doesn't match the signature.
if state.reachable {
if !builder.is_unreachable() {
match environ.return_mode() {
ReturnMode::NormalReturns => {
bitcast_wasm_returns(environ, &mut state.stack, builder);
builder.ins().return_(&state.stack)
}
ReturnMode::FallthroughReturn => builder.ins().fallthrough_return(&state.stack),
};
bitcast_wasm_returns(environ, &mut state.stack, builder);
builder.ins().return_(&state.stack);
}
}
@@ -279,7 +274,7 @@ fn cur_srcloc(reader: &BinaryReader) -> ir::SourceLoc {
#[cfg(test)]
mod tests {
use super::{FuncTranslator, ReturnMode};
use super::FuncTranslator;
use crate::environ::DummyEnvironment;
use cranelift_codegen::ir::types::I32;
use cranelift_codegen::{ir, isa, settings, Context};
@@ -310,7 +305,6 @@ mod tests {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
ReturnMode::NormalReturns,
false,
);
@@ -349,7 +343,6 @@ mod tests {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
ReturnMode::NormalReturns,
false,
);
@@ -393,7 +386,6 @@ mod tests {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
ReturnMode::NormalReturns,
false,
);

View File

@@ -57,8 +57,7 @@ mod state;
mod translation_utils;
pub use crate::environ::{
DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, ReturnMode,
TargetEnvironment,
DummyEnvironment, FuncEnvironment, GlobalVariable, ModuleEnvironment, TargetEnvironment,
};
pub use crate::func_translator::FuncTranslator;
pub use crate::module_translator::translate_module;