From 51c40c761a044321a50935c9d312c7d9678192b2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 11:58:49 -0700 Subject: [PATCH 01/61] work --- src/passes/BranchHintAnalysis.cpp | 101 ++++++++++++++++++++++++++++++ src/passes/CMakeLists.txt | 1 + src/passes/pass.cpp | 3 + src/passes/passes.h | 1 + 4 files changed, 106 insertions(+) create mode 100644 src/passes/BranchHintAnalysis.cpp diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp new file mode 100644 index 00000000000..425f4263f1b --- /dev/null +++ b/src/passes/BranchHintAnalysis.cpp @@ -0,0 +1,101 @@ +/* + * Copyright 2025 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// Infer values for Branch Hints and emit the custom section with those +// annotations. +// +// Inspired by LLVM's BranchProbabilityInfo: +// https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/BranchProbabilityInfo.cpp +// + +#include "cfg/cfg-traversal.h" +#include "pass.h" +#include "wasm-builder.h" +#include "wasm.h" + +namespace wasm { + +namespace { + +// In each basic block we will store instructions that either branch, or that +// provide hints as to branching. +struct Info { + std::vector actions; +}; + +struct BranchHintAnalysis + : public WalkerPass< + CFGWalker, Info>> { + bool isFunctionParallel() override { return true; } + + // Locals are not modified here. + bool requiresNonNullableLocalFixups() override { return false; } + + std::unique_ptr create() override { + return std::make_unique(); + } + + // We only look at things that branch twice, which is all branching + // instructions but without br (without condition, which is an unconditional + // branch we don't need to hint about) and not switch (which Branch Hints do + // not support). + bool branches(Expression* curr) { + if (auto* br = curr->dynCast()) { + return !!br->condition; + } + return curr->is() || curr->is(); + } + + // An abstract probability of code being reached, in the range 0 - 100. + using Probability = uint8_t; + + // Returns the probability that an instruction is reached, if something about + // it suggests it is likely or not. + std::optional getProbability(Expression* curr) { + // Unreachable is assumed to never happen. + if (curr->is()) { + return 0; + } + + // Throw is assumed to almost never happen. + if (curr->is() || curr->is()) { + return 1; + } + + // Otherwise, we don't know. + // TODO: cold calls, noreturn calls, etc. + return {}; + } + + void visitExpression(Expression* curr) { + // Add all (reachable, so |currBasicBlock| exists) things that either branch + // or suggest probabilities of branching. + if (currBasicBlock && (branches(curr) || getProbability(curr))) { + currBasicBlock->contents.actions.push_back(getCurrentPointer()); + } + } + + void visitFunction(Function* curr) { + // Now that the walk is complete and we have a CFG, find things to optimize. + } +}; + +} // anonymous namespace + +Pass* createBranchHintAnalysisPass() { return new BranchHintAnalysis(); } + +} // namespace wasm diff --git a/src/passes/CMakeLists.txt b/src/passes/CMakeLists.txt index 6fab09b1bc2..9370060cf50 100644 --- a/src/passes/CMakeLists.txt +++ b/src/passes/CMakeLists.txt @@ -21,6 +21,7 @@ set(passes_SOURCES AlignmentLowering.cpp Asyncify.cpp AvoidReinterprets.cpp + BranchHintAnalysis.cpp CoalesceLocals.cpp CodePushing.cpp CodeFolding.cpp diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 2042bc71d3a..e602fc7092d 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -99,6 +99,9 @@ void PassRegistry::registerPasses() { registerPass("avoid-reinterprets", "Tries to avoid reinterpret operations via more loads", createAvoidReinterpretsPass); + registerPass("branch-hint-analysis", + "Infers branch hints and emits branch hinting section", + createBranchHintAnalysisPass); registerPass( "dae", "removes arguments to calls in an lto-like manner", createDAEPass); registerPass("dae-optimizing", diff --git a/src/passes/passes.h b/src/passes/passes.h index e051e466e72..e871be68faf 100644 --- a/src/passes/passes.h +++ b/src/passes/passes.h @@ -26,6 +26,7 @@ Pass* createAbstractTypeRefiningPass(); Pass* createAlignmentLoweringPass(); Pass* createAsyncifyPass(); Pass* createAvoidReinterpretsPass(); +Pass* createBranchHintAnalysisPass(); Pass* createCoalesceLocalsPass(); Pass* createCoalesceLocalsWithLearningPass(); Pass* createCodeFoldingPass(); From be1ed316c7c6911af94357b1dc346010772e7205 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 13:39:49 -0700 Subject: [PATCH 02/61] go --- src/passes/BranchHintAnalysis.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 425f4263f1b..31d95c18fed 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -91,6 +91,12 @@ struct BranchHintAnalysis void visitFunction(Function* curr) { // Now that the walk is complete and we have a CFG, find things to optimize. + for (auto& block : basicBlocks) { + std::cout << "block\n"; + for (auto** currp : block->contents.actions) { + std::cout << " " << **currp << "\n"; + } + } } }; From 6a33a99d65a02d61fcb85b47ac72dccc9ca6b93b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 13:44:34 -0700 Subject: [PATCH 03/61] go --- src/passes/BranchHintAnalysis.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 31d95c18fed..10acaaaa1fe 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -53,7 +53,7 @@ struct BranchHintAnalysis // instructions but without br (without condition, which is an unconditional // branch we don't need to hint about) and not switch (which Branch Hints do // not support). - bool branches(Expression* curr) { + bool isBranching(Expression* curr) { if (auto* br = curr->dynCast()) { return !!br->condition; } @@ -84,7 +84,7 @@ struct BranchHintAnalysis void visitExpression(Expression* curr) { // Add all (reachable, so |currBasicBlock| exists) things that either branch // or suggest probabilities of branching. - if (currBasicBlock && (branches(curr) || getProbability(curr))) { + if (currBasicBlock && (isBranching(curr) || getProbability(curr))) { currBasicBlock->contents.actions.push_back(getCurrentPointer()); } } From 7f48996f906b444b8707484efe26a36e18dd8892 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 13:48:05 -0700 Subject: [PATCH 04/61] test --- test/lit/passes/branch-hint-analysis.wast | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/lit/passes/branch-hint-analysis.wast diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast new file mode 100644 index 00000000000..7fb8fddd84b --- /dev/null +++ b/test/lit/passes/branch-hint-analysis.wast @@ -0,0 +1,22 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --branch-hint-analysis -all -S -o - \ +;; RUN: | filecheck %s +;; + +(module + (tag $e) + + (func $brs (param $x i32) + (block $block + (br_if $block + (local.get $x) + ) + (nop) + (br_if $block + (local.get $x) + ) + (throw $e) + ) + (unreachable) + ) +) From cd6904b625cac4b10f2cb2d6a41d8a2f1b2b9ffa Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 13:59:10 -0700 Subject: [PATCH 05/61] go --- src/passes/BranchHintAnalysis.cpp | 28 +++++++++++++++++------ test/lit/passes/branch-hint-analysis.wast | 14 ++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 10acaaaa1fe..8f2bed18cbc 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -60,12 +60,14 @@ struct BranchHintAnalysis return curr->is() || curr->is(); } - // An abstract probability of code being reached, in the range 0 - 100. - using Probability = uint8_t; + // An abstract chance (probability, but in less letters) of code being + // reached, in the range 0 - 100. + using Chance = uint8_t; + static constexpr Chance MaxChance = 100; - // Returns the probability that an instruction is reached, if something about + // Returns the chance that an instruction is reached, if something about // it suggests it is likely or not. - std::optional getProbability(Expression* curr) { + std::optional getChance(Expression* curr) { // Unreachable is assumed to never happen. if (curr->is()) { return 0; @@ -83,19 +85,31 @@ struct BranchHintAnalysis void visitExpression(Expression* curr) { // Add all (reachable, so |currBasicBlock| exists) things that either branch - // or suggest probabilities of branching. - if (currBasicBlock && (isBranching(curr) || getProbability(curr))) { + // or suggest chances of branching. + if (currBasicBlock && (isBranching(curr) || getChance(curr))) { currBasicBlock->contents.actions.push_back(getCurrentPointer()); } } void visitFunction(Function* curr) { // Now that the walk is complete and we have a CFG, find things to optimize. - for (auto& block : basicBlocks) { + // First, compute the chance of each basic block from its contents. + std::vector blockChances(basicBlocks.size(), MaxChance); + for (Index i = 0; i < basicBlocks.size(); ++i) { std::cout << "block\n"; + auto& block = basicBlocks[i]; + auto& chance = blockChances[i]; for (auto** currp : block->contents.actions) { std::cout << " " << **currp << "\n"; + // The chance of a basic block is the lowest thing we can find: if + // we see nop, call, unreachable, then the nop tells us nothing, the + // call may suggests a low chance if it is cold, but the + // unreachable suggests a very low chance, which we trust. + if (auto currChance = getChance(*currp)) { + chance = std::min(chance, *currChance); + } } + std::cout << " => " << chance << "\n"; } } }; diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 7fb8fddd84b..71510df102d 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -4,8 +4,22 @@ ;; (module + ;; CHECK: (tag $e (type $0)) (tag $e) + ;; CHECK: (func $brs (type $1) (param $x i32) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) (func $brs (param $x i32) (block $block (br_if $block From 334f8e0c24be24856894fdf32f994c9cbb3e3c98 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 14:03:03 -0700 Subject: [PATCH 06/61] go --- src/passes/BranchHintAnalysis.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 8f2bed18cbc..9db99760dcc 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -109,8 +109,12 @@ struct BranchHintAnalysis chance = std::min(chance, *currChance); } } - std::cout << " => " << chance << "\n"; + std::cout << " => " << int(chance) << "\n"; } + + // We consider the chance of a block to be no higher than the things it + // targets, that is, chance(block) := max(chance(target) for target). Flow + // chances to sources of blocks to achieve that. } }; From f53e343adf6a0f3d01e816672c31885e9645cb6d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 14:09:31 -0700 Subject: [PATCH 07/61] go --- src/passes/BranchHintAnalysis.cpp | 51 +++++++++++++++++------ test/lit/passes/branch-hint-analysis.wast | 2 + 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 9db99760dcc..782b430cb04 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -24,6 +24,7 @@ #include "cfg/cfg-traversal.h" #include "pass.h" +#include "unique_deferring_queue.h" #include "wasm-builder.h" #include "wasm.h" @@ -31,10 +32,19 @@ namespace wasm { namespace { -// In each basic block we will store instructions that either branch, or that -// provide hints as to branching. +// An abstract chance (probability, but in less letters) of code being +// reached, in the range 0 - 100. +using Chance = uint8_t; +static constexpr Chance MaxChance = 100; + struct Info { + // In each basic block we will store instructions that either branch, or that + // provide hints as to branching. std::vector actions; + + // The chance of the block being reached. We assume any can be reached, unless + // we see a good hint otherwise. + Chance chance = MaxChance; }; struct BranchHintAnalysis @@ -60,11 +70,6 @@ struct BranchHintAnalysis return curr->is() || curr->is(); } - // An abstract chance (probability, but in less letters) of code being - // reached, in the range 0 - 100. - using Chance = uint8_t; - static constexpr Chance MaxChance = 100; - // Returns the chance that an instruction is reached, if something about // it suggests it is likely or not. std::optional getChance(Expression* curr) { @@ -94,27 +99,47 @@ struct BranchHintAnalysis void visitFunction(Function* curr) { // Now that the walk is complete and we have a CFG, find things to optimize. // First, compute the chance of each basic block from its contents. - std::vector blockChances(basicBlocks.size(), MaxChance); for (Index i = 0; i < basicBlocks.size(); ++i) { std::cout << "block\n"; auto& block = basicBlocks[i]; - auto& chance = blockChances[i]; for (auto** currp : block->contents.actions) { std::cout << " " << **currp << "\n"; // The chance of a basic block is the lowest thing we can find: if // we see nop, call, unreachable, then the nop tells us nothing, the // call may suggests a low chance if it is cold, but the // unreachable suggests a very low chance, which we trust. - if (auto currChance = getChance(*currp)) { - chance = std::min(chance, *currChance); + if (auto chance = getChance(*currp)) { + block->chance = std::min(block->chance, *chance); } } - std::cout << " => " << int(chance) << "\n"; + std::cout << " => " << int(block->chance) << "\n"; } // We consider the chance of a block to be no higher than the things it // targets, that is, chance(block) := max(chance(target) for target). Flow - // chances to sources of blocks to achieve that. + // chances to sources of blocks to achieve that, starting from the indexes + // of all blocks. + UniqueDeferredQueue work; + for (auto& block : basicBlocks) { + work.push(&block); + } + while (!work.empty()) { + auto* block = work.pop(); + // Apply this block to its predecessors, potentially raising their + // chances. + for (auto* in : block->in) { + if (block->chance > in->chance) { + in->chance = block->chance; + work.push(in); + } + } + } + + for (Index i = 0; i < basicBlocks.size(); ++i) { + std::cout << "2block\n"; + auto& block = basicBlocks[i]; + std::cout << " => " << int(block->chance) << "\n"; + } } }; diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 71510df102d..61f208ab62c 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -34,3 +34,5 @@ (unreachable) ) ) + +;; TODO: calls etc. From 0eb1114bc213381be04c3201ef08bf6185cce390 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 14:10:45 -0700 Subject: [PATCH 08/61] go --- src/passes/BranchHintAnalysis.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 782b430cb04..daaabfbff49 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -24,7 +24,7 @@ #include "cfg/cfg-traversal.h" #include "pass.h" -#include "unique_deferring_queue.h" +#include "support/unique_deferring_queue.h" #include "wasm-builder.h" #include "wasm.h" @@ -109,10 +109,10 @@ struct BranchHintAnalysis // call may suggests a low chance if it is cold, but the // unreachable suggests a very low chance, which we trust. if (auto chance = getChance(*currp)) { - block->chance = std::min(block->chance, *chance); + block->contents.chance = std::min(block->contents.chance, *chance); } } - std::cout << " => " << int(block->chance) << "\n"; + std::cout << " => " << int(block->contents.chance) << "\n"; } // We consider the chance of a block to be no higher than the things it @@ -121,15 +121,15 @@ struct BranchHintAnalysis // of all blocks. UniqueDeferredQueue work; for (auto& block : basicBlocks) { - work.push(&block); + work.push(block.get()); } while (!work.empty()) { auto* block = work.pop(); // Apply this block to its predecessors, potentially raising their // chances. for (auto* in : block->in) { - if (block->chance > in->chance) { - in->chance = block->chance; + if (block->contents.chance > in->contents.chance) { + in->contents.chance = block->contents.chance; work.push(in); } } @@ -138,7 +138,7 @@ struct BranchHintAnalysis for (Index i = 0; i < basicBlocks.size(); ++i) { std::cout << "2block\n"; auto& block = basicBlocks[i]; - std::cout << " => " << int(block->chance) << "\n"; + std::cout << " => " << int(block->contents.chance) << "\n"; } } }; From cc5598aa19f8e828233e15af6cadecc3c9df4dad Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 14:16:37 -0700 Subject: [PATCH 09/61] go --- src/passes/BranchHintAnalysis.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index daaabfbff49..218f9a6816e 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -135,11 +135,37 @@ struct BranchHintAnalysis } } + // Debug for (Index i = 0; i < basicBlocks.size(); ++i) { std::cout << "2block\n"; auto& block = basicBlocks[i]; std::cout << " => " << int(block->contents.chance) << "\n"; } + + // Apply the final chances: when a branch between two options has a higher + // higher chance to go one way then the other, mark it as likely or unlikely + // accordingly. TODO: should we not mark when the difference is small? + for (auto& block : basicBlocks) { + if (block->contents.actions.empty() || block->out.size() != 2) { + continue; + } + + auto* last = block->contents.actions.back(); + if (!isBranching(last)) { + continue; + } + + // Compare the probabilities of the two targets. + auto firstChance = block->out[0]->contents.chance; + auto secondChance = block->out[1]->contents.chance; + if (firstChance == secondChance) { + continue; + } + + // We have a useful hint! + curr->codeAnnotations[last].branchLikely = (firstChance < secondChance); + // XXX order? + } } }; From c10420302206a0091fc6f43db9f7ee8fc7621fbb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 14:18:40 -0700 Subject: [PATCH 10/61] go --- src/passes/BranchHintAnalysis.cpp | 2 +- test/lit/passes/branch-hint-analysis.wast | 36 ++++++++++++++++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 218f9a6816e..58eb5dac410 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -150,7 +150,7 @@ struct BranchHintAnalysis continue; } - auto* last = block->contents.actions.back(); + auto* last = *block->contents.actions.back(); if (!isBranching(last)) { continue; } diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 61f208ab62c..86f03002651 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -4,34 +4,48 @@ ;; (module - ;; CHECK: (tag $e (type $0)) + ;; CHECK: (tag $e (type $1)) (tag $e) - ;; CHECK: (func $brs (type $1) (param $x i32) + ;; CHECK: (func $br-unreachable (type $0) (param $x i32) ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (br_if $block ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (br_if $block - ;; CHECK-NEXT: (local.get $x) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: (return) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) - (func $brs (param $x i32) + (func $br-unreachable (param $x i32) + ;; The br_if is unlikely, as it reaches an unreachable. (block $block (br_if $block (local.get $x) ) - (nop) + (return) + ) + (unreachable) + ) + + ;; CHECK: (func $br-unreachable-flip (type $0) (param $x i32) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $br-unreachable-flip (param $x i32) + ;; As above, but flipped: the hint should be reversed, since the unreachable + ;; is if we do not branch. + (block $block (br_if $block (local.get $x) ) - (throw $e) + (unreachable) ) - (unreachable) ) ) From a1e4d6b2cd3a1f98c45bbf8a8e3045ef05c87051 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 16 May 2025 16:42:41 -0700 Subject: [PATCH 11/61] todos --- src/cfg/cfg-traversal.h | 12 +++++- src/passes/BranchHintAnalysis.cpp | 9 ++++- test/lit/passes/branch-hint-analysis.wast | 49 ++++++++++++++++++++++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h index 46ecc4b2a30..f4037f3e828 100644 --- a/src/cfg/cfg-traversal.h +++ b/src/cfg/cfg-traversal.h @@ -196,6 +196,14 @@ struct CFGWalker : public PostWalker { self->startBasicBlock()); // before if -> ifFalse } + // Optional hook that is called on each branch between two paths. This + // receives the branching instruction and the two blocks it can reach. For + // example, for an if, the blocks it can reach correspond to the ifTrue and + // ifFalse targets. + void noteBranch(Expression* curr, BasicBlock* ifTrue, BasicBlock* ifFalse) { + // Do nothing by default. + } + static void doEndIf(SubType* self, Expression** currp) { auto* last = self->currBasicBlock; self->startBasicBlock(); @@ -488,7 +496,7 @@ struct CFGWalker : public PostWalker { self->pushTask(SubType::scan, &curr->cast()->ifTrue); self->pushTask(SubType::doStartIfTrue, currp); self->pushTask(SubType::scan, &curr->cast()->condition); - return; // don't do anything else + break; } case Expression::Id::LoopId: { self->pushTask(SubType::doEndLoop, currp); @@ -522,7 +530,7 @@ struct CFGWalker : public PostWalker { self->pushTask(SubType::doStartCatches, currp); self->pushTask(SubType::scan, &curr->cast()->body); self->pushTask(SubType::doStartTry, currp); - return; // don't do anything else + break; } case Expression::Id::TryTableId: { self->pushTask(SubType::doEndTryTable, currp); diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 58eb5dac410..bb98ad81495 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -64,9 +64,11 @@ struct BranchHintAnalysis // branch we don't need to hint about) and not switch (which Branch Hints do // not support). bool isBranching(Expression* curr) { +std::cout << "chak isBranching " << *curr << '\n'; if (auto* br = curr->dynCast()) { return !!br->condition; } +std::cout << "chak isBranching2 " << *curr << " : " << (curr->is() || curr->is()) << '\n'; return curr->is() || curr->is(); } @@ -89,9 +91,13 @@ struct BranchHintAnalysis } void visitExpression(Expression* curr) { +std::cout << "visit " << *curr << " : " << currBasicBlock << '\n'; // Add all (reachable, so |currBasicBlock| exists) things that either branch // or suggest chances of branching. - if (currBasicBlock && (isBranching(curr) || getChance(curr))) { + auto cond = currBasicBlock && (isBranching(curr) || getChance(curr)); + std::cout << "cond: " << cond << '\n'; + if (cond) { +std::cout << " add!\n"; currBasicBlock->contents.actions.push_back(getCurrentPointer()); } } @@ -165,6 +171,7 @@ struct BranchHintAnalysis // We have a useful hint! curr->codeAnnotations[last].branchLikely = (firstChance < secondChance); // XXX order? + // XXX messy to assume the order... and if doesn't even visit in the right time... instead, add explicit hooks, called from doFinishIf in cfg-trav etc. } } }; diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 86f03002651..44647f8cb1a 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -47,6 +47,53 @@ (unreachable) ) ) + + ;; CHECK: (func $if-unreachable (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-unreachable (param $x i32) + ;; The unreachable branch should be marked unlikely. + (if + (local.get $x) + (then + (unreachable) + ) + (else + (nop) + ) + ) + ) + + ;; CHECK: (func $if-unreachable-flip (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-unreachable-flip (param $x i32) + ;; As above, but flipped, so the hint is flipped too. + (if + (local.get $x) + (then + (nop) + ) + (else + (unreachable) + ) + ) + ) ) -;; TODO: calls etc. From cac418b604cb6f08a11cabc4331894a4708541eb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:24:20 -0700 Subject: [PATCH 12/61] undo --- src/cfg/cfg-traversal.h | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h index f4037f3e828..46ecc4b2a30 100644 --- a/src/cfg/cfg-traversal.h +++ b/src/cfg/cfg-traversal.h @@ -196,14 +196,6 @@ struct CFGWalker : public PostWalker { self->startBasicBlock()); // before if -> ifFalse } - // Optional hook that is called on each branch between two paths. This - // receives the branching instruction and the two blocks it can reach. For - // example, for an if, the blocks it can reach correspond to the ifTrue and - // ifFalse targets. - void noteBranch(Expression* curr, BasicBlock* ifTrue, BasicBlock* ifFalse) { - // Do nothing by default. - } - static void doEndIf(SubType* self, Expression** currp) { auto* last = self->currBasicBlock; self->startBasicBlock(); @@ -496,7 +488,7 @@ struct CFGWalker : public PostWalker { self->pushTask(SubType::scan, &curr->cast()->ifTrue); self->pushTask(SubType::doStartIfTrue, currp); self->pushTask(SubType::scan, &curr->cast()->condition); - break; + return; // don't do anything else } case Expression::Id::LoopId: { self->pushTask(SubType::doEndLoop, currp); @@ -530,7 +522,7 @@ struct CFGWalker : public PostWalker { self->pushTask(SubType::doStartCatches, currp); self->pushTask(SubType::scan, &curr->cast()->body); self->pushTask(SubType::doStartTry, currp); - break; + return; // don't do anything else } case Expression::Id::TryTableId: { self->pushTask(SubType::doEndTryTable, currp); From c05cf0b83d41c7b2df0f9a3ca228a37f67272720 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:28:55 -0700 Subject: [PATCH 13/61] todos --- src/passes/BranchHintAnalysis.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index bb98ad81495..af97d3e3e67 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -40,7 +40,7 @@ static constexpr Chance MaxChance = 100; struct Info { // In each basic block we will store instructions that either branch, or that // provide hints as to branching. - std::vector actions; + std::vector actions; // TODO * not **? // The chance of the block being reached. We assume any can be reached, unless // we see a good hint otherwise. @@ -59,6 +59,9 @@ struct BranchHintAnalysis return std::make_unique(); } + using Super = WalkerPass< + CFGWalker, Info>>; + // We only look at things that branch twice, which is all branching // instructions but without br (without condition, which is an unconditional // branch we don't need to hint about) and not switch (which Branch Hints do @@ -102,6 +105,19 @@ std::cout << " add!\n"; } } + // Override cfg-analysis's handling of If start. Ifs are control flow + // structures, so they do not appear in basic blocks (an If spans several), + // but we do need to know where the If begins, specifically, where the + // condition can branch + static void doStartIfTrue(SubType* self, Expression** currp) { + // Right before the Super creates a basic block for the ifTrue, note the + // basic block the condition is in. + if (self->currBasicBlock) { + currBasicBlock->contents.actions.push_back(currp); + } + Super::doStartIfTrue(self, currp); + } + void visitFunction(Function* curr) { // Now that the walk is complete and we have a CFG, find things to optimize. // First, compute the chance of each basic block from its contents. From 58d21d08c7aa4a33149c886a82c55738da410db1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:30:15 -0700 Subject: [PATCH 14/61] work --- src/passes/BranchHintAnalysis.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index af97d3e3e67..954cbb34e0a 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -109,11 +109,11 @@ std::cout << " add!\n"; // structures, so they do not appear in basic blocks (an If spans several), // but we do need to know where the If begins, specifically, where the // condition can branch - static void doStartIfTrue(SubType* self, Expression** currp) { + static void doStartIfTrue(BranchHintAnalysis* self, Expression** currp) { // Right before the Super creates a basic block for the ifTrue, note the // basic block the condition is in. if (self->currBasicBlock) { - currBasicBlock->contents.actions.push_back(currp); + self->currBasicBlock->contents.actions.push_back(currp); } Super::doStartIfTrue(self, currp); } From 0d4aee2859e70dd308491bf345f78dac20a023d0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:37:54 -0700 Subject: [PATCH 15/61] work --- src/passes/BranchHintAnalysis.cpp | 22 ++++++++++------------ test/lit/passes/branch-hint-analysis.wast | 2 ++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 954cbb34e0a..fcdc605a283 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -67,11 +67,9 @@ struct BranchHintAnalysis // branch we don't need to hint about) and not switch (which Branch Hints do // not support). bool isBranching(Expression* curr) { -std::cout << "chak isBranching " << *curr << '\n'; if (auto* br = curr->dynCast()) { return !!br->condition; } -std::cout << "chak isBranching2 " << *curr << " : " << (curr->is() || curr->is()) << '\n'; return curr->is() || curr->is(); } @@ -94,13 +92,9 @@ std::cout << "chak isBranching2 " << *curr << " : " << (curr->is() || curr-> } void visitExpression(Expression* curr) { -std::cout << "visit " << *curr << " : " << currBasicBlock << '\n'; // Add all (reachable, so |currBasicBlock| exists) things that either branch // or suggest chances of branching. - auto cond = currBasicBlock && (isBranching(curr) || getChance(curr)); - std::cout << "cond: " << cond << '\n'; - if (cond) { -std::cout << " add!\n"; + if (currBasicBlock && (isBranching(curr) || getChance(curr))) { currBasicBlock->contents.actions.push_back(getCurrentPointer()); } } @@ -122,10 +116,10 @@ std::cout << " add!\n"; // Now that the walk is complete and we have a CFG, find things to optimize. // First, compute the chance of each basic block from its contents. for (Index i = 0; i < basicBlocks.size(); ++i) { - std::cout << "block\n"; +std::cout << "block\n"; auto& block = basicBlocks[i]; for (auto** currp : block->contents.actions) { - std::cout << " " << **currp << "\n"; +std::cout << " " << **currp << "\n"; // The chance of a basic block is the lowest thing we can find: if // we see nop, call, unreachable, then the nop tells us nothing, the // call may suggests a low chance if it is cold, but the @@ -134,7 +128,7 @@ std::cout << " add!\n"; block->contents.chance = std::min(block->contents.chance, *chance); } } - std::cout << " => " << int(block->contents.chance) << "\n"; +std::cout << " => " << int(block->contents.chance) << "\n"; } // We consider the chance of a block to be no higher than the things it @@ -159,30 +153,34 @@ std::cout << " add!\n"; // Debug for (Index i = 0; i < basicBlocks.size(); ++i) { - std::cout << "2block\n"; +std::cout << "2block\n"; auto& block = basicBlocks[i]; - std::cout << " => " << int(block->contents.chance) << "\n"; +std::cout << " => " << int(block->contents.chance) << "\n"; } // Apply the final chances: when a branch between two options has a higher // higher chance to go one way then the other, mark it as likely or unlikely // accordingly. TODO: should we not mark when the difference is small? for (auto& block : basicBlocks) { +std::cout << "lastloop block\n"; if (block->contents.actions.empty() || block->out.size() != 2) { continue; } auto* last = *block->contents.actions.back(); +std::cout << " last " << *last << "\n"; if (!isBranching(last)) { continue; } +std::cout << " chances1\n"; // Compare the probabilities of the two targets. auto firstChance = block->out[0]->contents.chance; auto secondChance = block->out[1]->contents.chance; if (firstChance == secondChance) { continue; } +std::cout << " chances2\n"; // We have a useful hint! curr->codeAnnotations[last].branchLikely = (firstChance < secondChance); diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 44647f8cb1a..acb97ff92a5 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -49,6 +49,7 @@ ) ;; CHECK: (func $if-unreachable (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then @@ -73,6 +74,7 @@ ) ;; CHECK: (func $if-unreachable-flip (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then From 756f0a12e0f9ad38e112d9ded3ea27f0f2580d1b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:53:18 -0700 Subject: [PATCH 16/61] work --- src/passes/BranchHintAnalysis.cpp | 37 +++++++++++++++++------ test/lit/passes/branch-hint-analysis.wast | 4 +-- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index fcdc605a283..1ac37963449 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -174,19 +174,36 @@ std::cout << " last " << *last << "\n"; } std::cout << " chances1\n"; - // Compare the probabilities of the two targets. - auto firstChance = block->out[0]->contents.chance; - auto secondChance = block->out[1]->contents.chance; - if (firstChance == secondChance) { - continue; + // Compare the probabilities of the two targets and see if we can infer + // likelihood. + if (auto likely = getLikelihood(last, + block->out[0]->contents.chance, + block->out[1]->contents.chance)) { + // We have a useful hint! + curr->codeAnnotations[last].branchLikely = likely; } -std::cout << " chances2\n"; + } + } + + // Checks if a branch from a branching instruction to two targets is likely or + // not (or unknown). + std::optional getLikelihood(Expression* brancher, + Chance first, + Chance second) { + if (first == second) { + // No data to suggest likelihood either way. + return {}; + } - // We have a useful hint! - curr->codeAnnotations[last].branchLikely = (firstChance < secondChance); - // XXX order? - // XXX messy to assume the order... and if doesn't even visit in the right time... instead, add explicit hooks, called from doFinishIf in cfg-trav etc. + // |first, second| are the chances of the basic blocks after the brancher, + // in order. For Br*, the block right after them is reached if we do *not* + // branch (the condition is false), and the later block if we do, while for + // If, it is reversed: the block right after us is reached if the condition + // is true. + if (brancher->is()) { + std::swap(first, second); } + return first < second; } }; diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index acb97ff92a5..1b3d8a38d4f 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -49,7 +49,7 @@ ) ;; CHECK: (func $if-unreachable (type $0) (param $x i32) - ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then @@ -74,7 +74,7 @@ ) ;; CHECK: (func $if-unreachable-flip (type $0) (param $x i32) - ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then From c01f4135896d5165e7dbbbfa9a4d7086023a93ce Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 13:54:06 -0700 Subject: [PATCH 17/61] work --- test/lit/passes/branch-hint-analysis.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 1b3d8a38d4f..9b185b348f9 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -61,7 +61,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $if-unreachable (param $x i32) - ;; The unreachable branch should be marked unlikely. + ;; The unreachable means the condition is unlikely. (if (local.get $x) (then From a25ff450219b50e852286f0ec83a9fe0072b44fe Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:37:40 -0700 Subject: [PATCH 18/61] work --- test/lit/passes/branch-hint-analysis.wast | 52 ++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 9b185b348f9..a2bbb209bcb 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -4,7 +4,7 @@ ;; (module - ;; CHECK: (tag $e (type $1)) + ;; CHECK: (tag $e (type $2)) (tag $e) ;; CHECK: (func $br-unreachable (type $0) (param $x i32) @@ -48,6 +48,56 @@ ) ) + ;; CHECK: (func $br_on-unreachable (type $1) (param $x anyref) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block $block (result (ref any)) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (br_on_non_null $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $br_on-unreachable (param $x anyref) + ;; The br_on is unlikely, as it reaches an unreachable. + (drop + (block $block (result (ref any)) + (br_on_non_null $block + (local.get $x) + ) + (return) + ) + (unreachable) + ) + ) + + ;; CHECK: (func $br_on-unreachable-flip (type $1) (param $x anyref) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block $block (result (ref any)) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (br_on_non_null $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $br_on-unreachable-flip (param $x anyref) + ;; As above, but flipped, so the hint is flipped too. + (drop + (block $block (result (ref any)) + (br_on_non_null $block + (local.get $x) + ) + (unreachable) + ) + ) + ) + ;; CHECK: (func $if-unreachable (type $0) (param $x i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if From b4039a261311295472e4b98a6a8ac1d20a5474e5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:41:09 -0700 Subject: [PATCH 19/61] work --- src/passes/BranchHintAnalysis.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 1ac37963449..2702b88a71b 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -35,6 +35,8 @@ namespace { // An abstract chance (probability, but in less letters) of code being // reached, in the range 0 - 100. using Chance = uint8_t; +static constexpr Chance MinChance = 0; +static constexpr Chance TinyChance = 1; static constexpr Chance MaxChance = 100; struct Info { @@ -78,12 +80,12 @@ struct BranchHintAnalysis std::optional getChance(Expression* curr) { // Unreachable is assumed to never happen. if (curr->is()) { - return 0; + return MinChance; } // Throw is assumed to almost never happen. if (curr->is() || curr->is()) { - return 1; + return TinyChance; } // Otherwise, we don't know. From e2145f8035c1c6ef432228d14106e24e38d4bda2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:43:37 -0700 Subject: [PATCH 20/61] work --- test/lit/passes/branch-hint-analysis.wast | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index a2bbb209bcb..c8e2d0f2521 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -147,5 +147,55 @@ ) ) ) + + ;; CHECK: (func $if-throw (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-throw (param $x i32) + ;; A throw is unlikely. + (if + (local.get $x) + (then + (throw $e) + ) + (else + (nop) + ) + ) + ) + + ;; CHECK: (func $if-throw-unreachable (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-throw-unreachable (param $x i32) + ;; A throw is less likely than an unreachable. + (if + (local.get $x) + (then + (throw $e) + ) + (else + (unreachable) + ) + ) + ) ) From 71feef87a8076dd02946ac342ef92901c703143d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:44:55 -0700 Subject: [PATCH 21/61] work --- test/lit/passes/branch-hint-analysis.wast | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index c8e2d0f2521..a9731fbb0ab 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -173,6 +173,35 @@ ) ) + ;; CHECK: (func $if-throw_ref (type $3) (param $x i32) (param $y exnref) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (throw_ref + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-throw_ref (param $x i32) (param $y exnref) + ;; A throw is unlikely. + (if + (local.get $x) + (then + (throw_ref + (local.get $y) + ) + ) + (else + (nop) + ) + ) + ) + ;; CHECK: (func $if-throw-unreachable (type $0) (param $x i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if From 6fa5df1c4268a9ab7fb1a0ac1046c9558427c8c3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:46:16 -0700 Subject: [PATCH 22/61] work --- test/lit/passes/branch-hint-analysis.wast | 40 +++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index a9731fbb0ab..1ad1df62204 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -148,6 +148,46 @@ ) ) + ;; CHECK: (func $if-unreachable-one-arm (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-unreachable-one-arm (param $x i32) + ;; The unreachable means the condition is unlikely. + (if + (local.get $x) + (then + (unreachable) + ) + ) + ) + + ;; CHECK: (func $if-one-arm-unreachable-later (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $if-one-arm-unreachable-later (param $x i32) + ;; The unreachable after means the condition is likely. + (if + (local.get $x) + (then + (return) + ) + ) + (unreachable) + ) + ;; CHECK: (func $if-throw (type $0) (param $x i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if From 0bdcb36101d466fb23d4b67ed2078321265012fb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:47:33 -0700 Subject: [PATCH 23/61] work --- test/lit/passes/branch-hint-analysis.wast | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 1ad1df62204..2ccead4415f 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -266,5 +266,29 @@ ) ) ) + + ;; CHECK: (func $if-unreachable-double (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-unreachable-double (param $x i32) + ;; Two unreachables are equally unlikely, so nothing to hint. + (if + (local.get $x) + (then + (unreachable) + ) + (else + (unreachable) + ) + ) + ) ) From 34977602418d5848d9badae55575310784bcf817 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:49:29 -0700 Subject: [PATCH 24/61] work --- test/lit/passes/branch-hint-analysis.wast | 36 +++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 2ccead4415f..00fca2803ec 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -290,5 +290,41 @@ ) ) ) + + ;; CHECK: (func $if-nesting (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-nesting (param $x i32) + ;; We know nothing for this if. + (if + (local.get $x) + (then + (return) + ) + (else + ;; This condition is likely false. + (if + (local.get $x) + (then + (unreachable) + ) + ) + ) + ) + ) ) From dea45ca6f8415815e498541be1190c3683072c76 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 May 2025 16:51:13 -0700 Subject: [PATCH 25/61] work --- test/lit/passes/branch-hint-analysis.wast | 41 +++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 00fca2803ec..a00f99fbb72 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -326,5 +326,46 @@ ) ) ) + + ;; CHECK: (func $if-nesting-2 (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $if-nesting-2 (param $x i32) + ;; The else is unlikely, so we hint here. + (if + (local.get $x) + (then + (return) + ) + (else + ;; Both arms are equally likely, and all this code is unlikely. + (if + (local.get $x) + (then + (unreachable) + ) + (else + (unreachable) + ) + ) + ) + ) + ) ) From f2653013d3e0bb7f236d44bff57b7a7c064fb48f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 08:41:00 -0700 Subject: [PATCH 26/61] BAD --- src/passes/BranchHintAnalysis.cpp | 41 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 2702b88a71b..80b295c037a 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -47,6 +47,14 @@ struct Info { // The chance of the block being reached. We assume any can be reached, unless // we see a good hint otherwise. Chance chance = MaxChance; + + void dump(Function* func) { + std::cout << " info\n"; + if (!actions.empty()) { + std::cout << " with last " << **actions.back() << '\n'; + } + std::cout << " with chance " << int(chance) << '\n'; + } }; struct BranchHintAnalysis @@ -118,11 +126,9 @@ struct BranchHintAnalysis // Now that the walk is complete and we have a CFG, find things to optimize. // First, compute the chance of each basic block from its contents. for (Index i = 0; i < basicBlocks.size(); ++i) { -std::cout << "block\n"; auto& block = basicBlocks[i]; for (auto** currp : block->contents.actions) { -std::cout << " " << **currp << "\n"; - // The chance of a basic block is the lowest thing we can find: if + // The naive chance of a basic block is the lowest thing we can find: if // we see nop, call, unreachable, then the nop tells us nothing, the // call may suggests a low chance if it is cold, but the // unreachable suggests a very low chance, which we trust. @@ -130,9 +136,11 @@ std::cout << " " << **currp << "\n"; block->contents.chance = std::min(block->contents.chance, *chance); } } -std::cout << " => " << int(block->contents.chance) << "\n"; } + // Debug + dumpCFG("pre"); + // We consider the chance of a block to be no higher than the things it // targets, that is, chance(block) := max(chance(target) for target). Flow // chances to sources of blocks to achieve that, starting from the indexes @@ -143,22 +151,25 @@ std::cout << " => " << int(block->contents.chance) << "\n"; } while (!work.empty()) { auto* block = work.pop(); - // Apply this block to its predecessors, potentially raising their - // chances. - for (auto* in : block->in) { - if (block->contents.chance > in->contents.chance) { - in->contents.chance = block->contents.chance; +std::cout << "work on " << debugIds[block] << '\n'; + + // Compute this block from its successors. The naive chance we already + // computed may decrease if all successors have lower probability. + auto maxOut = MinChance; + for (auto* out : block->out) { + maxOut = std::max(maxOut, out->contents.chance); + } + + auto& chance = block->contents.chance; + if (maxOut < chance) { + chance = maxOut; + for (auto* in : block->in) { work.push(in); } } } - // Debug - for (Index i = 0; i < basicBlocks.size(); ++i) { -std::cout << "2block\n"; - auto& block = basicBlocks[i]; -std::cout << " => " << int(block->contents.chance) << "\n"; - } + dumpCFG("analzyed"); // Apply the final chances: when a branch between two options has a higher // higher chance to go one way then the other, mark it as likely or unlikely From 0bd28743bb255baf3435923d2cfed5c0f55fd5d6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 08:57:53 -0700 Subject: [PATCH 27/61] work --- src/passes/BranchHintAnalysis.cpp | 14 +++++++++--- test/lit/passes/branch-hint-analysis.wast | 26 ++++++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 80b295c037a..a84602a50f4 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -51,9 +51,9 @@ struct Info { void dump(Function* func) { std::cout << " info\n"; if (!actions.empty()) { - std::cout << " with last " << **actions.back() << '\n'; + std::cout << " with last: " << getExpressionName(*actions.back()) << '\n'; } - std::cout << " with chance " << int(chance) << '\n'; + std::cout << " with chance: " << int(chance) << '\n'; } }; @@ -147,12 +147,19 @@ struct BranchHintAnalysis // of all blocks. UniqueDeferredQueue work; for (auto& block : basicBlocks) { - work.push(block.get()); + // Blocks with no successors have nothing new to compute in the loop + // below. + if (!block->out.empty()) { + work.push(block.get()); + } } while (!work.empty()) { auto* block = work.pop(); std::cout << "work on " << debugIds[block] << '\n'; + // We should not get here if there is no work. + assert(!block->out.empty()); + // Compute this block from its successors. The naive chance we already // computed may decrease if all successors have lower probability. auto maxOut = MinChance; @@ -161,6 +168,7 @@ std::cout << "work on " << debugIds[block] << '\n'; } auto& chance = block->contents.chance; +std::cout << " old " << int(chance) << ", maxOut " << int(maxOut) << '\n'; if (maxOut < chance) { chance = maxOut; for (auto* in : block->in) { diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index a00f99fbb72..afa279805b6 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -188,6 +188,27 @@ (unreachable) ) + ;; CHECK: (func $if-one-arm-unreachable-always (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $if-one-arm-unreachable-always (param $x i32) + ;; The unreachable after means the if arm is unlikely, like the code after + ;; the if, so we do not hint. + (if + (local.get $x) + (then + (nop) ;; used to be a return here + ) + ) + (unreachable) + ) + ;; CHECK: (func $if-throw (type $0) (param $x i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if @@ -328,6 +349,7 @@ ) ;; CHECK: (func $if-nesting-2 (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then @@ -354,7 +376,8 @@ (return) ) (else - ;; Both arms are equally likely, and all this code is unlikely. + ;; Both arms are equally likely, so we do not hint here, but all this + ;; code is unlikely. (if (local.get $x) (then @@ -369,3 +392,4 @@ ) ) +;; throw after unreachable rtc. From ab112dc8818bfbe89c1b9edb215df5ca6ca7680a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 09:51:29 -0700 Subject: [PATCH 28/61] work --- test/lit/passes/branch-hint-analysis.wast | 44 +++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index afa279805b6..09b2973fa85 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -390,6 +390,46 @@ ) ) ) -) -;; throw after unreachable rtc. + ;; CHECK: (func $loop (type $4) (param $x i32) (param $y i32) + ;; CHECK-NEXT: (loop $loop + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br $loop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $loop (param $x i32) (param $y i32) + ;; We should not error when computing cycles. + (loop $loop + (drop (i32.const 10)) + ;; No hint for the first if, but the second is unlikely. + (if + (local.get $x) + (then + (return) + ) + ) + (if + (local.get $y) + (then + (unreachable) + ) + ) + (br $loop) + ) + ) +) From 33b91180ecfe94946a15420f749cf2c076f692db Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 09:52:08 -0700 Subject: [PATCH 29/61] work --- test/lit/passes/branch-hint-analysis.wast | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 09b2973fa85..de299f7c4e1 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -402,6 +402,9 @@ ;; CHECK-NEXT: (return) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $y) @@ -409,6 +412,9 @@ ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (br $loop) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -423,12 +429,14 @@ (return) ) ) + (drop (i32.const 20)) (if (local.get $y) (then (unreachable) ) ) + (drop (i32.const 30)) (br $loop) ) ) From fbf8cb90479e68dae392178d63ab9366dce7d5e8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 10:11:34 -0700 Subject: [PATCH 30/61] BAD --- src/passes/BranchHintAnalysis.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index a84602a50f4..4bc5319baeb 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -49,11 +49,11 @@ struct Info { Chance chance = MaxChance; void dump(Function* func) { - std::cout << " info\n"; + //std::cerr << " info\n"; if (!actions.empty()) { - std::cout << " with last: " << getExpressionName(*actions.back()) << '\n'; + //std::cerr << " with last: " << getExpressionName(*actions.back()) << '\n'; } - std::cout << " with chance: " << int(chance) << '\n'; + //std::cerr << " with chance: " << int(chance) << '\n'; } }; @@ -138,8 +138,7 @@ struct BranchHintAnalysis } } - // Debug - dumpCFG("pre"); + //dumpCFG("pre"); // We consider the chance of a block to be no higher than the things it // targets, that is, chance(block) := max(chance(target) for target). Flow @@ -155,7 +154,7 @@ struct BranchHintAnalysis } while (!work.empty()) { auto* block = work.pop(); -std::cout << "work on " << debugIds[block] << '\n'; +//std::cerr << "work on " << debugIds[block] << '\n'; // We should not get here if there is no work. assert(!block->out.empty()); @@ -168,7 +167,7 @@ std::cout << "work on " << debugIds[block] << '\n'; } auto& chance = block->contents.chance; -std::cout << " old " << int(chance) << ", maxOut " << int(maxOut) << '\n'; +//std::cerr << " old " << int(chance) << ", maxOut " << int(maxOut) << '\n'; if (maxOut < chance) { chance = maxOut; for (auto* in : block->in) { @@ -177,24 +176,24 @@ std::cout << " old " << int(chance) << ", maxOut " << int(maxOut) << '\n'; } } - dumpCFG("analzyed"); + //dumpCFG("analzyed"); // Apply the final chances: when a branch between two options has a higher // higher chance to go one way then the other, mark it as likely or unlikely // accordingly. TODO: should we not mark when the difference is small? for (auto& block : basicBlocks) { -std::cout << "lastloop block\n"; +//std::cerr << "lastloop block\n"; if (block->contents.actions.empty() || block->out.size() != 2) { continue; } auto* last = *block->contents.actions.back(); -std::cout << " last " << *last << "\n"; +//std::cerr << " last " << *last << "\n"; if (!isBranching(last)) { continue; } -std::cout << " chances1\n"; +//std::cerr << " chances1\n"; // Compare the probabilities of the two targets and see if we can infer // likelihood. if (auto likely = getLikelihood(last, From 6c4d1e88a43e71758016f59dd90397c2103b6014 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 13:15:50 -0700 Subject: [PATCH 31/61] fix --- src/wasm/wasm-ir-builder.cpp | 21 ++++++++++++++++++--- test/lit/branch-hinting.wast | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 5052310f530..49ecc6c38c3 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -151,9 +151,20 @@ void IRBuilder::push(Expression* expr) { applyDebugLoc(expr); if (binaryPos && func && lastBinaryPos != *binaryPos) { - func->expressionLocations[expr] = - BinaryLocations::Span{BinaryLocation(lastBinaryPos - codeSectionOffset), - BinaryLocation(*binaryPos - codeSectionOffset)}; + auto start = BinaryLocation(lastBinaryPos - codeSectionOffset); + auto end = BinaryLocation(*binaryPos - codeSectionOffset); + // Some expressions already have their start noted, and we are just seeing + // their last segment (like an Else). + // TODO: does Try etc. need this too? + if (expr->is()) { + auto iter = func->expressionLocations.find(expr); + assert(iter != func->expressionLocations.end()); + iter->second.end = end; + // The true start from before is before the start of the current segment. + assert(iter->second.start < start); + } else { + func->expressionLocations[expr] = BinaryLocations::Span{start, end}; + } lastBinaryPos = *binaryPos; } @@ -933,6 +944,10 @@ Result<> IRBuilder::visitElse() { if (binaryPos && func) { func->delimiterLocations[iff][BinaryLocations::Else] = lastBinaryPos - codeSectionOffset; + + // Note the start of the if (which will be lost as the If is closed and the + // Else begins, but the if spans them both). + func->expressionLocations[iff].start = scope.startPos - codeSectionOffset; } return pushScope(ScopeCtx::makeElse(std::move(scope))); diff --git a/test/lit/branch-hinting.wast b/test/lit/branch-hinting.wast index 07277f0bc67..8ca0442c45d 100644 --- a/test/lit/branch-hinting.wast +++ b/test/lit/branch-hinting.wast @@ -259,6 +259,38 @@ ) ) + (func $branch-hints-if-else (param $x i32) + (@metadata.code.branch_hint "\01") + (if + (local.get $x) + (then + (drop + (i32.const 1) + ) + ) + (else + (drop + (i32.const 0) + ) + ) + ) + ) + + (func $branch-hints-if-else-result (param $x i32) + (drop + (@metadata.code.branch_hint "\01") + (if (result i32) + (local.get $x) + (then + (i32.const 1) + ) + (else + (i32.const 0) + ) + ) + ) + ) + ;; CHECK: (func $branch-hints-br_on (type $1) (param $x anyref) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (drop From 11f2c9a33b6c7d9f69c1dcaac555d3a9cdee4c19 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 13:17:56 -0700 Subject: [PATCH 32/61] finish --- src/wasm/wasm-ir-builder.cpp | 8 ++-- test/lit/branch-hinting.wast | 90 ++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 49ecc6c38c3..c9749f0610f 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -155,14 +155,14 @@ void IRBuilder::push(Expression* expr) { auto end = BinaryLocation(*binaryPos - codeSectionOffset); // Some expressions already have their start noted, and we are just seeing // their last segment (like an Else). - // TODO: does Try etc. need this too? - if (expr->is()) { - auto iter = func->expressionLocations.find(expr); - assert(iter != func->expressionLocations.end()); + auto iter = func->expressionLocations.find(expr); + if (iter != func->expressionLocations.end()) { + // Just update the end. iter->second.end = end; // The true start from before is before the start of the current segment. assert(iter->second.start < start); } else { + // Add a whole entry. func->expressionLocations[expr] = BinaryLocations::Span{start, end}; } lastBinaryPos = *binaryPos; diff --git a/test/lit/branch-hinting.wast b/test/lit/branch-hinting.wast index 8ca0442c45d..ba8f3716c3f 100644 --- a/test/lit/branch-hinting.wast +++ b/test/lit/branch-hinting.wast @@ -259,6 +259,54 @@ ) ) + ;; CHECK: (func $branch-hints-if-else (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; RTRIP: (func $branch-hints-if-else (type $0) (param $x i32) + ;; RTRIP-NEXT: (@metadata.code.branch_hint "\01") + ;; RTRIP-NEXT: (if + ;; RTRIP-NEXT: (local.get $x) + ;; RTRIP-NEXT: (then + ;; RTRIP-NEXT: (drop + ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: (else + ;; RTRIP-NEXT: (drop + ;; RTRIP-NEXT: (i32.const 0) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; SRCMP: (func $branch-hints-if-else (type $0) (param $x i32) + ;; SRCMP-NEXT: (@metadata.code.branch_hint "\01") + ;; SRCMP-NEXT: (if + ;; SRCMP-NEXT: (local.get $x) + ;; SRCMP-NEXT: (then + ;; SRCMP-NEXT: (drop + ;; SRCMP-NEXT: (i32.const 1) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: (else + ;; SRCMP-NEXT: (drop + ;; SRCMP-NEXT: (i32.const 0) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) (func $branch-hints-if-else (param $x i32) (@metadata.code.branch_hint "\01") (if @@ -276,6 +324,48 @@ ) ) + ;; CHECK: (func $branch-hints-if-else-result (type $0) (param $x i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if (result i32) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; RTRIP: (func $branch-hints-if-else-result (type $0) (param $x i32) + ;; RTRIP-NEXT: (drop + ;; RTRIP-NEXT: (@metadata.code.branch_hint "\01") + ;; RTRIP-NEXT: (if (result i32) + ;; RTRIP-NEXT: (local.get $x) + ;; RTRIP-NEXT: (then + ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: (else + ;; RTRIP-NEXT: (i32.const 0) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) + ;; SRCMP: (func $branch-hints-if-else-result (type $0) (param $x i32) + ;; SRCMP-NEXT: (drop + ;; SRCMP-NEXT: (@metadata.code.branch_hint "\01") + ;; SRCMP-NEXT: (if (result i32) + ;; SRCMP-NEXT: (local.get $x) + ;; SRCMP-NEXT: (then + ;; SRCMP-NEXT: (i32.const 1) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: (else + ;; SRCMP-NEXT: (i32.const 0) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) + ;; SRCMP-NEXT: ) (func $branch-hints-if-else-result (param $x i32) (drop (@metadata.code.branch_hint "\01") From 00db3edfe218bf240f601a098a2ec1b5d1cffa50 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 13:42:17 -0700 Subject: [PATCH 33/61] work --- test/lit/passes/branch-hint-analysis.wast | 55 +++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index de299f7c4e1..c1a571a55e8 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -440,4 +440,59 @@ (br $loop) ) ) + + ;; CHECK: (func $calls (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $calls + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $calls (param $x i32) + ;; Calls may branch out, but only when throwing, which means the code after + ;; the if is unlikely, so the if is likely. + (if + (local.get $x) + (then + (return) + ) + ) + (call $calls + (local.get $x) + ) + (unreachable) + ) + + ;; CHECK: (func $calls-throw (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $calls + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $calls-throw (param $x i32) + ;; Both sides throw or may throw (at best), so we do not hint. XXX + (if + (local.get $x) + (then + (throw $e) + ) + ) + (call $calls + (local.get $x) + ) + (unreachable) + ) ) From 3cbc86c23a93ef75d6dac771e5e00a78362a40f0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 14:37:55 -0700 Subject: [PATCH 34/61] feedback --- src/wasm/wasm-ir-builder.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index c9749f0610f..1fb39b14ec6 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -151,19 +151,17 @@ void IRBuilder::push(Expression* expr) { applyDebugLoc(expr); if (binaryPos && func && lastBinaryPos != *binaryPos) { - auto start = BinaryLocation(lastBinaryPos - codeSectionOffset); - auto end = BinaryLocation(*binaryPos - codeSectionOffset); + auto span = + BinaryLocations::Span{BinaryLocation(lastBinaryPos - codeSectionOffset), + BinaryLocation(*binaryPos - codeSectionOffset)}; // Some expressions already have their start noted, and we are just seeing // their last segment (like an Else). - auto iter = func->expressionLocations.find(expr); - if (iter != func->expressionLocations.end()) { + auto [iter, inserted] = func->expressionLocations.insert({expr, span}); + if (!inserted) { // Just update the end. - iter->second.end = end; + iter->second.end = span.end; // The true start from before is before the start of the current segment. - assert(iter->second.start < start); - } else { - // Add a whole entry. - func->expressionLocations[expr] = BinaryLocations::Span{start, end}; + assert(iter->second.start < span.start); } lastBinaryPos = *binaryPos; } @@ -995,6 +993,8 @@ Result<> IRBuilder::visitCatch(Name tag) { if (binaryPos && func) { auto& delimiterLocs = func->delimiterLocations[tryy]; delimiterLocs[delimiterLocs.size()] = lastBinaryPos - codeSectionOffset; + // TODO: As in visitElse, we likely need to stash the Try start. Here we + // also need to account for multiple catches. } CHECK_ERR(pushScope(ScopeCtx::makeCatch(std::move(scope), tryy))); From cd19403ac2ba6c255bdc8da92a9f980968250d11 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 15:36:50 -0700 Subject: [PATCH 35/61] CFG: add throwing links from calls to out? --- src/cfg/cfg-traversal.h | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h index 46ecc4b2a30..6bafa0c00ca 100644 --- a/src/cfg/cfg-traversal.h +++ b/src/cfg/cfg-traversal.h @@ -161,26 +161,21 @@ struct CFGWalker : public PostWalker { // exit blocks to flow to. bool hasSyntheticExit = false; + // Given a basic block, add a link to the exit block, indicating this block + // can reach the outside (unwind). + void linkToExit(BasicBlock* block) { + // TODO simplify exit/hasSynth + if (!hasSyntheticExit) { + exit = makeBasicBlock(); + hasSyntheticExit = true; + } + link(block, exit); + } + static void doEndReturn(SubType* self, Expression** currp) { auto* last = self->currBasicBlock; self->startUnreachableBlock(); - if (!self->exit) { - // This is our first exit block and may be our only exit block, so just - // set it. - self->exit = last; - } else if (!self->hasSyntheticExit) { - // We now have multiple exit blocks, so we need to create a synthetic one. - // It will be added to the list of basic blocks at the end of the - // function. - auto* lastExit = self->exit; - self->exit = self->makeBasicBlock(); - self->link(lastExit, self->exit); - self->link(last, self->exit); - self->hasSyntheticExit = true; - } else { - // We already have a synthetic exit block. Just link it up. - self->link(last, self->exit); - } + self->linkToExit(last); } static void doStartIfTrue(SubType* self, Expression** currp) { @@ -354,6 +349,11 @@ struct CFGWalker : public PostWalker { // the outside) can only happen at the end of basic blocks. auto* last = self->currBasicBlock; self->link(last, self->startBasicBlock()); + + if (!self->ignoreBranchesOutsideOfFunc) { + // Add a branch to the outside of the func. + self->linkToExit(last); + } } } From 5e6b7fab2ea61b5f21a6c018504a33ddb33a615d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 15:40:07 -0700 Subject: [PATCH 36/61] Revert "CFG: add throwing links from calls to out?" This reverts commit cd19403ac2ba6c255bdc8da92a9f980968250d11. --- src/cfg/cfg-traversal.h | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h index 6bafa0c00ca..46ecc4b2a30 100644 --- a/src/cfg/cfg-traversal.h +++ b/src/cfg/cfg-traversal.h @@ -161,21 +161,26 @@ struct CFGWalker : public PostWalker { // exit blocks to flow to. bool hasSyntheticExit = false; - // Given a basic block, add a link to the exit block, indicating this block - // can reach the outside (unwind). - void linkToExit(BasicBlock* block) { - // TODO simplify exit/hasSynth - if (!hasSyntheticExit) { - exit = makeBasicBlock(); - hasSyntheticExit = true; - } - link(block, exit); - } - static void doEndReturn(SubType* self, Expression** currp) { auto* last = self->currBasicBlock; self->startUnreachableBlock(); - self->linkToExit(last); + if (!self->exit) { + // This is our first exit block and may be our only exit block, so just + // set it. + self->exit = last; + } else if (!self->hasSyntheticExit) { + // We now have multiple exit blocks, so we need to create a synthetic one. + // It will be added to the list of basic blocks at the end of the + // function. + auto* lastExit = self->exit; + self->exit = self->makeBasicBlock(); + self->link(lastExit, self->exit); + self->link(last, self->exit); + self->hasSyntheticExit = true; + } else { + // We already have a synthetic exit block. Just link it up. + self->link(last, self->exit); + } } static void doStartIfTrue(SubType* self, Expression** currp) { @@ -349,11 +354,6 @@ struct CFGWalker : public PostWalker { // the outside) can only happen at the end of basic blocks. auto* last = self->currBasicBlock; self->link(last, self->startBasicBlock()); - - if (!self->ignoreBranchesOutsideOfFunc) { - // Add a branch to the outside of the func. - self->linkToExit(last); - } } } From 2a5fa350092c2e5b059396cb90c983e6cc70c33e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 16:26:37 -0700 Subject: [PATCH 37/61] simp --- src/passes/BranchHintAnalysis.cpp | 34 ++++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 4bc5319baeb..a54297dcb31 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -44,8 +44,8 @@ struct Info { // provide hints as to branching. std::vector actions; // TODO * not **? - // The chance of the block being reached. We assume any can be reached, unless - // we see a good hint otherwise. + // The chance of the block being reached. We assume it is likely to be reached + // until we see a signal otherwise. Chance chance = MaxChance; void dump(Function* func) { @@ -102,11 +102,21 @@ struct BranchHintAnalysis } void visitExpression(Expression* curr) { - // Add all (reachable, so |currBasicBlock| exists) things that either branch - // or suggest chances of branching. - if (currBasicBlock && (isBranching(curr) || getChance(curr))) { + // Ignore unreachable code. + if (!currBasicBlock) { + return; + } + + // Add all things that branch. + if (isBranching(curr)) { currBasicBlock->contents.actions.push_back(getCurrentPointer()); } + + // Apply all signals: if something tells us the block is unlikely, mark it + // so. + if (auto chance = getChance(curr)) { + currBasicBlock->contents.chance = std::min(currBasicBlock->contents.chance, *chance); + } } // Override cfg-analysis's handling of If start. Ifs are control flow @@ -124,20 +134,6 @@ struct BranchHintAnalysis void visitFunction(Function* curr) { // Now that the walk is complete and we have a CFG, find things to optimize. - // First, compute the chance of each basic block from its contents. - for (Index i = 0; i < basicBlocks.size(); ++i) { - auto& block = basicBlocks[i]; - for (auto** currp : block->contents.actions) { - // The naive chance of a basic block is the lowest thing we can find: if - // we see nop, call, unreachable, then the nop tells us nothing, the - // call may suggests a low chance if it is cold, but the - // unreachable suggests a very low chance, which we trust. - if (auto chance = getChance(*currp)) { - block->contents.chance = std::min(block->contents.chance, *chance); - } - } - } - //dumpCFG("pre"); // We consider the chance of a block to be no higher than the things it From c075d0fdcc01d042233d3a75a83b37fbd7b9e7bb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 16:30:40 -0700 Subject: [PATCH 38/61] work --- test/lit/passes/branch-hint-analysis.wast | 34 +++++++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index c1a571a55e8..4d28023fe17 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -441,7 +441,7 @@ ) ) - ;; CHECK: (func $calls (type $0) (param $x i32) + ;; CHECK: (func $calls-unreachable (type $0) (param $x i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) @@ -454,7 +454,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) - (func $calls (param $x i32) + (func $calls-unreachable (param $x i32) ;; Calls may branch out, but only when throwing, which means the code after ;; the if is unlikely, so the if is likely. (if @@ -469,8 +469,32 @@ (unreachable) ) + ;; CHECK: (func $calls (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $calls + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $calls (param $x i32) + ;; As above but no unreachable at the end. + (if + (local.get $x) + (then + (return) + ) + ) + (call $calls + (local.get $x) + ) + ) + ;; CHECK: (func $calls-throw (type $0) (param $x i32) - ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then @@ -480,10 +504,9 @@ ;; CHECK-NEXT: (call $calls ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) (func $calls-throw (param $x i32) - ;; Both sides throw or may throw (at best), so we do not hint. XXX + ;; The throw is less likely than a call, which only might throw. (if (local.get $x) (then @@ -493,6 +516,5 @@ (call $calls (local.get $x) ) - (unreachable) ) ) From 7af9c99a7041e1e5983f2c46e5a7544f46e00678 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 16:35:20 -0700 Subject: [PATCH 39/61] work --- test/lit/passes/branch-hint-analysis.wast | 102 +++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 4d28023fe17..9cfa1b7e6e0 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -481,7 +481,8 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $calls (param $x i32) - ;; As above but no unreachable at the end. + ;; As above but no unreachable at the end. The call might throw, but it also + ;; might exit normally, so we have no hint to give here. (if (local.get $x) (then @@ -517,4 +518,103 @@ (local.get $x) ) ) + + ;; CHECK: (func $lots-in-middle (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 40) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 50) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $lots-in-middle (param $x i32) + ;; The unreachable after all the code below means the condition is likely. + (if + (local.get $x) + (then + (return) + ) + ) + ;; ...lots of code, but no exits from function... + (nop) + (drop + (i32.const 10) + ) + (if + (local.get $x) + (then + (drop + (i32.const 20) + ) + ) + (else + (local.set $x + (i32.const 30) + ) + ) + ) + (block $block + (br_if $block + (local.get $x) + ) + (drop + (i32.const 40) + ) + ) + (if + (local.get $x) + (then + (if + (local.get $x) + (then + (drop + (i32.const 50) + ) + ) + ) + ) + ) + ;; Finally, an unreachable. + (unreachable) + ) ) From 7ff5c3dfcbf92b935a2517e56b7f9045b6ec7681 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 16:36:48 -0700 Subject: [PATCH 40/61] work --- test/lit/passes/branch-hint-analysis.wast | 98 +++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index 9cfa1b7e6e0..b09f9b76761 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -617,4 +617,102 @@ ;; Finally, an unreachable. (unreachable) ) + + ;; CHECK: (func $lots-in-middle-return (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 50) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $lots-in-middle-return (param $x i32) + ;; As above, but now a return in the middle of the code below. No hint here, + ;; but we can hint on the br_if near that return. + (if + (local.get $x) + (then + (return) + ) + ) + (nop) + (drop + (i32.const 10) + ) + (if + (local.get $x) + (then + (drop + (i32.const 20) + ) + ) + (else + (local.set $x + (i32.const 30) + ) + ) + ) + (block $block + (br_if $block + (local.get $x) + ) + (drop + (return) + ) + ) + (if + (local.get $x) + (then + (if + (local.get $x) + (then + (drop + (i32.const 50) + ) + ) + ) + ) + ) + (unreachable) + ) ) From b6be9694977b1896aec107d8a40cd34a8160e5ef Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 May 2025 16:37:37 -0700 Subject: [PATCH 41/61] work --- test/lit/passes/branch-hint-analysis.wast | 94 +++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis.wast b/test/lit/passes/branch-hint-analysis.wast index b09f9b76761..62b4ba400a4 100644 --- a/test/lit/passes/branch-hint-analysis.wast +++ b/test/lit/passes/branch-hint-analysis.wast @@ -715,4 +715,98 @@ ) (unreachable) ) + + ;; CHECK: (func $lots-in-middle-nothing (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (br_if $block + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 40) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 50) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $lots-in-middle-nothing (param $x i32) + ;; As above, but no final unreachable, so no hints. + (if + (local.get $x) + (then + (return) + ) + ) + (nop) + (drop + (i32.const 10) + ) + (if + (local.get $x) + (then + (drop + (i32.const 20) + ) + ) + (else + (local.set $x + (i32.const 30) + ) + ) + ) + (block $block + (br_if $block + (local.get $x) + ) + (drop + (i32.const 40) + ) + ) + (if + (local.get $x) + (then + (if + (local.get $x) + (then + (drop + (i32.const 50) + ) + ) + ) + ) + ) + ) ) From 77cb8cd225c63e01b8713d5c24636bac382c3023 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 08:43:23 -0700 Subject: [PATCH 42/61] cfg.prep --- src/passes/BranchHintAnalysis.cpp | 45 +++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index a54297dcb31..af922bb6ea4 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -23,6 +23,7 @@ // #include "cfg/cfg-traversal.h" +#include "ir/module-utils.h" #include "pass.h" #include "support/unique_deferring_queue.h" #include "wasm-builder.h" @@ -39,6 +40,7 @@ static constexpr Chance MinChance = 0; static constexpr Chance TinyChance = 1; static constexpr Chance MaxChance = 100; +// Info we store in a basic block. struct Info { // In each basic block we will store instructions that either branch, or that // provide hints as to branching. @@ -57,20 +59,12 @@ struct Info { } }; -struct BranchHintAnalysis - : public WalkerPass< - CFGWalker, Info>> { - bool isFunctionParallel() override { return true; } +// Analyze Branch Hints in a function, using a CFG. +struct BranchHintCFGAnalysis + : public CFGWalker, Info> { - // Locals are not modified here. - bool requiresNonNullableLocalFixups() override { return false; } - - std::unique_ptr create() override { - return std::make_unique(); - } - - using Super = WalkerPass< - CFGWalker, Info>>; + using Super = + CFGWalker, Info>; // We only look at things that branch twice, which is all branching // instructions but without br (without condition, which is an unconditional @@ -123,7 +117,7 @@ struct BranchHintAnalysis // structures, so they do not appear in basic blocks (an If spans several), // but we do need to know where the If begins, specifically, where the // condition can branch - static void doStartIfTrue(BranchHintAnalysis* self, Expression** currp) { + static void doStartIfTrue(BranchHintCFGAnalysis* self, Expression** currp) { // Right before the Super creates a basic block for the ifTrue, note the // basic block the condition is in. if (self->currBasicBlock) { @@ -223,6 +217,29 @@ struct BranchHintAnalysis } }; +// A BranchHintCFGAnalysis with the additional info that +// CallGraphPropertyAnalysis::FunctionInfo needs, which we will store in the map +// generated by CallGraphPropertyAnalysis. +struct StoredBranchHintCFGAnalysis : public BranchHintCFGAnalysis, + ModuleUtils::CallGraphPropertyAnalysis::FunctionInfo { +}; + +struct BranchHintAnalysis : public Pass { + // Locals are not modified here. + bool requiresNonNullableLocalFixups() override { return false; } + + void run(Module* module) override { + ModuleUtils::CallGraphPropertyAnalysis analyzer( + *module, [&](Function* func, StoredBranchHintCFGAnalysis& analysis) { + if (func->imported()) { + return; + } + + analysis.walkFunctionInModule(func, module); + }); + } +}; + } // anonymous namespace Pass* createBranchHintAnalysisPass() { return new BranchHintAnalysis(); } From af7ee3986c5b3a2b521f099e46d74ad7da6aa8a9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 08:55:27 -0700 Subject: [PATCH 43/61] work --- src/passes/BranchHintAnalysis.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index af922bb6ea4..33c4c455d9b 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -66,6 +66,8 @@ struct BranchHintCFGAnalysis using Super = CFGWalker, Info>; + PassOptions passOptions; + // We only look at things that branch twice, which is all branching // instructions but without br (without condition, which is an unconditional // branch we don't need to hint about) and not switch (which Branch Hints do @@ -77,6 +79,10 @@ struct BranchHintCFGAnalysis return curr->is() || curr->is(); } + bool isCall(Expression* curr) { + return ShallowEffectAnalyzer(passOptions, *getModule(), curr).calls; + } + // Returns the chance that an instruction is reached, if something about // it suggests it is likely or not. std::optional getChance(Expression* curr) { @@ -101,8 +107,8 @@ struct BranchHintCFGAnalysis return; } - // Add all things that branch. - if (isBranching(curr)) { + // Add all things that branch or call. + if (isBranching(curr) || isCall(curr)) { currBasicBlock->contents.actions.push_back(getCurrentPointer()); } @@ -229,14 +235,20 @@ struct BranchHintAnalysis : public Pass { bool requiresNonNullableLocalFixups() override { return false; } void run(Module* module) override { + // Analyze each function, computing chances inside it. ModuleUtils::CallGraphPropertyAnalysis analyzer( *module, [&](Function* func, StoredBranchHintCFGAnalysis& analysis) { if (func->imported()) { return; } + analysis.passOptions = getPassOptions(); analysis.walkFunctionInModule(func, module); }); + + // Link up the CFGs from each function to a single unified CFG, by linking a + // call in one function to the entry blocks in the called function. + // TODO } }; From b219c2d8f8073f43bb2e0ee7f26291c873f15f3a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 10:18:58 -0700 Subject: [PATCH 44/61] work --- src/passes/BranchHintAnalysis.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 33c4c455d9b..bc066db36f4 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -44,7 +44,7 @@ static constexpr Chance MaxChance = 100; struct Info { // In each basic block we will store instructions that either branch, or that // provide hints as to branching. - std::vector actions; // TODO * not **? + std::vector actions; // The chance of the block being reached. We assume it is likely to be reached // until we see a signal otherwise. @@ -53,7 +53,7 @@ struct Info { void dump(Function* func) { //std::cerr << " info\n"; if (!actions.empty()) { - //std::cerr << " with last: " << getExpressionName(*actions.back()) << '\n'; + //std::cerr << " with last: " << getExpressionName(actions.back()) << '\n'; } //std::cerr << " with chance: " << int(chance) << '\n'; } @@ -109,7 +109,7 @@ struct BranchHintCFGAnalysis // Add all things that branch or call. if (isBranching(curr) || isCall(curr)) { - currBasicBlock->contents.actions.push_back(getCurrentPointer()); + currBasicBlock->contents.actions.push_back(curr); } // Apply all signals: if something tells us the block is unlikely, mark it @@ -127,7 +127,7 @@ struct BranchHintCFGAnalysis // Right before the Super creates a basic block for the ifTrue, note the // basic block the condition is in. if (self->currBasicBlock) { - self->currBasicBlock->contents.actions.push_back(currp); + self->currBasicBlock->contents.actions.push_back(*currp); } Super::doStartIfTrue(self, currp); } @@ -183,7 +183,7 @@ struct BranchHintCFGAnalysis continue; } - auto* last = *block->contents.actions.back(); + auto* last = block->contents.actions.back(); //std::cerr << " last " << *last << "\n"; if (!isBranching(last)) { continue; @@ -248,7 +248,6 @@ struct BranchHintAnalysis : public Pass { // Link up the CFGs from each function to a single unified CFG, by linking a // call in one function to the entry blocks in the called function. - // TODO } }; From 6e6bba55b41c2de561aa082c19e79e41bbd916e6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 10:57:22 -0700 Subject: [PATCH 45/61] work --- src/passes/BranchHintAnalysis.cpp | 114 ++++++++++++++++++++---------- 1 file changed, 78 insertions(+), 36 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index bc066db36f4..105b6ec06ad 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -66,8 +66,6 @@ struct BranchHintCFGAnalysis using Super = CFGWalker, Info>; - PassOptions passOptions; - // We only look at things that branch twice, which is all branching // instructions but without br (without condition, which is an unconditional // branch we don't need to hint about) and not switch (which Branch Hints do @@ -80,7 +78,8 @@ struct BranchHintCFGAnalysis } bool isCall(Expression* curr) { - return ShallowEffectAnalyzer(passOptions, *getModule(), curr).calls; + // TODO: we could infer something for indirect calls too. + return curr->is(); } // Returns the chance that an instruction is reached, if something about @@ -133,9 +132,11 @@ struct BranchHintCFGAnalysis } void visitFunction(Function* curr) { - // Now that the walk is complete and we have a CFG, find things to optimize. - //dumpCFG("pre"); + flow(curr); + } + // Flow chances in a function, to find the chances of all blocks inside it. + void flow(Function* func) { // We consider the chance of a block to be no higher than the things it // targets, that is, chance(block) := max(chance(target) for target). Flow // chances to sources of blocks to achieve that, starting from the indexes @@ -171,34 +172,6 @@ struct BranchHintCFGAnalysis } } } - - //dumpCFG("analzyed"); - - // Apply the final chances: when a branch between two options has a higher - // higher chance to go one way then the other, mark it as likely or unlikely - // accordingly. TODO: should we not mark when the difference is small? - for (auto& block : basicBlocks) { -//std::cerr << "lastloop block\n"; - if (block->contents.actions.empty() || block->out.size() != 2) { - continue; - } - - auto* last = block->contents.actions.back(); -//std::cerr << " last " << *last << "\n"; - if (!isBranching(last)) { - continue; - } - -//std::cerr << " chances1\n"; - // Compare the probabilities of the two targets and see if we can infer - // likelihood. - if (auto likely = getLikelihood(last, - block->out[0]->contents.chance, - block->out[1]->contents.chance)) { - // We have a useful hint! - curr->codeAnnotations[last].branchLikely = likely; - } - } } // Checks if a branch from a branching instruction to two targets is likely or @@ -242,12 +215,81 @@ struct BranchHintAnalysis : public Pass { return; } - analysis.passOptions = getPassOptions(); analysis.walkFunctionInModule(func, module); }); - // Link up the CFGs from each function to a single unified CFG, by linking a - // call in one function to the entry blocks in the called function. + // Whenever a function's entry block has low chance, that means callers are + // low chance as well. Build a mapping to connect each entry function to the + // callers, so we can update them later down. + using BasicBlock = StoredBranchHintCFGAnalysis::BasicBlock; + std::unordered_map> entryToCallersMap; + for (auto& [_, analysis] : analyzer.map) { + for (auto& callerBlock : analysis.basicBlocks) { + // Calls only appear at the end of blocks. + if (callerBlock->contents.actions.empty()) { + continue; + } + auto* last = block->contents.actions.back(); + if (auto* call = last->dynCast()) { + auto* target = module->getFunction(call->target); + auto* targetEntryBlock = analyzer.map[target].entry; + entryToCallersMap[targetEntryBlock].push_back(callerBlock); + } + } + } + + // Flow back from entries to callers. We start from all entries with low + // chance and put them in a work queue. + UniqueDeferredQueue work; + for (auto& [entry, callers] : entryToCallersMap) { + if (entry->contents.chance < MaxChance) { + work.push(entry); + } + } + while (!work.empty()) { + auto* entry = work.pop(); + auto entryChance = entry->contents.chance; + // Find callers with higher chance: we can infer they have lower, now. + for (auto* caller : entryToCallersMap) { + auto& callerChance = caller->contents.chance; + if (callerChance > entryChance) { + callerChance = entryChance; + + // This adjustment to a basic block's chance may lead to more + // inferences inside that function: do a flow. TODO + } + } + } + + // Finally, apply all we've inferred. TODO: parallelize. + + // Apply the final chances: when a branch between two options has a higher + // chance to go one way then the other, mark it as likely or unlikely + // accordingly. TODO: should we not mark when the difference is small? + for (auto& [func, analysis] : analyzer.map) { + for (auto& block : analysis.basicBlocks) { + //std::cerr << "lastloop block\n"; + if (block->contents.actions.empty() || block->out.size() != 2) { + continue; + } + + auto* last = block->contents.actions.back(); + //std::cerr << " last " << *last << "\n"; + if (!isBranching(last)) { + continue; + } + + //std::cerr << " chances1\n"; + // Compare the probabilities of the two targets and see if we can infer + // likelihood. + if (auto likely = getLikelihood(last, + block->out[0]->contents.chance, + block->out[1]->contents.chance)) { + // We have a useful hint! + func->codeAnnotations[last].branchLikely = likely; + } + } + } } }; From cdc732996d0eadec9031578b67f465fbd4cb3681 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:07:52 -0700 Subject: [PATCH 46/61] work --- src/passes/BranchHintAnalysis.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 105b6ec06ad..cda4c6815e0 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -229,11 +229,11 @@ struct BranchHintAnalysis : public Pass { if (callerBlock->contents.actions.empty()) { continue; } - auto* last = block->contents.actions.back(); + auto* last = callerBlock->contents.actions.back(); if (auto* call = last->dynCast()) { auto* target = module->getFunction(call->target); auto* targetEntryBlock = analyzer.map[target].entry; - entryToCallersMap[targetEntryBlock].push_back(callerBlock); + entryToCallersMap[targetEntryBlock].push_back(callerBlock.get()); } } } @@ -250,7 +250,7 @@ struct BranchHintAnalysis : public Pass { auto* entry = work.pop(); auto entryChance = entry->contents.chance; // Find callers with higher chance: we can infer they have lower, now. - for (auto* caller : entryToCallersMap) { + for (auto* caller : entryToCallersMap[entry]) { auto& callerChance = caller->contents.chance; if (callerChance > entryChance) { callerChance = entryChance; @@ -275,14 +275,14 @@ struct BranchHintAnalysis : public Pass { auto* last = block->contents.actions.back(); //std::cerr << " last " << *last << "\n"; - if (!isBranching(last)) { + if (!analysis.isBranching(last)) { continue; } //std::cerr << " chances1\n"; // Compare the probabilities of the two targets and see if we can infer // likelihood. - if (auto likely = getLikelihood(last, + if (auto likely = analysis.getLikelihood(last, block->out[0]->contents.chance, block->out[1]->contents.chance)) { // We have a useful hint! From 674bd8b83a33c114f0d3ccbb1b5dc97075cf6f14 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:16:03 -0700 Subject: [PATCH 47/61] work --- src/passes/BranchHintAnalysis.cpp | 44 ++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index cda4c6815e0..db816379f91 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -198,9 +198,11 @@ struct BranchHintCFGAnalysis // A BranchHintCFGAnalysis with the additional info that // CallGraphPropertyAnalysis::FunctionInfo needs, which we will store in the map -// generated by CallGraphPropertyAnalysis. -struct StoredBranchHintCFGAnalysis : public BranchHintCFGAnalysis, - ModuleUtils::CallGraphPropertyAnalysis::FunctionInfo { +// generated by CallGraphPropertyAnalysis. This structure provides all the info +// for one function. +// TODO: we don't actually use CallGraphPropertyAnalysis?! +struct FunctionAnalysis : public BranchHintCFGAnalysis, + ModuleUtils::CallGraphPropertyAnalysis::FunctionInfo { }; struct BranchHintAnalysis : public Pass { @@ -209,8 +211,8 @@ struct BranchHintAnalysis : public Pass { void run(Module* module) override { // Analyze each function, computing chances inside it. - ModuleUtils::CallGraphPropertyAnalysis analyzer( - *module, [&](Function* func, StoredBranchHintCFGAnalysis& analysis) { + ModuleUtils::CallGraphPropertyAnalysis analyzer( + *module, [&](Function* func, FunctionAnalysis& analysis) { if (func->imported()) { return; } @@ -218,11 +220,18 @@ struct BranchHintAnalysis : public Pass { analysis.walkFunctionInModule(func, module); }); + using BasicBlock = FunctionAnalysis::BasicBlock; + + // A block plus the context of which function it is in. + struct BlockContext { + BasicBlock* block; + FunctionAnalysis* analysis; + }; + // Whenever a function's entry block has low chance, that means callers are // low chance as well. Build a mapping to connect each entry function to the // callers, so we can update them later down. - using BasicBlock = StoredBranchHintCFGAnalysis::BasicBlock; - std::unordered_map> entryToCallersMap; + std::unordered_map> entryToCallersMap; for (auto& [_, analysis] : analyzer.map) { for (auto& callerBlock : analysis.basicBlocks) { // Calls only appear at the end of blocks. @@ -233,7 +242,7 @@ struct BranchHintAnalysis : public Pass { if (auto* call = last->dynCast()) { auto* target = module->getFunction(call->target); auto* targetEntryBlock = analyzer.map[target].entry; - entryToCallersMap[targetEntryBlock].push_back(callerBlock.get()); + entryToCallersMap[targetEntryBlock].push_back(BlockContext{callerBlock.get(), &analysis}); } } } @@ -241,7 +250,7 @@ struct BranchHintAnalysis : public Pass { // Flow back from entries to callers. We start from all entries with low // chance and put them in a work queue. UniqueDeferredQueue work; - for (auto& [entry, callers] : entryToCallersMap) { + for (auto& [entry, _] : entryToCallersMap) { if (entry->contents.chance < MaxChance) { work.push(entry); } @@ -250,13 +259,24 @@ struct BranchHintAnalysis : public Pass { auto* entry = work.pop(); auto entryChance = entry->contents.chance; // Find callers with higher chance: we can infer they have lower, now. - for (auto* caller : entryToCallersMap[entry]) { - auto& callerChance = caller->contents.chance; + for (auto* callerContext : entryToCallersMap[entry]) { + auto& callerChance = callerContext.block->contents.chance; if (callerChance > entryChance) { callerChance = entryChance; // This adjustment to a basic block's chance may lead to more - // inferences inside that function: do a flow. TODO + // inferences inside that function: do a flow. TODO we could flow only + // from the caller blocks, and we could do these flows in parallel. + auto* callerAnalysis = callerContext.analysis; + auto* callerEntry = callerAnalysis->entry; + auto oldCallerEntryChance = callerEntry->content.chance; + callerAnalysis.flow(); + + // If our entry decreased in chance, we can propagate that to our + // callers too. + if (oldCallerEntryChance > callerEntry->content.chance) { + work.push(callerEntry); + } } } } From 2958e462a1d028ee16662ae15819c24aeced066d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:17:44 -0700 Subject: [PATCH 48/61] work --- src/passes/BranchHintAnalysis.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index db816379f91..61d58978e1b 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -231,7 +231,7 @@ struct BranchHintAnalysis : public Pass { // Whenever a function's entry block has low chance, that means callers are // low chance as well. Build a mapping to connect each entry function to the // callers, so we can update them later down. - std::unordered_map> entryToCallersMap; + std::unordered_map> entryToCallersMap; for (auto& [_, analysis] : analyzer.map) { for (auto& callerBlock : analysis.basicBlocks) { // Calls only appear at the end of blocks. @@ -242,7 +242,8 @@ struct BranchHintAnalysis : public Pass { if (auto* call = last->dynCast()) { auto* target = module->getFunction(call->target); auto* targetEntryBlock = analyzer.map[target].entry; - entryToCallersMap[targetEntryBlock].push_back(BlockContext{callerBlock.get(), &analysis}); + auto context = BlockContext{callerBlock.get(), &analysis}; + entryToCallersMap[targetEntryBlock].push_back(context); } } } @@ -259,7 +260,7 @@ struct BranchHintAnalysis : public Pass { auto* entry = work.pop(); auto entryChance = entry->contents.chance; // Find callers with higher chance: we can infer they have lower, now. - for (auto* callerContext : entryToCallersMap[entry]) { + for (auto& callerContext : entryToCallersMap[entry]) { auto& callerChance = callerContext.block->contents.chance; if (callerChance > entryChance) { callerChance = entryChance; @@ -269,12 +270,12 @@ struct BranchHintAnalysis : public Pass { // from the caller blocks, and we could do these flows in parallel. auto* callerAnalysis = callerContext.analysis; auto* callerEntry = callerAnalysis->entry; - auto oldCallerEntryChance = callerEntry->content.chance; + auto oldCallerEntryChance = callerEntry->contents.chance; callerAnalysis.flow(); // If our entry decreased in chance, we can propagate that to our // callers too. - if (oldCallerEntryChance > callerEntry->content.chance) { + if (oldCallerEntryChance > callerEntry->contents.chance) { work.push(callerEntry); } } From aca85ae51460cf9437c3b1589b6247e49cb1ac1c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:18:42 -0700 Subject: [PATCH 49/61] work --- src/passes/BranchHintAnalysis.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 61d58978e1b..fc6efa9732f 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -132,11 +132,11 @@ struct BranchHintCFGAnalysis } void visitFunction(Function* curr) { - flow(curr); + flow(); } // Flow chances in a function, to find the chances of all blocks inside it. - void flow(Function* func) { + void flow() { // We consider the chance of a block to be no higher than the things it // targets, that is, chance(block) := max(chance(target) for target). Flow // chances to sources of blocks to achieve that, starting from the indexes @@ -271,7 +271,7 @@ struct BranchHintAnalysis : public Pass { auto* callerAnalysis = callerContext.analysis; auto* callerEntry = callerAnalysis->entry; auto oldCallerEntryChance = callerEntry->contents.chance; - callerAnalysis.flow(); + callerAnalysis->flow(); // If our entry decreased in chance, we can propagate that to our // callers too. From b82598538be7e8c389e5821096091bddab61d985 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:24:16 -0700 Subject: [PATCH 50/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 test/lit/passes/branch-hint-analysis-lto.wast diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast new file mode 100644 index 00000000000..24359c002fe --- /dev/null +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -0,0 +1,64 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --branch-hint-analysis -all -S -o - \ +;; RUN: | filecheck %s + +;; Tests "lto"-like behavior of BranchHintAnalysis, that is, inferences between +;; functions. + +(module + (func $unreachable + ;; Helper for below. + (unreachable) + ) + + (func $throw + ;; Helper for below. + (throw $e) + ) + + (func $call-unreachable (param $x i32) + ;; This is unlikely as the call's target is. + (if + (local.get $x) + (then + (call $unreachable) + ) + ) + ) + + (func $call-throw (param $x i32) + ;; This is unlikely as the call's target is. + (if + (local.get $x) + (then + (call $throw) + ) + ) + ) + + (func $call-both (param $x i32) + ;; Unreachable is less likely than throw. + (if + (local.get $x) + (then + (call $throw) + ) + (else + (call $unreachable) + ) + ) + ) + + (func $call-both-flip (param $x i32) + ;; As above, but flipped. + (if + (local.get $x) + (then + (call $unreachable) + ) + (else + (call $throw) + ) + ) + ) +) From 69428e4898f868781ce675398c9bd89edea1d09d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:24:46 -0700 Subject: [PATCH 51/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 24359c002fe..616f6cfded5 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -61,4 +61,7 @@ ) ) ) + +;; TODO: flow back from call, to get the hint +;; TODO: flow back throgh chain of calls ) From f0b3cbf79d9889a4a3655cf7f72b54b69814a704 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:26:34 -0700 Subject: [PATCH 52/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 616f6cfded5..55f7659bd0b 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -6,16 +6,40 @@ ;; functions. (module + ;; CHECK: (tag $e (type $1)) + (tag $e) + + ;; CHECK: (func $unreachable (type $1) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) (func $unreachable ;; Helper for below. (unreachable) ) + ;; CHECK: (func $throw (type $1) + ;; CHECK-NEXT: (throw $e) + ;; CHECK-NEXT: ) (func $throw ;; Helper for below. (throw $e) ) + ;; CHECK: (func $nop (type $1) + ;; CHECK-NEXT: ) + (func $nop + ;; Helper for below. + ) + + ;; CHECK: (func $call-unreachable (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $call-unreachable (param $x i32) ;; This is unlikely as the call's target is. (if @@ -26,6 +50,15 @@ ) ) + ;; CHECK: (func $call-throw (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $call-throw (param $x i32) ;; This is unlikely as the call's target is. (if @@ -36,6 +69,36 @@ ) ) + ;; CHECK: (func $call-nop (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-nop (param $x i32) + ;; Nothing to infer here. + (if + (local.get $x) + (then + (call $nop) + ) + ) + ) + + ;; CHECK: (func $call-both (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $call-both (param $x i32) ;; Unreachable is less likely than throw. (if @@ -49,6 +112,18 @@ ) ) + ;; CHECK: (func $call-both-flip (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $call-both-flip (param $x i32) ;; As above, but flipped. (if From 4ea7d48625a2315e517cd78145d01dae6c9f969d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:28:24 -0700 Subject: [PATCH 53/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 55f7659bd0b..4524686eaf6 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -137,6 +137,32 @@ ) ) + ;; CHECK: (func $call-both-mix (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-both-mix (param $x i32) + ;; As above, but mix calls in one arm. Nothing to infer. + (if + (local.get $x) + (then + (call $unreachable) + ) + (else + (call $unreachable) + (call $throw) + ) + ) + ) + ;; TODO: flow back from call, to get the hint ;; TODO: flow back throgh chain of calls ) From 43775a9f567b6eca6c5911a145a456eff035dea4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:28:52 -0700 Subject: [PATCH 54/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 4524686eaf6..85651b1d110 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -163,6 +163,32 @@ ) ) + ;; CHECK: (func $call-both-mix-flip (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-both-mix-flip (param $x i32) + ;; As above, but flipped in that arm. Still nothing to infer. + (if + (local.get $x) + (then + (call $unreachable) + ) + (else + (call $throw) + (call $unreachable) + ) + ) + ) + ;; TODO: flow back from call, to get the hint ;; TODO: flow back throgh chain of calls ) From 4f26b794811069c3a23b8e5c192a44b998cac8f1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:32:16 -0700 Subject: [PATCH 55/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 85651b1d110..eb3beff7624 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -189,6 +189,54 @@ ) ) -;; TODO: flow back from call, to get the hint + ;; CHECK: (func $flow-back (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $flow-back (param $x i32) + ;; We must flow back what we infer about the call, to previous blocks, in + ;; order to infer a hint on this if. + (if + (local.get $x) + (then + ;; Add some basic blocks and code in the middle. + (drop + (i32.const 10) + ) + (if + (local.get $x) + (then + (drop + (i32.const 20) + ) + ) + ) + (drop + (i32.const 30) + ) + (call $unreachable) + ) + ) + ) + ;; TODO: flow back throgh chain of calls ) From 9d7ac5f6c1af42c7e606c3ac0be3f04506b9921e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:37:17 -0700 Subject: [PATCH 56/61] work --- test/lit/passes/branch-hint-analysis-lto.wast | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index eb3beff7624..50f2fc95f8c 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -238,5 +238,63 @@ ) ) -;; TODO: flow back throgh chain of calls + ;; CHECK: (func $chain-0 (type $1) + ;; CHECK-NEXT: (call $throw) + ;; CHECK-NEXT: ) + (func $chain-0 + ;; The end of a chain of functions that calls something unlikely. Helper for + ;; below. + (call $throw) + ) + + ;; CHECK: (func $chain-1 (type $1) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $chain-0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (call $unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $chain-1 + ;; This callchain segment needs some flow internally. We can also infer this + ;; if is likely (since throw is more likely than unreachable). + (if + (i32.const 10) + (then + (call $chain-0) + ) + (else + (call $unreachable) + ) + ) + ) + + ;; CHECK: (func $chain-2 (type $1) + ;; CHECK-NEXT: (call $chain-1) + ;; CHECK-NEXT: ) + (func $chain-2 + (call $chain-1) + ) + + ;; CHECK: (func $call-chain (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $chain-2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-chain (param $x i32) + ;; This is unlikely as the callchain ends that way. XXX + (if + (local.get $x) + (then + (call $chain-2) + ) + ) + ) ) From 0edd8a62732e19614c61bc474dd3282034c1aa4a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 11:55:31 -0700 Subject: [PATCH 57/61] work --- src/passes/BranchHintAnalysis.cpp | 16 ++++++++-------- test/lit/passes/branch-hint-analysis-lto.wast | 3 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index fc6efa9732f..03b833d2f7b 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -51,11 +51,11 @@ struct Info { Chance chance = MaxChance; void dump(Function* func) { - //std::cerr << " info\n"; + std::cerr << " info\n"; if (!actions.empty()) { - //std::cerr << " with last: " << getExpressionName(actions.back()) << '\n'; + std::cerr << " with last: " << getExpressionName(actions.back()) << '\n'; } - //std::cerr << " with chance: " << int(chance) << '\n'; + std::cerr << " with chance: " << int(chance) << '\n'; } }; @@ -263,14 +263,13 @@ struct BranchHintAnalysis : public Pass { for (auto& callerContext : entryToCallersMap[entry]) { auto& callerChance = callerContext.block->contents.chance; if (callerChance > entryChance) { - callerChance = entryChance; - // This adjustment to a basic block's chance may lead to more // inferences inside that function: do a flow. TODO we could flow only // from the caller blocks, and we could do these flows in parallel. auto* callerAnalysis = callerContext.analysis; auto* callerEntry = callerAnalysis->entry; auto oldCallerEntryChance = callerEntry->contents.chance; + callerChance = entryChance; callerAnalysis->flow(); // If our entry decreased in chance, we can propagate that to our @@ -288,19 +287,20 @@ struct BranchHintAnalysis : public Pass { // chance to go one way then the other, mark it as likely or unlikely // accordingly. TODO: should we not mark when the difference is small? for (auto& [func, analysis] : analyzer.map) { +//std::cerr << "lastloop on " << func->name << '\n'; for (auto& block : analysis.basicBlocks) { - //std::cerr << "lastloop block\n"; +//std::cerr << " lastloop block " << block.get() << " with chance " << int(block->contents.chance) << "\n"; if (block->contents.actions.empty() || block->out.size() != 2) { continue; } auto* last = block->contents.actions.back(); - //std::cerr << " last " << *last << "\n"; +//std::cerr << " last " << *last << "\n"; if (!analysis.isBranching(last)) { continue; } - //std::cerr << " chances1\n"; +//std::cerr << " chances1\n"; // Compare the probabilities of the two targets and see if we can infer // likelihood. if (auto likely = analysis.getLikelihood(last, diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index 50f2fc95f8c..c3bdb5d76b1 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -281,6 +281,7 @@ ) ;; CHECK: (func $call-chain (type $0) (param $x i32) + ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: (then @@ -289,7 +290,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $call-chain (param $x i32) - ;; This is unlikely as the callchain ends that way. XXX + ;; This is unlikely as the callchain ends that way. (if (local.get $x) (then From edc6294b081e4d294f815a2c3d06ca909f3e9e23 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 12:43:50 -0700 Subject: [PATCH 58/61] work --- src/passes/BranchHintAnalysis.cpp | 2 +- test/lit/passes/branch-hint-analysis-lto.wast | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 03b833d2f7b..6ebb42bf2d2 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -252,7 +252,7 @@ struct BranchHintAnalysis : public Pass { // chance and put them in a work queue. UniqueDeferredQueue work; for (auto& [entry, _] : entryToCallersMap) { - if (entry->contents.chance < MaxChance) { + if (entry && entry->contents.chance < MaxChance) { work.push(entry); } } diff --git a/test/lit/passes/branch-hint-analysis-lto.wast b/test/lit/passes/branch-hint-analysis-lto.wast index c3bdb5d76b1..1f939923037 100644 --- a/test/lit/passes/branch-hint-analysis-lto.wast +++ b/test/lit/passes/branch-hint-analysis-lto.wast @@ -6,6 +6,9 @@ ;; functions. (module + ;; CHECK: (import "a" "b" (func $import (type $1))) + (import "a" "b" (func $import)) + ;; CHECK: (tag $e (type $1)) (tag $e) @@ -298,4 +301,22 @@ ) ) ) + + ;; CHECK: (func $call-import (type $0) (param $x i32) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (call $import) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-import (param $x i32) + ;; We know nothing about imports. + (if + (local.get $x) + (then + (call $import) + ) + ) + ) ) From 1688bfe8cfea3a473710963a0d8ecd9d8f695266 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 12:55:52 -0700 Subject: [PATCH 59/61] work --- src/passes/BranchHintAnalysis.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 6ebb42bf2d2..6746d6ae301 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -231,6 +231,11 @@ struct BranchHintAnalysis : public Pass { // Whenever a function's entry block has low chance, that means callers are // low chance as well. Build a mapping to connect each entry function to the // callers, so we can update them later down. + // + // How much this cross-function analysis matters varies a lot by codebase, + // anywhere from 3%, 7%, 20%, to 50%. (The 50% is on code that uses partial + // inlining heavily, leaving many outlined throws, which can then be marked + // unlikely.) std::unordered_map> entryToCallersMap; for (auto& [_, analysis] : analyzer.map) { for (auto& callerBlock : analysis.basicBlocks) { From b212ac7dba88067ae35abee9d9aeda23b890089d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 May 2025 12:56:19 -0700 Subject: [PATCH 60/61] work --- src/passes/BranchHintAnalysis.cpp | 56 +++++++++++++++++-------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index 6746d6ae301..d928aea1582 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -48,12 +48,13 @@ struct Info { // The chance of the block being reached. We assume it is likely to be reached // until we see a signal otherwise. - Chance chance = MaxChance; + Chance chance = MaxChance; void dump(Function* func) { std::cerr << " info\n"; if (!actions.empty()) { - std::cerr << " with last: " << getExpressionName(actions.back()) << '\n'; + std::cerr << " with last: " << getExpressionName(actions.back()) + << '\n'; } std::cerr << " with chance: " << int(chance) << '\n'; } @@ -61,10 +62,13 @@ struct Info { // Analyze Branch Hints in a function, using a CFG. struct BranchHintCFGAnalysis - : public CFGWalker, Info> { + : public CFGWalker, + Info> { - using Super = - CFGWalker, Info>; + using Super = CFGWalker, + Info>; // We only look at things that branch twice, which is all branching // instructions but without br (without condition, which is an unconditional @@ -114,7 +118,8 @@ struct BranchHintCFGAnalysis // Apply all signals: if something tells us the block is unlikely, mark it // so. if (auto chance = getChance(curr)) { - currBasicBlock->contents.chance = std::min(currBasicBlock->contents.chance, *chance); + currBasicBlock->contents.chance = + std::min(currBasicBlock->contents.chance, *chance); } } @@ -131,9 +136,7 @@ struct BranchHintCFGAnalysis Super::doStartIfTrue(self, currp); } - void visitFunction(Function* curr) { - flow(); - } + void visitFunction(Function* curr) { flow(); } // Flow chances in a function, to find the chances of all blocks inside it. void flow() { @@ -151,7 +154,7 @@ struct BranchHintCFGAnalysis } while (!work.empty()) { auto* block = work.pop(); -//std::cerr << "work on " << debugIds[block] << '\n'; + // std::cerr << "work on " << debugIds[block] << '\n'; // We should not get here if there is no work. assert(!block->out.empty()); @@ -164,7 +167,8 @@ struct BranchHintCFGAnalysis } auto& chance = block->contents.chance; -//std::cerr << " old " << int(chance) << ", maxOut " << int(maxOut) << '\n'; + // std::cerr << " old " << int(chance) << ", maxOut " << int(maxOut) << + // '\n'; if (maxOut < chance) { chance = maxOut; for (auto* in : block->in) { @@ -176,9 +180,8 @@ struct BranchHintCFGAnalysis // Checks if a branch from a branching instruction to two targets is likely or // not (or unknown). - std::optional getLikelihood(Expression* brancher, - Chance first, - Chance second) { + std::optional + getLikelihood(Expression* brancher, Chance first, Chance second) { if (first == second) { // No data to suggest likelihood either way. return {}; @@ -201,9 +204,9 @@ struct BranchHintCFGAnalysis // generated by CallGraphPropertyAnalysis. This structure provides all the info // for one function. // TODO: we don't actually use CallGraphPropertyAnalysis?! -struct FunctionAnalysis : public BranchHintCFGAnalysis, - ModuleUtils::CallGraphPropertyAnalysis::FunctionInfo { -}; +struct FunctionAnalysis + : public BranchHintCFGAnalysis, + ModuleUtils::CallGraphPropertyAnalysis::FunctionInfo {}; struct BranchHintAnalysis : public Pass { // Locals are not modified here. @@ -236,7 +239,8 @@ struct BranchHintAnalysis : public Pass { // anywhere from 3%, 7%, 20%, to 50%. (The 50% is on code that uses partial // inlining heavily, leaving many outlined throws, which can then be marked // unlikely.) - std::unordered_map> entryToCallersMap; + std::unordered_map> + entryToCallersMap; for (auto& [_, analysis] : analyzer.map) { for (auto& callerBlock : analysis.basicBlocks) { // Calls only appear at the end of blocks. @@ -292,25 +296,27 @@ struct BranchHintAnalysis : public Pass { // chance to go one way then the other, mark it as likely or unlikely // accordingly. TODO: should we not mark when the difference is small? for (auto& [func, analysis] : analyzer.map) { -//std::cerr << "lastloop on " << func->name << '\n'; + // std::cerr << "lastloop on " << func->name << '\n'; for (auto& block : analysis.basicBlocks) { -//std::cerr << " lastloop block " << block.get() << " with chance " << int(block->contents.chance) << "\n"; + // std::cerr << " lastloop block " << block.get() << " with chance " << + // int(block->contents.chance) << "\n"; if (block->contents.actions.empty() || block->out.size() != 2) { continue; } auto* last = block->contents.actions.back(); -//std::cerr << " last " << *last << "\n"; + // std::cerr << " last " << *last << "\n"; if (!analysis.isBranching(last)) { continue; } -//std::cerr << " chances1\n"; + // std::cerr << " chances1\n"; // Compare the probabilities of the two targets and see if we can infer // likelihood. - if (auto likely = analysis.getLikelihood(last, - block->out[0]->contents.chance, - block->out[1]->contents.chance)) { + if (auto likely = + analysis.getLikelihood(last, + block->out[0]->contents.chance, + block->out[1]->contents.chance)) { // We have a useful hint! func->codeAnnotations[last].branchLikely = likely; } From a70ce945c9c8dfad8c56f93ca96b975e0e887c7b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 May 2025 13:15:32 -0700 Subject: [PATCH 61/61] clean --- src/passes/BranchHintAnalysis.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/passes/BranchHintAnalysis.cpp b/src/passes/BranchHintAnalysis.cpp index d928aea1582..1a77505d522 100644 --- a/src/passes/BranchHintAnalysis.cpp +++ b/src/passes/BranchHintAnalysis.cpp @@ -154,7 +154,6 @@ struct BranchHintCFGAnalysis } while (!work.empty()) { auto* block = work.pop(); - // std::cerr << "work on " << debugIds[block] << '\n'; // We should not get here if there is no work. assert(!block->out.empty()); @@ -167,8 +166,6 @@ struct BranchHintCFGAnalysis } auto& chance = block->contents.chance; - // std::cerr << " old " << int(chance) << ", maxOut " << int(maxOut) << - // '\n'; if (maxOut < chance) { chance = maxOut; for (auto* in : block->in) { @@ -296,21 +293,16 @@ struct BranchHintAnalysis : public Pass { // chance to go one way then the other, mark it as likely or unlikely // accordingly. TODO: should we not mark when the difference is small? for (auto& [func, analysis] : analyzer.map) { - // std::cerr << "lastloop on " << func->name << '\n'; for (auto& block : analysis.basicBlocks) { - // std::cerr << " lastloop block " << block.get() << " with chance " << - // int(block->contents.chance) << "\n"; if (block->contents.actions.empty() || block->out.size() != 2) { continue; } auto* last = block->contents.actions.back(); - // std::cerr << " last " << *last << "\n"; if (!analysis.isBranching(last)) { continue; } - // std::cerr << " chances1\n"; // Compare the probabilities of the two targets and see if we can infer // likelihood. if (auto likely =