From 1456ddff5c8f97daee1337efa845368feadb14cd Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Apr 2026 21:21:33 -0700 Subject: [PATCH] Simplify branch hint instrumentation The previous branch hint instrumentation logic would introduce a scratch local to hold the condition so it could be passed into both the logging function and the original branching instruction. The de-instrumentation pass would then need to find this local and attempt to undo the data flow change. Simplify all of this by having the logging function return the condition value so it can interpose between the condition and the branch without any new locals. De-instrumentation can now just replace the call to the log function with its condition parameter. To allow further simplification, also change the order of parameters to the logging function so the condition value is the first parameter. This ensures that we don't need to introduce a scratch local even when the condition is a `pop`, because the pop will remain the leftmost leaf expression in the catch body. --- scripts/fuzz_opt.py | 4 +- scripts/fuzz_shell.js | 5 +- src/passes/InstrumentBranchHints.cpp | 219 ++++-------------- src/tools/execution-results.h | 3 + test/lit/name-overlap.wast | 4 +- .../lit/passes/deinstrument-branch-hints.wast | 169 +------------- test/lit/passes/delete-branch-hints.wast | 96 +++----- test/lit/passes/instrument-branch-hints.wast | 219 ++++++------------ 8 files changed, 158 insertions(+), 561 deletions(-) diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py index 3e41356477e..8af017f956d 100755 --- a/scripts/fuzz_opt.py +++ b/scripts/fuzz_opt.py @@ -2281,7 +2281,7 @@ def handle(self, wasm): for line in out.splitlines(): if line.startswith(LOG_BRANCH_PREFIX): # (1:-1 strips away the '[', ']' at the edges) - _, _, id_, hint, actual = line[1:-1].split(' ') + _, _, actual, hint, id_ = line[1:-1].split(' ') all_ids.add(id_) if hint != actual: # This hint was misleading. @@ -2443,7 +2443,7 @@ def handle(self, wasm): continue for line in group: if line.startswith(LOG_BRANCH_PREFIX): - _, _, id_, hint, actual = line[1:-1].split(' ') + _, _, actual, hint, id_ = line[1:-1].split(' ') hint = int(hint) actual = int(actual) assert hint in (0, 1) diff --git a/scripts/fuzz_shell.js b/scripts/fuzz_shell.js index 28f5e2f44fb..d16c49624b9 100644 --- a/scripts/fuzz_shell.js +++ b/scripts/fuzz_shell.js @@ -400,8 +400,9 @@ var baseImports = { }); }, - 'log-branch': (id, expected, actual) => { - console.log(`[LoggingExternalInterface log-branch ${id} ${expected} ${actual}]`); + 'log-branch': (actual, expected, id) => { + console.log(`[LoggingExternalInterface log-branch ${actual} ${expected} ${id}]`); + return actual; }, }, // Emscripten support. diff --git a/src/passes/InstrumentBranchHints.cpp b/src/passes/InstrumentBranchHints.cpp index 0fa6f3009b6..c552ddae891 100644 --- a/src/passes/InstrumentBranchHints.cpp +++ b/src/passes/InstrumentBranchHints.cpp @@ -28,9 +28,9 @@ // into // // @metadata.branch.hint B -// ;; log the ID of the condition (123), the prediction (B), and the actual -// ;; runtime result (temp == condition). -// if (temp = condition; log(123, B, temp); temp) { +// ;; log the actual runtime result (condition), the prediction (B), and the +// ;; ID (123), and return that result. +// if (log(condition, B, 123)) { // X // } else { // Y @@ -39,19 +39,20 @@ // Concretely, we emit calls to this logging function: // // (import "fuzzing-support" "log-branch" -// (func $log-branch (param i32 i32 i32)) ;; ID, prediction, actual +// (func $log-branch (param i32 i32 i32) (result i32)) // ) // // This can be used to verify that branch hints are accurate, by implementing // the import like this for example: // -// imports['fuzzing-support']['log-branch'] = (id, prediction, actual) => { +// imports['fuzzing-support']['log-branch'] = (actual, prediction, id) => { // // We only care about truthiness of the expected and actual values. // expected = +!!expected; // actual = +!!actual; // // Throw if the hint said this branch would be taken, but it was not, or // // vice versa. // if (expected != actual) throw `Bad branch hint! (${id})`; +// return actual; // }; // // A pass to delete branch hints is also provided, which finds instrumentations @@ -63,28 +64,28 @@ // would do this transformation: // // @metadata.branch.hint A -// if (temp = condition; log(10, A, temp); temp) { // 10 matches one of 10,20 +// if (log(condition, A, 10)) { // 10 matches one of 10,20 // X // } // @metadata.branch.hint B -// if (temp = condition; log(99, B, temp); temp) { // 99 does not match +// if (log(condition, B, 99)) { // 99 does not match // Y // } // // => // // // Used to be a branch hint here, but it was deleted. -// if (temp = condition; log(10, A, temp); temp) { +// if (log(condition, A, 10)) { // X // } // @metadata.branch.hint B // this one is unmodified. -// if (temp = condition; log(99, B, temp); temp) { +// if (log(condition, B, 99)) { // Y // } // // A pass to undo the instrumentation is also provided, which does // -// if (temp = condition; log(123, A, temp); temp) { +// if (log(condition, A, 123)) { // X // } // @@ -95,14 +96,8 @@ // } // -#include "ir/drop.h" #include "ir/effects.h" -#include "ir/eh-utils.h" -#include "ir/find_all.h" -#include "ir/local-graph.h" #include "ir/names.h" -#include "ir/parents.h" -#include "ir/properties.h" #include "ir/utils.h" #include "pass.h" #include "support/string.h" @@ -133,8 +128,6 @@ int branchId = 1; struct InstrumentBranchHints : public WalkerPass> { - using Super = WalkerPass>; - // The internal name of our import. Name logBranch; @@ -148,8 +141,6 @@ struct InstrumentBranchHints // TODO: BrOn, but the condition there is not an i32 - bool addedInstrumentation = false; - template void processCondition(T* curr) { if (curr->condition->type == Type::unreachable) { // This branch is not even reached. @@ -167,25 +158,11 @@ struct InstrumentBranchHints int id = branchId++; // Instrument the condition. - auto tempLocal = builder.addVar(getFunction(), Type::i32); - auto* set = builder.makeLocalSet(tempLocal, curr->condition); auto* idConst = builder.makeConst(Literal(int32_t(id))); auto* guess = builder.makeConst(Literal(int32_t(*likely))); - auto* get1 = builder.makeLocalGet(tempLocal, Type::i32); - auto* log = builder.makeCall(logBranch, {idConst, guess, get1}, Type::none); - auto* get2 = builder.makeLocalGet(tempLocal, Type::i32); - curr->condition = builder.makeBlock({set, log, get2}); - addedInstrumentation = true; - } - - void doWalkFunction(Function* func) { - Super::doWalkFunction(func); - // Our added blocks may have caused nested pops. - if (addedInstrumentation) { - EHUtils::handleBlockNestedPops(func, *getModule()); - addedInstrumentation = false; - } + curr->condition = + builder.makeCall(logBranch, {curr->condition, guess, idConst}, Type::i32); } void doWalkModule(Module* module) { @@ -193,7 +170,12 @@ struct InstrumentBranchHints // This file already has our import. We nop it out, as whatever the // current code does may be dangerous (it may log incorrect hints). auto* func = module->getFunction(existing); - func->body = Builder(*module).makeNop(); + Builder builder(*module); + if (func->getSig().results == Type::none) { + func->body = builder.makeNop(); + } else { + func->body = builder.makeUnreachable(); + } func->module = func->base = Name(); func->type = func->type.with(Exact); } @@ -201,7 +183,7 @@ struct InstrumentBranchHints // Add our import. auto* func = module->addFunction(Builder::makeFunction( Names::getValidFunctionName(*module, BASE), - Type(Signature({Type::i32, Type::i32, Type::i32}, Type::none), + Type(Signature({Type::i32, Type::i32, Type::i32}, Type::i32), NonNullable, Inexact), {})); @@ -210,7 +192,7 @@ struct InstrumentBranchHints logBranch = func->name; // Walk normally, using logBranch as we go. - Super::doWalkModule(module); + PostWalker::doWalkModule(module); // Update ref.func type changes. ReFinalize().run(getPassRunner(), module); @@ -228,12 +210,6 @@ struct InstrumentationProcessor : public WalkerPass> { // The internal name of our import. Name logBranch; - // A LocalGraph, so we can identify the pattern. - std::unique_ptr localGraph; - - // A map of expressions to their parents, so we can identify the pattern. - std::unique_ptr parents; - Sub* self() { return static_cast(this); } void visitIf(If* curr) { self()->processCondition(curr); } @@ -246,15 +222,6 @@ struct InstrumentationProcessor : public WalkerPass> { // TODO: BrOn, but the condition there is not an i32 - void doWalkFunction(Function* func) { - localGraph = std::make_unique(func, this->getModule()); - localGraph->computeSetInfluences(); - - parents = std::make_unique(func->body); - - Super::doWalkFunction(func); - } - void doWalkModule(Module* module) { logBranch = getLogBranchImport(module); if (!logBranch) { @@ -267,73 +234,14 @@ struct InstrumentationProcessor : public WalkerPass> { // Helpers - // Instrumentation info for a chunk of code that is the result of the - // instrumentation pass. - struct Instrumentation { - // The condition before the instrumentation (a pointer to it, so we can - // replace it). - Expression** originalCondition; - // The local that the original condition is stored in temporarily. - Index tempLocal; - // The call to the logging that the instrumentation added. - Call* call; - }; - - // Check if an expression's condition is an instrumentation, and return the - // info if so. - std::optional getInstrumentation(Expression* condition) { - // We must identify this pattern: - // - // (br_if - // (block - // (local.set $temp (condition)) - // (call $log (id, prediction, (local.get $temp))) - // (local.get $temp) - // ) - // - // The block may vanish during roundtrip though, so we just follow back from - // the last local.get, which appears in the condition: - // - // (local.set $temp (condition)) - // (call $log (id, prediction, (local.get $temp))) - // (br_if - // (local.get $temp) - // - auto* fallthrough = Properties::getFallthrough( - condition, this->getPassOptions(), *this->getModule()); - auto* get = fallthrough->template dynCast(); - if (!get) { - return {}; - } - auto& sets = localGraph->getSets(get); - if (sets.size() != 1) { - return {}; - } - auto* set = *sets.begin(); - if (!set) { - return {}; - } - auto& gets = localGraph->getSetInfluences(set); - if (gets.size() != 2) { - return {}; - } - // The set has two gets: the get in the condition we began at, and - // another. - LocalGet* otherGet = nullptr; - for (auto* get2 : gets) { - if (get2 != get) { - otherGet = get2; - } - } - assert(otherGet); - // See if that other get is used in a logging. The parent should be a - // logging call. - auto* call = parents->getParent(otherGet)->template dynCast(); + // Check if an expression's condition is instrumented, and return the + // instrumentation call if so. Otherwise return null. + Call* getInstrumentation(Expression* condition) { + auto* call = condition->dynCast(); if (!call || call->target != logBranch) { - return {}; + return nullptr; } - // Great, this is indeed a prior instrumentation. - return Instrumentation{&set->value, set->index, call}; + return call; } }; @@ -344,8 +252,8 @@ struct DeleteBranchHints : public InstrumentationProcessor { std::unordered_set idsToDelete; template void processCondition(T* curr) { - if (auto info = getInstrumentation(curr->condition)) { - if (auto* c = info->call->operands[0]->template dynCast()) { + if (auto* call = getInstrumentation(curr->condition)) { + if (auto* c = call->operands[2]->template dynCast()) { auto id = c->value.geti32(); if (idsToDelete.contains(id)) { // Remove the branch hint. @@ -368,78 +276,31 @@ struct DeleteBranchHints : public InstrumentationProcessor { }; struct DeInstrumentBranchHints - : public InstrumentationProcessor { + : public WalkerPass> { - template void processCondition(T* curr) { - if (auto info = getInstrumentation(curr->condition)) { - // Replace the instrumented condition with the original one (swap so that - // the IR remains valid: we cannot use the same expression twice in our - // IR, and the original condition is still used in another place, until - // we remove the logging calls; since we will remove the calls anyhow, we - // just need some valid IR there). - // - // Check for dangerous effects in the condition we are about to replace, - // to avoid a situation where the condition looks like this: - // - // (set $temp (original condition)) - // ..effects.. - // (local.get $temp) - // - // We cannot replace all this with the original condition, as it would - // remove the effects. (Even in that case we will remove the actual call - // to log the branch hint, below, so this just prevents some cleanup that - // is normally safe - the cleanup is mainly useful to allow inspection of - // testcases for debugging.) - EffectAnalyzer effects(getPassOptions(), *getModule(), curr->condition); - // The only condition we allow is a write to the temp local from the - // instrumentation, which getInstrumentation() verified has no other uses - // than us. - effects.localsWritten.erase(info->tempLocal); - if (!effects.hasUnremovableSideEffects()) { - std::swap(curr->condition, *info->originalCondition); - } - } - } + // The internal name of our import. + Name logBranch; - void visitFunction(Function* func) { - if (func->imported()) { - return; - } - // At the very end, remove all logging calls (we use them during the main - // walk to identify instrumentation). - for (auto** callp : FindAllPointers(func->body).list) { - auto* call = (*callp)->cast(); - if (call->target == logBranch) { - Builder builder(*getModule()); - Expression* last; - if (call->type == Type::none) { - last = builder.makeNop(); - } else { - last = builder.makeUnreachable(); - } - *callp = getDroppedChildrenAndAppend(call, - *getModule(), - getPassOptions(), - last, - // We know the call is removable. - DropMode::IgnoreParentEffects); - } + void visitCall(Call* curr) { + if (curr->target == logBranch) { + // Replace the call with its first operand (the original condition). + replaceCurrent(curr->operands[0]); } } void doWalkModule(Module* module) { - auto logBranchImport = getLogBranchImport(module); - if (!logBranchImport) { + logBranch = getLogBranchImport(module); + if (!logBranch) { Fatal() << "No branch hint logging import found. Was this code instrumented?"; } // Mark the log-branch import as having no side effects - we are removing it // entirely here, and its effect should not stop us when we compute effects. - module->getFunction(logBranchImport)->effects = + module->getFunction(logBranch)->effects = std::make_shared(getPassOptions(), *module); - InstrumentationProcessor::doWalkModule(module); + WalkerPass>::doWalkModule(module); } }; diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index 516d9f60603..7717ec23811 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -180,6 +180,9 @@ struct LoggingExternalInterface : public ShellExternalInterface { } } std::cout << "]\n"; + if (import->base == "log-branch") { + return arguments[0]; + } return {}; } else if (import->base == "throw") { // Throw something, depending on the value of the argument. 0 means diff --git a/test/lit/name-overlap.wast b/test/lit/name-overlap.wast index 4caa4785e56..78229488afc 100644 --- a/test/lit/name-overlap.wast +++ b/test/lit/name-overlap.wast @@ -15,11 +15,11 @@ ;; CHECK: (type $1 (func (param f32))) - ;; CHECK: (type $2 (func (param i32 i32 i32))) + ;; CHECK: (type $2 (func (param i32 i32 i32) (result i32))) ;; CHECK: (import "fuzzing-support" "log-i64" (func $fimport$2 (type $0) (param i64))) (import "fuzzing-support" "log-i64" (func $fimport$2 (param i64))) ;; CHECK: (import "fuzzing-support" "log-f32" (func $fimport$3 (type $1) (param f32))) (import "fuzzing-support" "log-f32" (func $fimport$3 (param f32))) ) -;; CHECK: (import "fuzzing-support" "log-branch" (func $fimport$2_2 (type $2) (param i32 i32 i32))) +;; CHECK: (import "fuzzing-support" "log-branch" (func $fimport$2_2 (type $2) (param i32 i32 i32) (result i32))) diff --git a/test/lit/passes/deinstrument-branch-hints.wast b/test/lit/passes/deinstrument-branch-hints.wast index 3f9019028e7..4ff67e3e019 100644 --- a/test/lit/passes/deinstrument-branch-hints.wast +++ b/test/lit/passes/deinstrument-branch-hints.wast @@ -5,15 +5,12 @@ (module ;; CHECK: (type $0 (func)) - ;; CHECK: (type $1 (func (param i32 i32 i32))) + ;; CHECK: (type $1 (func (param i32 i32 i32) (result i32))) - ;; CHECK: (type $2 (func (result i32))) - - ;; CHECK: (import "fuzzing-support" "log-branch" (func $log (type $1) (param i32 i32 i32))) - (import "fuzzing-support" "log-branch" (func $log (param i32 i32 i32))) + ;; CHECK: (import "fuzzing-support" "log-branch" (func $log (type $1) (param i32 i32 i32) (result i32))) + (import "fuzzing-support" "log-branch" (func $log (param i32 i32 i32) (result i32))) ;; CHECK: (func $if (type $0) - ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if ;; CHECK-NEXT: (i32.const 42) @@ -30,21 +27,14 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $if - (local $temp i32) ;; The instrumentation should be removed, and the if's condition should ;; be 42. (@metadata.code.branch_hint "\00") (if - (block (result i32) - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 1) - (i32.const 0) - (local.get $temp) - ) - (local.get $temp) + (call $log + (i32.const 42) + (i32.const 0) + (i32.const 1) ) (then (drop @@ -60,7 +50,6 @@ ) ;; CHECK: (func $br (type $0) - ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (br_if $out @@ -70,151 +59,13 @@ ;; CHECK-NEXT: ) (func $br ;; The same, with a br. - (local $temp i32) - (block $out - (@metadata.code.branch_hint "\01") - (br_if $out - (block (result i32) - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 4) - (i32.const 0) - (local.get $temp) - ) - (local.get $temp) - ) - ) - ) - ) - - ;; CHECK: (func $br-before (type $0) - ;; CHECK-NEXT: (local $temp i32) - ;; CHECK-NEXT: (block $out - ;; CHECK-NEXT: (local.set $temp - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") - ;; CHECK-NEXT: (br_if $out - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $br-before - ;; As above, but the instrumentation is before us, leaving only a local.get - ;; in the br's condition. We should still identify the pattern and remove - ;; the logging (but we leave the local.set for other things to clean up). - (local $temp i32) (block $out - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 4) - (i32.const 0) - (local.get $temp) - ) (@metadata.code.branch_hint "\01") (br_if $out - (local.get $temp) - ) - ) - ) - - ;; CHECK: (func $br-before-effects (type $0) - ;; CHECK-NEXT: (local $temp i32) - ;; CHECK-NEXT: (local $other i32) - ;; CHECK-NEXT: (block $out - ;; CHECK-NEXT: (local.set $temp - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (block - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.tee $other - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") - ;; CHECK-NEXT: (br_if $out - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $br-before-effects - ;; As above, but there are effects in the call's children that we must - ;; keep. - (local $temp i32) - (local $other i32) - (block $out - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 4) - (local.tee $other ;; this tee must be kept around + (call $log + (i32.const 42) (i32.const 0) - ) - (local.get $temp) - ) - (@metadata.code.branch_hint "\01") - (br_if $out - (local.get $temp) - ) - ) - ) - - ;; CHECK: (func $if-unreachable (type $2) (result i32) - ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (block $block (result i32) - ;; CHECK-NEXT: (br_if $block - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (block - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (then - ;; CHECK-NEXT: (unreachable) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $if-unreachable (result i32) - (local $0 i32) - ;; The unreachable here must be executed. Normally we replace the br_if's - ;; entire condition, but here we only remove the call to $log. - (block $block (result i32) - (br_if $block - (i32.const 0) - (block (result i32) - (block - (local.set $0 - (i32.const 42) - ) - (if - (i32.const 1) - (then - (unreachable) - ) - ) - ) - (call $log - (i32.const 0) - (i32.const 0) - (local.get $0) - ) - (local.get $0) + (i32.const 4) ) ) ) diff --git a/test/lit/passes/delete-branch-hints.wast b/test/lit/passes/delete-branch-hints.wast index 375b10d16c2..bbc59604a53 100644 --- a/test/lit/passes/delete-branch-hints.wast +++ b/test/lit/passes/delete-branch-hints.wast @@ -5,24 +5,17 @@ (module ;; CHECK: (type $0 (func)) - ;; CHECK: (type $1 (func (param i32 i32 i32))) + ;; CHECK: (type $1 (func (param i32 i32 i32) (result i32))) - ;; CHECK: (import "fuzzing-support" "log-branch" (func $log (type $1) (param i32 i32 i32))) - (import "fuzzing-support" "log-branch" (func $log (param i32 i32 i32))) + ;; CHECK: (import "fuzzing-support" "log-branch" (func $log (type $1) (param i32 i32 i32) (result i32))) + (import "fuzzing-support" "log-branch" (func $log (param i32 i32 i32) (result i32))) ;; CHECK: (func $if-10 (type $0) - ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $temp - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log - ;; CHECK-NEXT: (i32.const 10) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $temp) + ;; CHECK-NEXT: (call $log + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 10) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -37,21 +30,14 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $if-10 - (local $temp i32) ;; The branch hint should be removed, since the ID "10" is in the list of ;; 10, 30. (@metadata.code.branch_hint "\00") (if - (block (result i32) - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 10) - (i32.const 0) - (local.get $temp) - ) - (local.get $temp) + (call $log + (i32.const 42) + (i32.const 0) + (i32.const 10) ) (then (drop @@ -67,19 +53,12 @@ ) ;; CHECK: (func $if-20 (type $0) - ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $temp - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log - ;; CHECK-NEXT: (i32.const 20) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $temp) + ;; CHECK-NEXT: (call $log + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 20) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -94,20 +73,13 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $if-20 - (local $temp i32) ;; The branch hint should *not* be removed: 20 is not in the list. (@metadata.code.branch_hint "\00") (if - (block (result i32) - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 20) - (i32.const 0) - (local.get $temp) - ) - (local.get $temp) + (call $log + (i32.const 42) + (i32.const 0) + (i32.const 20) ) (then (drop @@ -123,39 +95,25 @@ ) ;; CHECK: (func $br-30 (type $0) - ;; CHECK-NEXT: (local $temp i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (br_if $out - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $temp - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log - ;; CHECK-NEXT: (i32.const 30) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $temp) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $temp) + ;; CHECK-NEXT: (call $log + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 30) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $br-30 ;; The hint should be removed. - (local $temp i32) (block $out (@metadata.code.branch_hint "\01") (br_if $out - (block (result i32) - (local.set $temp - (i32.const 42) - ) - (call $log - (i32.const 30) - (i32.const 0) - (local.get $temp) - ) - (local.get $temp) + (call $log + (i32.const 42) + (i32.const 0) + (i32.const 30) ) ) ) diff --git a/test/lit/passes/instrument-branch-hints.wast b/test/lit/passes/instrument-branch-hints.wast index 5a6d73f5e0e..8998e3dbdb4 100644 --- a/test/lit/passes/instrument-branch-hints.wast +++ b/test/lit/passes/instrument-branch-hints.wast @@ -11,28 +11,20 @@ ;; CHECK: (type $3 (func (param anyref))) - ;; CHECK: (type $4 (func (param i32 i32 i32))) + ;; CHECK: (type $4 (func (param i32 i32 i32) (result i32))) - ;; CHECK: (import "fuzzing-support" "log-branch" (func $log-branch (type $4) (param i32 i32 i32))) + ;; CHECK: (import "fuzzing-support" "log-branch" (func $log-branch (type $4) (param i32 i32 i32) (result i32))) ;; CHECK: (tag $i32 (type $1) (param i32)) (tag $i32 (param i32)) ;; CHECK: (func $if (type $0) - ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -47,16 +39,10 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (i32.const 142) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 2) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 142) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (i32.const 2) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -95,7 +81,6 @@ ) ;; CHECK: (func $if-2 (type $0) - ;; CHECK-NEXT: (local $0 i32) ;; CHECK-NEXT: (if ;; CHECK-NEXT: (i32.const 242) ;; CHECK-NEXT: (then @@ -111,16 +96,10 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (i32.const 342) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 3) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 342) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 3) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -158,21 +137,13 @@ ) ;; CHECK: (func $br (type $0) - ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (br_if $out - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 4) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 4) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop @@ -182,16 +153,10 @@ ;; CHECK-NEXT: (block $out1 ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (br_if $out1 - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (i32.const 142) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 5) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 142) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (i32.const 5) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop @@ -239,22 +204,15 @@ ;; CHECK: (func $br_value (type $2) (result f64) ;; CHECK-NEXT: (local $scratch f64) - ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (block $out (result f64) ;; CHECK-NEXT: (local.set $scratch ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (br_if $out ;; CHECK-NEXT: (f64.const 3.14159) - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 6) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 6) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -277,54 +235,33 @@ ) ;; CHECK: (func $nested (type $0) - ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (local $1 i32) - ;; CHECK-NEXT: (local $2 i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $2 - ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") - ;; CHECK-NEXT: (if (result i32) - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 7) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (then - ;; CHECK-NEXT: (i32.const 142) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (else - ;; CHECK-NEXT: (i32.const 242) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") + ;; CHECK-NEXT: (if (result i32) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (i32.const 7) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (i32.const 142) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (i32.const 242) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 9) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $2) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $2) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 9) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (i32.const 342) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 8) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (i32.const 342) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 8) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -385,29 +322,18 @@ ) ;; CHECK: (func $eh-pop (type $0) - ;; CHECK-NEXT: (local $0 i32) - ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (block $label ;; CHECK-NEXT: (try ;; CHECK-NEXT: (do ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (catch $i32 - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (pop i32) - ;; CHECK-NEXT: ) ;; CHECK-NEXT: (@metadata.code.branch_hint "\00") ;; CHECK-NEXT: (br_if $label - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 10) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (pop i32) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (i32.const 10) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -423,8 +349,8 @@ (catch $i32 (@metadata.code.branch_hint "\00") (br_if $label - ;; This pop will end up in a block after our instrumentation, which - ;; requires fixups. + ;; This pop will end up as the first parameter of the call, so it + ;; will not need fixups. (pop i32) ) ) @@ -436,43 +362,38 @@ ;; This module has an existing import with our module and base names. We nop it ;; and create a fresh one, to avoid confusion. (module - (import "fuzzing-support" "log-branch" (func $existing (param i32 i32 i32))) + (import "fuzzing-support" "log-branch" (func $existing (param i32 i32 i32) (result i32))) - ;; CHECK: (type $0 (func (param i32 i32 i32))) + ;; CHECK: (type $0 (func (param i32 i32 i32) (result i32))) ;; CHECK: (type $1 (func)) - ;; CHECK: (import "fuzzing-support" "log-branch" (func $log-branch (type $0) (param i32 i32 i32))) + ;; CHECK: (import "fuzzing-support" "log-branch" (func $log-branch (type $0) (param i32 i32 i32) (result i32))) - ;; CHECK: (func $existing (type $0) (param $0 i32) (param $1 i32) (param $2 i32) - ;; CHECK-NEXT: (nop) + ;; CHECK: (func $existing (type $0) (param $0 i32) (param $1 i32) (param $2 i32) (result i32) + ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) ;; CHECK: (func $if (type $1) ;; CHECK-NEXT: (local $x i32) - ;; CHECK-NEXT: (local $1 i32) ;; CHECK-NEXT: (@metadata.code.branch_hint "\01") ;; CHECK-NEXT: (if - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $1 - ;; CHECK-NEXT: (block (result i32) - ;; CHECK-NEXT: (local.set $x - ;; CHECK-NEXT: (i32.const 42) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $log-branch + ;; CHECK-NEXT: (block (result i32) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (call $existing ;; CHECK-NEXT: (i32.const 42) ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $x) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $log-branch - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (then ;; CHECK-NEXT: (drop @@ -489,10 +410,12 @@ (local.set $x (i32.const 42) ) - (call $existing - (i32.const 42) - (i32.const 1) - (local.get $x) + (drop + (call $existing + (i32.const 42) + (i32.const 1) + (local.get $x) + ) ) (local.get $x) )