From 518b7a7e236b084fbcf9cd72265c7b01e309621f Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 11 Sep 2020 13:12:48 +0200 Subject: [PATCH] wasm: Remove duplicated parameters when popping an If Parameters are duplicated when pushing an If block, so they're available to the Else block without an extra heap allocation. However, when truncating the stack after popping the If control frame, the stack size at entry doesn't account for the duplicated parameters. That is intentional: the Else block uses this value to know what's the stack size when it is entered, so there's nothing to change there. This patch makes the wasm translation truncates the value stack to the right size after an If block, by taking those duplicated parameters into account. --- cranelift/wasm/src/code_translator.rs | 22 +++++++++++++++++++++- cranelift/wasmtests/multi-17.wat | 26 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 cranelift/wasmtests/multi-17.wat diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index 0a0b3ce4a7..3a4bea1294 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -323,13 +323,33 @@ pub fn translate_operator( // since we truncate the stack back to the original height // below. } + builder.switch_to_block(next_block); builder.seal_block(next_block); + // If it is a loop we also have to seal the body loop block if let ControlStackFrame::Loop { header, .. } = frame { builder.seal_block(header) } - state.stack.truncate(frame.original_stack_size()); + + // The "If" frame pushes its parameters twice, so they're available to the else block + // (see also `FuncTranslationState::push_if`). + // Yet, the original_stack_size member accounts for them only once, so that the else + // block can see the same number of parameters as the consequent block. As a matter of + // fact, we need to substract an extra number of parameter values for if blocks. + let num_duplicated_params = if let ControlStackFrame::If { + num_param_values, .. + } = frame + { + debug_assert!(num_param_values <= frame.original_stack_size()); + num_param_values + } else { + 0 + }; + + state + .stack + .truncate(frame.original_stack_size() - num_duplicated_params); state .stack .extend_from_slice(builder.block_params(next_block)); diff --git a/cranelift/wasmtests/multi-17.wat b/cranelift/wasmtests/multi-17.wat new file mode 100644 index 0000000000..e190851e85 --- /dev/null +++ b/cranelift/wasmtests/multi-17.wat @@ -0,0 +1,26 @@ +(module + (func $main (type 0) (param i32 i32 i32) (result i32) + i32.const 0 + i32.const 0 + i32.const 0 + i32.const 0 + + i32.const 0 + if (param i32 i32 i32) (result i32) ;; label = @1 + br 0 (;@1;) + else + call $main + end + + i32.const 0 + + i32.const 0 + if (param i32 i32 i32) (result i32) ;; label = @1 + drop + drop + else + drop + drop + end + ) + (export "main" (func $main)))