T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix #975 (Structure of cranelift-wasm/src/translation_utils.rs causes many pointless heap allocations) (#1010)

Browse Source 
This patch restricts the `Err(..)` return from `blocktype_to_type` to be
`Err(..)` only in the case where it really is an error to continue.  The three
use points of `blocktype_to_type` are changed to check for an `Err(..)` rather
than silently ignoring it.  There are also cosmetic changes to `type_to_type`
and `tabletype_to_type`.

When compiling wasm_lua_binarytrees, this reduces the number of blocks
allocated by CL by 1.9%.  Instruction count falls by 0.1%.

Details:

* `type_to_type` and `tabletype_to_type`:

   - Added the function name in the failure message

   - No functional change for non-error cases

   - Push the `Ok(..)` to expression leaves, where it really applies.  This
     corrects the misleading impression that, in the case of an unsupported
     type, the function returns `Ok` wrapped around whatever
     `wasm_unsupported` returns.  It doesn't do that, but it certainly reads
     like that.  This assumes that the LLVM backend will do tail merging, so
     the generated code will be unchanged.

* `blocktype_to_type`:

  - Change return type from `WasmResult<ir::Type>` to `WasmResult<Option<ir::Type>>`

  - Manually inline the call to `type_to_type`, to make this function easier
    to read.

  - For the non-error case: map `TypeOrFuncType::Type(Type::EmptyBlockType)`
    to `Ok(None)` rather than `Err(..)`, since that's what all the call sites
    expect - For the error cases, add the function name in the failure
    messages

* cranelift-wasm/src/code_translator.rs

  - For the three uses of `blocktype_to_type`, use `?` to detect failures and
    drop out immediately, meaning that the code will no longer silently ignore
    errors.
This commit is contained in:
julian-seward1
2019-09-10 15:06:10 +02:00
committed by GitHub
parent 81fa5e7696
commit 63367d205c
2 changed files with 32 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cranelift/wasm/src/code_translator.rs
View File

@@ -133,7 +133,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
***********************************************************************************/
Operator::Block { ty } => {
let next = builder.create_ebb();
if let Ok(ty_cre) = blocktype_to_type(*ty) {
if let Some(ty_cre) = blocktype_to_type(*ty)? {
builder.append_ebb_param(next, ty_cre);
}
state.push_block(next, num_return_values(*ty)?);
@@ -141,7 +141,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
Operator::Loop { ty } => {
let loop_body = builder.create_ebb();
let next = builder.create_ebb();
if let Ok(ty_cre) = blocktype_to_type(*ty) {
if let Some(ty_cre) = blocktype_to_type(*ty)? {
builder.append_ebb_param(next, ty_cre);
}
builder.ins().jump(loop_body, &[]);
@@ -168,7 +168,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
// and we add nothing;
// - either the If have an Else clause, in that case the destination of this jump
// instruction will be changed later when we translate the Else operator.
if let Ok(ty_cre) = blocktype_to_type(*ty) {
if let Some(ty_cre) = blocktype_to_type(*ty)? {
builder.append_ebb_param(if_not, ty_cre);
}
state.push_if(jump_inst, if_not, num_return_values(*ty)?);