From c18d60a8fa961d2c297c73ad7518f9ea87166d1f Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 15 Nov 2024 17:22:29 -0800 Subject: [PATCH 1/3] [NFC] Finalize blocks with explicit breakability in IRBuilder Since IRBuilder already knows what labels are used by branches, it is easy for it to pass that information when finalizing blocks. This avoids finalization having to walk the blocks looking for branches, speeding up a future version of the binary parser that uses IRBuilder by 10%. --- src/passes/Outlining.cpp | 7 +++++++ src/wasm/wasm-ir-builder.cpp | 12 ++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/passes/Outlining.cpp b/src/passes/Outlining.cpp index 429511557e1..85abb57e422 100644 --- a/src/passes/Outlining.cpp +++ b/src/passes/Outlining.cpp @@ -304,6 +304,13 @@ struct Outlining : public Pass { // Position the outlined functions first in the functions vector to make // the outlining lit tests far more readable. moveOutlinedFunctions(module, substrings.size()); + + // Because we visit control flow in an odd order, IRBuilder is not able to + // properly track branches, so it may not have finalized blocks with the + // correct types. ReFinalize now to fix any issues. + PassRunner runner(getPassRunner()); + runner.add(std::make_unique()); + runner.run(); } Name addOutlinedFunction(Module* module, diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 0d46156be28..5253c91ee5d 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -972,7 +972,12 @@ Result<> IRBuilder::visitEnd() { block->type = blockType; return block; } - return builder.makeBlock(label, {curr}, blockType); + auto* block = builder.makeBlock(); + block->name = label; + block->list.push_back(curr); + block->finalize(blockType, + scope.labelUsed ? Block::HasBreak : Block::NoBreak); + return block; }; if (auto* func = scope.getFunction()) { @@ -985,9 +990,8 @@ Result<> IRBuilder::visitEnd() { } else if (auto* block = scope.getBlock()) { assert(*expr == block); block->name = scope.label; - // TODO: Track branches so we can know whether this block is a target and - // finalize more efficiently. - block->finalize(block->type); + block->finalize(block->type, + scope.labelUsed ? Block::HasBreak : Block::NoBreak); push(block); } else if (auto* loop = scope.getLoop()) { loop->body = *expr; From 5b2c76f5607020dc7ce76e88d80c1e5f17227c40 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 18 Nov 2024 11:04:14 -0800 Subject: [PATCH 2/3] Use hints when generating fresh labels in IRBuilder IRBuilder often has to generate new label names for blocks and other scopes. Previously it would generate each new name by starting with "block" or "label" and incrementing a suffix until finding a fresh name, but this made name generation quadratic in the number of names to generate. To spend less time generating names, track a hint index at which to start looking for a fresh name and increment it every time a name is generated. This speeds up a version of the binary parser that uses IRBuilder by about 15%. --- src/wasm-ir-builder.h | 7 ++- src/wasm/wasm-ir-builder.cpp | 6 ++- test/lit/basic/reference-types.wast | 38 +++++++------- test/lit/passes/outlining.wast | 4 +- test/lit/wat-kitchen-sink.wast | 18 +++---- test/wasm2js/br_table_temp.2asm.js | 68 +++++++++++++------------- test/wasm2js/br_table_temp.2asm.js.opt | 20 ++++---- test/wasm2js/labels.2asm.js | 8 +-- 8 files changed, 87 insertions(+), 82 deletions(-) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index d6e45314933..30e770e28dd 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -507,16 +507,19 @@ class IRBuilder : public UnifiedExpressionVisitor> { // its stack. std::unordered_map> labelDepths; - Name makeFresh(Name label) { + Name makeFresh(Name label, Index hint = 0) { return Names::getValidName( label, [&](Name candidate) { return labelDepths.insert({candidate, {}}).second; }, - 0, + hint, ""); } + Index blockHint = 0; + Index labelHint = 0; + void pushScope(ScopeCtx scope) { if (auto label = scope.getOriginalLabel()) { // Assign a fresh label to the scope, if necessary. diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 5253c91ee5d..a73c7f2acb4 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -987,6 +987,8 @@ Result<> IRBuilder::visitEnd() { EHUtils::handleBlockNestedPops(func, wasm); } this->func = nullptr; + blockHint = 0; + labelHint = 0; } else if (auto* block = scope.getBlock()) { assert(*expr == block); block->name = scope.label; @@ -1073,9 +1075,9 @@ Result IRBuilder::getLabelName(Index label, bool forDelegate) { if (!scopeLabel) { // The scope does not already have a name, so we need to create one. if ((*scope)->getBlock()) { - scopeLabel = makeFresh("block"); + scopeLabel = makeFresh("block", blockHint++); } else { - scopeLabel = makeFresh("label"); + scopeLabel = makeFresh("label", labelHint++); } } if (!forDelegate) { diff --git a/test/lit/basic/reference-types.wast b/test/lit/basic/reference-types.wast index 6ef51c23d39..44494c80d97 100644 --- a/test/lit/basic/reference-types.wast +++ b/test/lit/basic/reference-types.wast @@ -361,25 +361,17 @@ ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: (drop - ;; CHECK-TEXT-NEXT: (block $block0 (result eqref) - ;; CHECK-TEXT-NEXT: (br_if $block0 - ;; CHECK-TEXT-NEXT: (global.get $global_eqref) - ;; CHECK-TEXT-NEXT: (i32.const 1) - ;; CHECK-TEXT-NEXT: ) - ;; CHECK-TEXT-NEXT: ) - ;; CHECK-TEXT-NEXT: ) - ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block1 (result eqref) ;; CHECK-TEXT-NEXT: (br_if $block1 - ;; CHECK-TEXT-NEXT: (ref.null none) + ;; CHECK-TEXT-NEXT: (global.get $global_eqref) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: (drop - ;; CHECK-TEXT-NEXT: (block $block2 (result funcref) + ;; CHECK-TEXT-NEXT: (block $block2 (result eqref) ;; CHECK-TEXT-NEXT: (br_if $block2 - ;; CHECK-TEXT-NEXT: (local.get $local_funcref) + ;; CHECK-TEXT-NEXT: (ref.null none) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -387,7 +379,7 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block3 (result funcref) ;; CHECK-TEXT-NEXT: (br_if $block3 - ;; CHECK-TEXT-NEXT: (global.get $global_funcref) + ;; CHECK-TEXT-NEXT: (local.get $local_funcref) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -395,7 +387,7 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block4 (result funcref) ;; CHECK-TEXT-NEXT: (br_if $block4 - ;; CHECK-TEXT-NEXT: (ref.null nofunc) + ;; CHECK-TEXT-NEXT: (global.get $global_funcref) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -403,15 +395,15 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block5 (result funcref) ;; CHECK-TEXT-NEXT: (br_if $block5 - ;; CHECK-TEXT-NEXT: (ref.func $foo) + ;; CHECK-TEXT-NEXT: (ref.null nofunc) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: (drop - ;; CHECK-TEXT-NEXT: (block $block6 (result anyref) + ;; CHECK-TEXT-NEXT: (block $block6 (result funcref) ;; CHECK-TEXT-NEXT: (br_if $block6 - ;; CHECK-TEXT-NEXT: (local.get $local_anyref) + ;; CHECK-TEXT-NEXT: (ref.func $foo) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -419,7 +411,7 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block7 (result anyref) ;; CHECK-TEXT-NEXT: (br_if $block7 - ;; CHECK-TEXT-NEXT: (global.get $global_anyref) + ;; CHECK-TEXT-NEXT: (local.get $local_anyref) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -427,7 +419,7 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block8 (result anyref) ;; CHECK-TEXT-NEXT: (br_if $block8 - ;; CHECK-TEXT-NEXT: (ref.null none) + ;; CHECK-TEXT-NEXT: (global.get $global_anyref) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -435,7 +427,7 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block9 (result anyref) ;; CHECK-TEXT-NEXT: (br_if $block9 - ;; CHECK-TEXT-NEXT: (local.get $local_eqref) + ;; CHECK-TEXT-NEXT: (ref.null none) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) ;; CHECK-TEXT-NEXT: ) @@ -443,6 +435,14 @@ ;; CHECK-TEXT-NEXT: (drop ;; CHECK-TEXT-NEXT: (block $block10 (result anyref) ;; CHECK-TEXT-NEXT: (br_if $block10 + ;; CHECK-TEXT-NEXT: (local.get $local_eqref) + ;; CHECK-TEXT-NEXT: (i32.const 1) + ;; CHECK-TEXT-NEXT: ) + ;; CHECK-TEXT-NEXT: ) + ;; CHECK-TEXT-NEXT: ) + ;; CHECK-TEXT-NEXT: (drop + ;; CHECK-TEXT-NEXT: (block $block11 (result anyref) + ;; CHECK-TEXT-NEXT: (br_if $block11 ;; CHECK-TEXT-NEXT: (ref.null none) ;; CHECK-TEXT-NEXT: (i32.const 1) ;; CHECK-TEXT-NEXT: ) diff --git a/test/lit/passes/outlining.wast b/test/lit/passes/outlining.wast index 5119572b395..a02e6f2cd89 100644 --- a/test/lit/passes/outlining.wast +++ b/test/lit/passes/outlining.wast @@ -675,8 +675,8 @@ ;; CHECK: (func $a (type $1) (param $0 i32) (result i32) ;; CHECK-NEXT: (call $outline$) ;; CHECK-NEXT: (block $block - ;; CHECK-NEXT: (block $block0 - ;; CHECK-NEXT: (br_table $block $block0 + ;; CHECK-NEXT: (block $block1 + ;; CHECK-NEXT: (br_table $block $block1 ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (return diff --git a/test/lit/wat-kitchen-sink.wast b/test/lit/wat-kitchen-sink.wast index 97d9f57e835..8b77de74dac 100644 --- a/test/lit/wat-kitchen-sink.wast +++ b/test/lit/wat-kitchen-sink.wast @@ -2570,14 +2570,14 @@ ) ;; CHECK: (func $label-index (type $0) - ;; CHECK-NEXT: (block $block1 + ;; CHECK-NEXT: (block $block2 ;; CHECK-NEXT: (block $block - ;; CHECK-NEXT: (block $block0 + ;; CHECK-NEXT: (block $block1 ;; CHECK-NEXT: (block $l ;; CHECK-NEXT: (br $block) - ;; CHECK-NEXT: (br $block0) - ;; CHECK-NEXT: (br $l) ;; CHECK-NEXT: (br $block1) + ;; CHECK-NEXT: (br $l) + ;; CHECK-NEXT: (br $block2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -2844,9 +2844,9 @@ ;; CHECK: (func $br-table-index (type $0) ;; CHECK-NEXT: (block $block ;; CHECK-NEXT: (block $l - ;; CHECK-NEXT: (block $block1 - ;; CHECK-NEXT: (block $block0 - ;; CHECK-NEXT: (br_table $block $l $block0 $block1 + ;; CHECK-NEXT: (block $block2 + ;; CHECK-NEXT: (block $block1 + ;; CHECK-NEXT: (br_table $block $l $block1 $block2 ;; CHECK-NEXT: (i32.const 0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -4821,9 +4821,9 @@ ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (block $block (result (ref $to-f32-cont)) ;; CHECK-NEXT: (tuple.drop 3 - ;; CHECK-NEXT: (block $block0 (type $34) (result i32 i64 (ref null $simple-cont)) + ;; CHECK-NEXT: (block $block1 (type $34) (result i32 i64 (ref null $simple-cont)) ;; CHECK-NEXT: (local.set $f - ;; CHECK-NEXT: (resume $simple-cont (on $empty $block) (on $tag-pair-to-pair $block0) + ;; CHECK-NEXT: (resume $simple-cont (on $empty $block) (on $tag-pair-to-pair $block1) ;; CHECK-NEXT: (i32.const 0) ;; CHECK-NEXT: (i64.const 1) ;; CHECK-NEXT: (local.get $ct) diff --git a/test/wasm2js/br_table_temp.2asm.js b/test/wasm2js/br_table_temp.2asm.js index 9bc6cafa4f3..a8592a186c4 100644 --- a/test/wasm2js/br_table_temp.2asm.js +++ b/test/wasm2js/br_table_temp.2asm.js @@ -115,7 +115,7 @@ function asmFunc(imports) { function $11($0_1) { $0_1 = $0_1 | 0; var $2_1 = 0, $4_1 = 0, $3_1 = 0; - block0 : { + block1 : { block : { $2_1 = 33; $3_1 = $2_1; @@ -124,7 +124,7 @@ function asmFunc(imports) { case 0: break block; default: - break block0; + break block1; }; } $4_1 = 32; @@ -134,7 +134,7 @@ function asmFunc(imports) { function $12($0_1) { $0_1 = $0_1 | 0; - block3 : { + block4 : { switch ($0_1 | 0) { case 3: return 100 | 0; @@ -145,7 +145,7 @@ function asmFunc(imports) { case 0: return 103 | 0; default: - break block3; + break block4; }; } return 104 | 0; @@ -154,11 +154,11 @@ function asmFunc(imports) { function $13($0_1) { $0_1 = $0_1 | 0; var $1_1 = 0, $3_1 = 0, $4_1 = 0, $5_1 = 0, $6_1 = 0, $7_1 = 0, $8_1 = 0; - block3 : { + block4 : { block : { - block0 : { - block1 : { - block2 : { + block1 : { + block2 : { + block3 : { $3_1 = 200; $4_1 = $3_1; $5_1 = $3_1; @@ -169,13 +169,13 @@ function asmFunc(imports) { case 0: break block; case 1: - break block0; - case 2: break block1; - case 3: + case 2: break block2; - default: + case 3: break block3; + default: + break block4; }; } $1_1 = $7_1; @@ -196,7 +196,7 @@ function asmFunc(imports) { function $14($0_1) { $0_1 = $0_1 | 0; - block0 : { + block1 : { switch ($0_1 | 0) { case 0: case 2: @@ -12508,7 +12508,7 @@ function asmFunc(imports) { case 24614: return 0 | 0; default: - break block0; + break block1; }; } return 1 | 0; @@ -13052,8 +13052,8 @@ function asmFunc(imports) { function $58($0_1) { $0_1 = $0_1 | 0; var $2_1 = 0, $4_1 = 0, $5_1 = 0, $3_1 = 0; - block1 : { - block0 : { + block2 : { + block1 : { block : { $2_1 = 16; $3_1 = $2_1; @@ -13063,9 +13063,9 @@ function asmFunc(imports) { case 0: break block; case 1: - break block0; - default: break block1; + default: + break block2; }; } $4_1 = 2 + $3_1 | 0; @@ -13079,8 +13079,8 @@ function asmFunc(imports) { $0_1 = $0_1 | 0; var $2_1 = 0, $3_1 = 0, $4_1 = 0, $5_1 = 0; block : { - block0 : { - block1 : { + block1 : { + block2 : { $2_1 = 8; $3_1 = $2_1; $4_1 = $2_1; @@ -13089,9 +13089,9 @@ function asmFunc(imports) { case 0: break block; case 1: - break block0; - default: break block1; + default: + break block2; }; } $4_1 = 16; @@ -13104,8 +13104,8 @@ function asmFunc(imports) { function $60($0_1) { $0_1 = $0_1 | 0; var $2_1 = 0, $3_1 = 0, $4_1 = 0, $5_1 = 0; - block1 : { - block0 : { + block2 : { + block1 : { block : { $2_1 = 8; $3_1 = $2_1; @@ -13115,9 +13115,9 @@ function asmFunc(imports) { case 0: break block; case 1: - break block0; - default: break block1; + default: + break block2; }; } $4_1 = 16; @@ -13130,14 +13130,14 @@ function asmFunc(imports) { function $61($0_1) { $0_1 = $0_1 | 0; var $3_1 = 0, $2_1 = 0, $4_1 = 0; - block0 : { + block1 : { block : { $2_1 = 8; $3_1 = $2_1; $4_1 = $2_1; switch ($0_1 | 0) { case 1: - break block0; + break block1; default: break block; }; @@ -13150,8 +13150,8 @@ function asmFunc(imports) { function $62($0_1) { $0_1 = $0_1 | 0; var $2_1 = 0, $3_1 = 0, $4_1 = 0, $5_1 = 0; - block1 : { - block0 : { + block2 : { + block1 : { block : { $2_1 = 8; $3_1 = $2_1; @@ -13161,9 +13161,9 @@ function asmFunc(imports) { case 0: break block; case 1: - break block0; - default: break block1; + default: + break block2; }; } $4_1 = 16; @@ -13176,14 +13176,14 @@ function asmFunc(imports) { function $63($0_1) { $0_1 = $0_1 | 0; var $3_1 = 0, $2_1 = 0, $4_1 = 0; - block0 : { + block1 : { block : { $2_1 = 8; $3_1 = $2_1; $4_1 = $2_1; switch ($0_1 | 0) { case 1: - break block0; + break block1; default: break block; }; diff --git a/test/wasm2js/br_table_temp.2asm.js.opt b/test/wasm2js/br_table_temp.2asm.js.opt index b64e46a77a3..f1dad0ab274 100644 --- a/test/wasm2js/br_table_temp.2asm.js.opt +++ b/test/wasm2js/br_table_temp.2asm.js.opt @@ -60,7 +60,7 @@ function asmFunc(imports) { function $12($0) { $0 = $0 | 0; - block3 : { + block4 : { switch ($0 | 0) { case 3: return 100; @@ -71,7 +71,7 @@ function asmFunc(imports) { case 0: return 103; default: - break block3; + break block4; }; } return 104; @@ -79,7 +79,7 @@ function asmFunc(imports) { function $13($0) { $0 = $0 | 0; - block3 : { + block4 : { switch ($0 | 0) { case 3: return 210; @@ -90,7 +90,7 @@ function asmFunc(imports) { case 0: return 213; default: - break block3; + break block4; }; } return 214; @@ -98,7 +98,7 @@ function asmFunc(imports) { function $14($0) { $0 = $0 | 0; - block0 : { + block1 : { switch ($0 | 0) { case 0: case 2: @@ -12410,7 +12410,7 @@ function asmFunc(imports) { case 24614: return 0; default: - break block0; + break block1; }; } return 1; @@ -12554,7 +12554,7 @@ function asmFunc(imports) { $0 = $0 | 0; var $1 = 0; $1 = 16; - block1 : { + block2 : { switch ($0 | 0) { case 0: $1 = 18; @@ -12562,7 +12562,7 @@ function asmFunc(imports) { $1 = $1 + 1 | 0; break; default: - break block1; + break block2; }; } return $1 | 0; @@ -12590,7 +12590,7 @@ function asmFunc(imports) { $0 = $0 | 0; var $1 = 0; $1 = 8; - block1 : { + block2 : { switch ($0 | 0) { case 0: $1 = 16; @@ -12598,7 +12598,7 @@ function asmFunc(imports) { $1 = $1 + 1 | 0; break; default: - break block1; + break block2; }; } return $1 | 0; diff --git a/test/wasm2js/labels.2asm.js b/test/wasm2js/labels.2asm.js index ac29783006e..431e172d9f0 100644 --- a/test/wasm2js/labels.2asm.js +++ b/test/wasm2js/labels.2asm.js @@ -143,10 +143,6 @@ function asmFunc(imports) { break label; } i = i + 1 | 0; - label0 : { - break label0; - } - i = i + 1 | 0; label1 : { break label1; } @@ -159,6 +155,10 @@ function asmFunc(imports) { break label3; } i = i + 1 | 0; + label4 : { + break label4; + } + i = i + 1 | 0; return i | 0; } From f4695a20dea351c7e32f643900e64b28158f66ff Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Mon, 18 Nov 2024 14:44:57 -0800 Subject: [PATCH 3/3] improve comment --- src/passes/Outlining.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/passes/Outlining.cpp b/src/passes/Outlining.cpp index 85abb57e422..b86e6cd17e1 100644 --- a/src/passes/Outlining.cpp +++ b/src/passes/Outlining.cpp @@ -305,9 +305,10 @@ struct Outlining : public Pass { // the outlining lit tests far more readable. moveOutlinedFunctions(module, substrings.size()); - // Because we visit control flow in an odd order, IRBuilder is not able to - // properly track branches, so it may not have finalized blocks with the - // correct types. ReFinalize now to fix any issues. + // Because we visit control flow in stringified order rather than normal + // postorder, IRBuilder is not able to properly track branches, so it may + // not have finalized blocks with the correct types. ReFinalize now to fix + // any issues. PassRunner runner(getPassRunner()); runner.add(std::make_unique()); runner.run();