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) )