From 29f1c13849675f46a9ef7e6ff8c4c43a166c4312 Mon Sep 17 00:00:00 2001 From: Yuri Iozzelli Date: Fri, 25 Feb 2022 15:38:03 +0100 Subject: [PATCH] Update WebAssembly Branch Hinting proposal The main change is the section name, which is now 'metadata.code.branch_hint'. The binary format has also a couple of minor changes. Semantics remain unchanged. Change-Id: I056c9f672ae494979e8ea55266fa766139b71d38 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3487788 Reviewed-by: Jakob Kummerow Commit-Queue: Jakob Kummerow Cr-Commit-Position: refs/heads/main@{#79292} --- src/wasm/function-body-decoder-impl.h | 6 +++--- src/wasm/module-decoder.cc | 14 ++++++------- .../unittests/wasm/module-decoder-unittest.cc | 20 ++++++++++--------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h index 60d0d36ec8..89095329e3 100644 --- a/src/wasm/function-body-decoder-impl.h +++ b/src/wasm/function-body-decoder-impl.h @@ -2234,6 +2234,7 @@ class WasmFullDecoder : public WasmDecoder { DCHECK_LE(this->pc_, this->end_); DCHECK_EQ(this->num_locals(), 0); + locals_offset_ = this->pc_offset(); this->InitializeLocalsFromSig(); uint32_t params_count = static_cast(this->num_locals()); uint32_t locals_length; @@ -2340,7 +2341,7 @@ class WasmFullDecoder : public WasmDecoder { } uint32_t pc_relative_offset() const { - return this->pc_offset() - first_instruction_offset; + return this->pc_offset() - locals_offset_; } void DecodeFunctionBody() { @@ -2374,7 +2375,6 @@ class WasmFullDecoder : public WasmDecoder { CALL_INTERFACE_IF_OK_AND_REACHABLE(StartFunctionBody, c); } - first_instruction_offset = this->pc_offset(); // Decode the function body. while (this->pc_ < this->end_) { // Most operations only grow the stack by at least one element (unary and @@ -2407,7 +2407,7 @@ class WasmFullDecoder : public WasmDecoder { } private: - uint32_t first_instruction_offset = 0; + uint32_t locals_offset_ = 0; Interface interface_; // The value stack, stored as individual pointers for maximum performance. diff --git a/src/wasm/module-decoder.cc b/src/wasm/module-decoder.cc index f9f37e98b8..77dcec61bd 100644 --- a/src/wasm/module-decoder.cc +++ b/src/wasm/module-decoder.cc @@ -36,7 +36,7 @@ namespace { constexpr char kNameString[] = "name"; constexpr char kSourceMappingURLString[] = "sourceMappingURL"; constexpr char kCompilationHintsString[] = "compilationHints"; -constexpr char kBranchHintsString[] = "branchHints"; +constexpr char kBranchHintsString[] = "metadata.code.branch_hint"; constexpr char kDebugInfoString[] = ".debug_info"; constexpr char kExternalDebugInfoString[] = "external_debug_info"; @@ -1345,11 +1345,6 @@ class ModuleDecoderImpl : public Decoder { break; } last_func_idx = func_idx; - uint8_t reserved = inner.consume_u8("reserved byte"); - if (reserved != 0x0) { - inner.errorf("Invalid reserved byte: %#x", reserved); - break; - } uint32_t num_hints = inner.consume_u32v("number of hints"); BranchHintMap func_branch_hints; TRACE("DecodeBranchHints[%d] module+%d\n", func_idx, @@ -1357,13 +1352,18 @@ class ModuleDecoderImpl : public Decoder { // Keep track of the previous branch offset to validate the ordering int64_t last_br_off = -1; for (uint32_t j = 0; j < num_hints; ++j) { - uint32_t br_dir = inner.consume_u32v("branch direction"); uint32_t br_off = inner.consume_u32v("branch instruction offset"); if (int64_t(br_off) <= last_br_off) { inner.errorf("Invalid branch offset: %d", br_off); break; } last_br_off = br_off; + uint32_t data_size = inner.consume_u32v("data size"); + if (data_size != 1) { + inner.errorf("Invalid data size: %#x. Expected 1.", data_size); + break; + } + uint32_t br_dir = inner.consume_u8("branch direction"); TRACE("DecodeBranchHints[%d][%d] module+%d\n", func_idx, br_off, static_cast(inner.pc() - inner.start())); WasmBranchHint hint; diff --git a/test/unittests/wasm/module-decoder-unittest.cc b/test/unittests/wasm/module-decoder-unittest.cc index a43edc970e..b456b94ddf 100644 --- a/test/unittests/wasm/module-decoder-unittest.cc +++ b/test/unittests/wasm/module-decoder-unittest.cc @@ -76,9 +76,11 @@ namespace module_decoder_unittest { 'H', 'i', 'n', 't', 's'), \ ADD_COUNT(__VA_ARGS__)) -#define SECTION_BRANCH_HINTS(...) \ - SECTION(Unknown, \ - ADD_COUNT('b', 'r', 'a', 'n', 'c', 'h', 'H', 'i', 'n', 't', 's'), \ +#define SECTION_BRANCH_HINTS(...) \ + SECTION(Unknown, \ + ADD_COUNT('m', 'e', 't', 'a', 'd', 'a', 't', 'a', '.', 'c', 'o', \ + 'd', 'e', '.', 'b', 'r', 'a', 'n', 'c', 'h', '_', 'h', \ + 'i', 'n', 't'), \ __VA_ARGS__) #define FAIL_IF_NO_EXPERIMENTAL_EH(data) \ @@ -2228,10 +2230,10 @@ TEST_F(WasmModuleVerifyTest, BranchHinting) { WASM_FEATURE_SCOPE(branch_hinting); static const byte data[] = { TYPE_SECTION(1, SIG_ENTRY_v_v), FUNCTION_SECTION(2, 0, 0), - SECTION_BRANCH_HINTS(ENTRY_COUNT(2), 0 /*func_index*/, 0 /*reserved*/, - ENTRY_COUNT(1), 1 /*likely*/, 2 /* if offset*/, - 1 /*func_index*/, 0 /*reserved*/, ENTRY_COUNT(1), - 0 /*unlikely*/, 4 /* br_if offset*/), + SECTION_BRANCH_HINTS(ENTRY_COUNT(2), 0 /*func_index*/, ENTRY_COUNT(1), + 3 /* if offset*/, 1 /*reserved*/, 1 /*likely*/, + 1 /*func_index*/, ENTRY_COUNT(1), + 5 /* br_if offset*/, 1 /*reserved*/, 0 /*unlikely*/), SECTION(Code, ENTRY_COUNT(2), ADD_COUNT(0, /*no locals*/ WASM_IF(WASM_I32V_1(1), WASM_NOP), WASM_END), @@ -2243,9 +2245,9 @@ TEST_F(WasmModuleVerifyTest, BranchHinting) { EXPECT_EQ(2u, result.value()->branch_hints.size()); EXPECT_EQ(WasmBranchHint::kLikely, - result.value()->branch_hints[0].GetHintFor(2)); + result.value()->branch_hints[0].GetHintFor(3)); EXPECT_EQ(WasmBranchHint::kUnlikely, - result.value()->branch_hints[1].GetHintFor(4)); + result.value()->branch_hints[1].GetHintFor(5)); } class WasmSignatureDecodeTest : public TestWithZone {