Commit 4c3db4fd authored by Jakob Kummerow's avatar Jakob Kummerow Committed by Commit Bot

[wasm] Repair decoder perf regression

With the value stack refactoring in 1b5c7e15 / r73193, the
combination of helper functions called by PeekArgs() ended
up checking the stack height repeatedly. This CL avoids that
by introducing a ValidateArgType() helper that does not check
stack height.
Bonus: achieve a small speedup by special-casing two of the
most common opcodes in the decoder's main dispatcher.

Fixed: chromium:1185082
Change-Id: I6d51aca844ef9377d203147f74ff8137e12a23e7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2745341
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73294}
parent 2966c896
...@@ -3487,8 +3487,19 @@ class WasmFullDecoder : public WasmDecoder<validate> { ...@@ -3487,8 +3487,19 @@ class WasmFullDecoder : public WasmDecoder<validate> {
uint8_t first_byte = *this->pc_; uint8_t first_byte = *this->pc_;
WasmOpcode opcode = static_cast<WasmOpcode>(first_byte); WasmOpcode opcode = static_cast<WasmOpcode>(first_byte);
CALL_INTERFACE_IF_REACHABLE(NextInstruction, opcode); CALL_INTERFACE_IF_REACHABLE(NextInstruction, opcode);
OpcodeHandler handler = GetOpcodeHandler(first_byte); int len;
int len = (*handler)(this, opcode); // Allowing two of the most common decoding functions to get inlined
// appears to be the sweet spot.
// Handling _all_ opcodes via a giant switch-statement has been tried
// and found to be slower than calling through the handler table.
if (opcode == kExprLocalGet) {
len = WasmFullDecoder::DecodeLocalGet(this, opcode);
} else if (opcode == kExprI32Const) {
len = WasmFullDecoder::DecodeI32Const(this, opcode);
} else {
OpcodeHandler handler = GetOpcodeHandler(first_byte);
len = (*handler)(this, opcode);
}
this->pc_ += len; this->pc_ += len;
} }
...@@ -3561,9 +3572,8 @@ class WasmFullDecoder : public WasmDecoder<validate> { ...@@ -3561,9 +3572,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
if (count == 0) return {}; if (count == 0) return {};
EnsureStackArguments(depth + count); EnsureStackArguments(depth + count);
ArgVector args(stack_value(depth + count), count); ArgVector args(stack_value(depth + count), count);
// Validate types. for (int i = 0; i < count; i++) {
for (int i = count - 1; i >= 0; --i, ++depth) { ValidateArgType(args, i, sig->GetParam(i));
Peek(depth, i, sig->GetParam(i));
} }
return args; return args;
} }
...@@ -3577,9 +3587,8 @@ class WasmFullDecoder : public WasmDecoder<validate> { ...@@ -3577,9 +3587,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
if (count == 0) return {}; if (count == 0) return {};
EnsureStackArguments(depth + count); EnsureStackArguments(depth + count);
ArgVector args(stack_value(depth + count), count); ArgVector args(stack_value(depth + count), count);
// Validate types. for (int i = 0; i < count; i++) {
for (int i = count - 1; i >= 0; i--, depth++) { ValidateArgType(args, i, type->field(i).Unpacked());
Peek(depth, i, type->field(i).Unpacked());
} }
return args; return args;
} }
...@@ -3592,9 +3601,8 @@ class WasmFullDecoder : public WasmDecoder<validate> { ...@@ -3592,9 +3601,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
int size = static_cast<int>(arg_types.size()); int size = static_cast<int>(arg_types.size());
EnsureStackArguments(size); EnsureStackArguments(size);
ArgVector args(stack_value(size), arg_types.size()); ArgVector args(stack_value(size), arg_types.size());
// Validate types. for (int i = 0; i < size; i++) {
for (int i = size - 1, depth = 0; i >= 0; i--, depth++) { ValidateArgType(args, i, arg_types[i]);
Peek(depth, base_index + i, arg_types[i]);
} }
return args; return args;
} }
...@@ -4800,6 +4808,15 @@ class WasmFullDecoder : public WasmDecoder<validate> { ...@@ -4800,6 +4808,15 @@ class WasmFullDecoder : public WasmDecoder<validate> {
return *(stack_end_ - depth - 1); return *(stack_end_ - depth - 1);
} }
V8_INLINE void ValidateArgType(ArgVector args, int index,
ValueType expected) {
Value val = args[index];
if (!VALIDATE(IsSubtypeOf(val.type, expected, this->module_) ||
val.type == kWasmBottom || expected == kWasmBottom)) {
PopTypeError(index, val, expected);
}
}
V8_INLINE void Drop(int count = 1) { V8_INLINE void Drop(int count = 1) {
DCHECK(!control_.empty()); DCHECK(!control_.empty());
uint32_t limit = control_.back().stack_depth; uint32_t limit = control_.back().stack_depth;
......
...@@ -74,9 +74,9 @@ load('test/mjsunit/wasm/wasm-module-builder.js'); ...@@ -74,9 +74,9 @@ load('test/mjsunit/wasm/wasm-module-builder.js');
assertPromiseResult(WebAssembly.instantiateStreaming(Promise.resolve(bytes), assertPromiseResult(WebAssembly.instantiateStreaming(Promise.resolve(bytes),
{mod: {pow: Math.pow}}) {mod: {pow: Math.pow}})
.then(assertUnreachable, .then(assertUnreachable,
error => assertEquals("WebAssembly.instantiateStreaming(): call[1] " + error => assertEquals("WebAssembly.instantiateStreaming(): call[0] " +
"expected type f32, found local.get of type " + "expected type f32, found local.get of type " +
"i32 @+94", "i32 @+92",
error.message))); error.message)));
})(); })();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment