diff --git a/scripts/test/fuzzing.py b/scripts/test/fuzzing.py index 9c0125a9206..b273e5bfaf7 100644 --- a/scripts/test/fuzzing.py +++ b/scripts/test/fuzzing.py @@ -109,6 +109,12 @@ # Contains a name with "__fuzz_split", indicating it is emitted by # wasm-split, confusing the fuzzer because it is in the initial content. 'fuzz_shell_second.wast', + # We cannot fuzz semantics-altering intrinsics, as when we optimize the + # behavior changes. + 'removable-if-unused.wast', + 'removable-if-unused-func.wast', + 'vacuum-removable-if-unused.wast', + 'vacuum-removable-if-unused-func.wast', ] diff --git a/src/ir/intrinsics.h b/src/ir/intrinsics.h index 43e080b806b..51ac3a6436b 100644 --- a/src/ir/intrinsics.h +++ b/src/ir/intrinsics.h @@ -111,6 +111,48 @@ class Intrinsics { std::vector getConfigureAllFunctions(Call* call); // As above, but looks through the module to find the configureAll. std::vector getConfigureAllFunctions(); + + // Get the code annotations for an expression in a function. + CodeAnnotation getAnnotations(Expression* curr, Function* func) { + auto& annotations = func->codeAnnotations; + auto iter = annotations.find(curr); + if (iter != annotations.end()) { + return iter->second; + } + return {}; + } + + // Get the code annotations for a function itself. + CodeAnnotation getAnnotations(Function* func) { + return getAnnotations(nullptr, func); + } + + // Given a call in a function, return all the annotations for it. The call may + // be annotated itself (which takes precedence), or the function it calls be + // annotated. + CodeAnnotation getCallAnnotations(Call* call, Function* func) { + // Combine annotations from the call itself and from the called function. + auto ret = getAnnotations(call, func); + + // Check on the called function, if it exists (it may not if the IR is still + // being built up). + if (auto* target = module.getFunctionOrNull(call->target)) { + auto funcAnnotations = getAnnotations(target); + + // Merge them, giving precedence for the call annotation. + if (!ret.branchLikely) { + ret.branchLikely = funcAnnotations.branchLikely; + } + if (!ret.inline_) { + ret.inline_ = funcAnnotations.inline_; + } + if (!ret.removableIfUnused) { + ret.removableIfUnused = funcAnnotations.removableIfUnused; + } + } + + return ret; + } }; } // namespace wasm diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 4f9786c0a51..3978afff45d 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1315,6 +1315,8 @@ struct AnnotationParserCtx { branchHint = &a; } else if (a.kind == Annotations::InlineHint) { inlineHint = &a; + } else if (a.kind == Annotations::RemovableIfUnusedHint) { + ret.removableIfUnused = true; } } diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index fb4375f4bc2..d92a412dbbd 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -2791,6 +2791,12 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) { restoreNormalColor(o); doIndent(o, indent); } + if (annotation.removableIfUnused) { + Colors::grey(o); + o << "(@" << Annotations::RemovableIfUnusedHint << ")\n"; + restoreNormalColor(o); + doIndent(o, indent); + } } } diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index b5e223d9f21..0bcb3a387e6 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -91,10 +92,8 @@ struct Vacuum : public WalkerPass> { curr->is() || curr->is() || curr->is()) { return curr; } - // Check if this expression itself has side effects, ignoring children. - EffectAnalyzer self(getPassOptions(), *getModule()); - self.visit(curr); - if (self.hasUnremovableSideEffects()) { + // Check if this expression itself must be kept. + if (mustKeepUnusedParent(curr)) { return curr; } // The result isn't used, and this has no side effects itself, so we can @@ -130,6 +129,26 @@ struct Vacuum : public WalkerPass> { } } + // Check if a parent expression must be kept around, given the knowledge that + // its result is unused (dropped). This is basically just a call to + // ShallowEffectAnalyzer to see if we can remove it, except that given the + // result is unused, the relevant hint may help us. (This just checks the + // parent itself: it may have children that the caller must check and keep + // around if so.) + bool mustKeepUnusedParent(Expression* curr) { + if (auto* call = curr->dynCast()) { + // If |curr| is marked as removable if unused, then it is removable + // without even checking effects. + if (Intrinsics(*getModule()) + .getCallAnnotations(call, getFunction()) + .removableIfUnused) { + return false; + } + } + ShallowEffectAnalyzer self(getPassOptions(), *getModule(), curr); + return self.hasUnremovableSideEffects(); + } + void visitBlock(Block* curr) { auto& list = curr->list; diff --git a/src/wasm-annotations.h b/src/wasm-annotations.h index 888c356dcb9..fec7f067795 100644 --- a/src/wasm-annotations.h +++ b/src/wasm-annotations.h @@ -27,6 +27,7 @@ namespace wasm::Annotations { extern const Name BranchHint; extern const Name InlineHint; +extern const Name RemovableIfUnusedHint; } // namespace wasm::Annotations diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 1706d657916..3bd3199b67a 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1441,6 +1441,7 @@ class WasmBinaryWriter { std::optional getBranchHintsBuffer(); std::optional getInlineHintsBuffer(); + std::optional getRemovableIfUnusedHintsBuffer(); // helpers void writeInlineString(std::string_view name); @@ -1733,6 +1734,7 @@ class WasmBinaryReader { void readBranchHints(size_t payloadLen); void readInlineHints(size_t payloadLen); + void readremovableIfUnusedHints(size_t payloadLen); std::tuple readMemoryAccess(bool isAtomic, bool isRMW); diff --git a/src/wasm.h b/src/wasm.h index 693ac145c28..e74d6532ac2 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2233,7 +2233,9 @@ struct BinaryLocations { // Forward declaration for FuncEffectsMap. class EffectAnalyzer; -// Code annotations for VMs. +// Annotation for a particular piece of code. This includes std::optionals for +// all possible annotations, with the ones present being filled in (or just a +// bool for an annotation with one possible value). struct CodeAnnotation { // Branch Hinting proposal: Whether the branch is likely, or unlikely. std::optional branchLikely; @@ -2243,8 +2245,15 @@ struct CodeAnnotation { static const uint8_t AlwaysInline = 127; std::optional inline_; + // Toolchain hint: If this expression's result is unused, then the entire + // thing can be considered dead and removable. See + // + // https://github.com/WebAssembly/binaryen/wiki/Optimizer-Cookbook#intrinsics + bool removableIfUnused = false; + bool operator==(const CodeAnnotation& other) const { - return branchLikely == other.branchLikely && inline_ == other.inline_; + return branchLikely == other.branchLikely && inline_ == other.inline_ && + removableIfUnused == other.removableIfUnused; } }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index c1fe77ee309..85768b55fbc 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1627,6 +1627,7 @@ std::optional WasmBinaryWriter::writeCodeAnnotations() { append(getBranchHintsBuffer()); append(getInlineHintsBuffer()); + append(getRemovableIfUnusedHintsBuffer()); return ret; } @@ -1769,6 +1770,19 @@ std::optional WasmBinaryWriter::getInlineHintsBuffer() { }); } +std::optional +WasmBinaryWriter::getRemovableIfUnusedHintsBuffer() { + return writeExpressionHints( + Annotations::RemovableIfUnusedHint, + [](const CodeAnnotation& annotation) { + return annotation.removableIfUnused; + }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { + // Hint size, always empty. + buffer << U32LEB(0); + }); +} + void WasmBinaryWriter::writeData(const char* data, size_t size) { for (size_t i = 0; i < size; i++) { o << int8_t(data[i]); @@ -2040,7 +2054,8 @@ void WasmBinaryReader::preScan() { auto sectionName = getInlineString(); if (sectionName == Annotations::BranchHint || - sectionName == Annotations::InlineHint) { + sectionName == Annotations::InlineHint || + sectionName == Annotations::RemovableIfUnusedHint) { // Code annotations require code locations. // TODO: We could note which functions require code locations, as an // optimization. @@ -2197,6 +2212,11 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back(AnnotationSectionInfo{ pos, [this, payloadLen]() { this->readInlineHints(payloadLen); }}); + } else if (sectionName == Annotations::RemovableIfUnusedHint) { + deferredAnnotationSections.push_back( + AnnotationSectionInfo{pos, [this, payloadLen]() { + this->readremovableIfUnusedHints(payloadLen); + }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { @@ -5507,6 +5527,19 @@ void WasmBinaryReader::readInlineHints(size_t payloadLen) { }); } +void WasmBinaryReader::readremovableIfUnusedHints(size_t payloadLen) { + readExpressionHints(Annotations::RemovableIfUnusedHint, + payloadLen, + [&](CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 0) { + throwError("bad removableIfUnusedHint size"); + } + + annotation.removableIfUnused = true; + }); +} + std::tuple WasmBinaryReader::readMemoryAccess(bool isAtomic, bool isRMW) { auto rawAlignment = getU32LEB(); diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 8704359302e..7c59ff7b2e9 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2661,6 +2661,12 @@ void IRBuilder::applyAnnotations(Expression* expr, assert(func); func->codeAnnotations[expr].inline_ = annotation.inline_; } + + if (annotation.removableIfUnused) { + // Only possible inside functions. + assert(func); + func->codeAnnotations[expr].removableIfUnused = true; + } } } // namespace wasm diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index ca20231aabd..4e901c152de 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -69,6 +69,7 @@ namespace Annotations { const Name BranchHint = "metadata.code.branch_hint"; const Name InlineHint = "metadata.code.inline"; +const Name RemovableIfUnusedHint = "binaryen.removable.if.unused"; } // namespace Annotations diff --git a/test/lit/passes/vacuum-removable-if-unused-func.wast b/test/lit/passes/vacuum-removable-if-unused-func.wast new file mode 100644 index 00000000000..8bc34e2ea71 --- /dev/null +++ b/test/lit/passes/vacuum-removable-if-unused-func.wast @@ -0,0 +1,88 @@ +;; RUN: wasm-opt -all --vacuum %s -S -o - | filecheck %s + +;; Test the function-level annotation of removable.if.unused. +(module + (@binaryen.removable.if.unused) + (func $calls-marked (param $x i32) (result i32) + ;; The function is marked as removable if unused, and this is dropped, so optimize. + (drop + (call $calls-marked + (i32.const 0) + ) + ) + ;; Not dropped, so keep it. + (local.set $x + (call $calls-marked + (i32.const 1) + ) + ) + (i32.const 2) + ) + + (func $calls-unmarked (param $x i32) (result i32) + ;; As above, but unmarked with the hint. We change nothing here. + (drop + (call $calls-unmarked + (i32.const 0) + ) + ) + (local.set $x + (call $calls-unmarked + (i32.const 1) + ) + ) + (i32.const 2) + ) + + (func $calls-other (param $x i32) (result i32) + ;; As above, but calling another function, to check we look for annotations in + ;; the right place. Both calls are dropped, and only the one to the marked + ;; function should be removed. + (drop + (call $calls-marked + (i32.const 0) + ) + ) + (drop + (call $calls-unmarked + (i32.const 1) + ) + ) + (i32.const 2) + ) +) + +;; CHECK: (module +;; CHECK-NEXT: (type $0 (func (param i32) (result i32))) +;; CHECK-NEXT: (@binaryen.removable.if.unused) +;; CHECK-NEXT: (func $calls-marked (type $0) (param $x i32) (result i32) +;; CHECK-NEXT: (local.set $x +;; CHECK-NEXT: (call $calls-marked +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i32.const 2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (func $calls-unmarked (type $0) (param $x i32) (result i32) +;; CHECK-NEXT: (drop +;; CHECK-NEXT: (call $calls-unmarked +;; CHECK-NEXT: (i32.const 0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (local.set $x +;; CHECK-NEXT: (call $calls-unmarked +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i32.const 2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (func $calls-other (type $0) (param $x i32) (result i32) +;; CHECK-NEXT: (drop +;; CHECK-NEXT: (call $calls-unmarked +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i32.const 2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) + diff --git a/test/lit/passes/vacuum-removable-if-unused.wast b/test/lit/passes/vacuum-removable-if-unused.wast new file mode 100644 index 00000000000..3d0f147ad69 --- /dev/null +++ b/test/lit/passes/vacuum-removable-if-unused.wast @@ -0,0 +1,89 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --vacuum -all -S -o - | filecheck %s + +(module + ;; CHECK: (func $calls-dropped (type $0) (param $x i32) (result i32) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $calls-dropped + ;; CHECK-NEXT: (i32.const 2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 3) + ;; CHECK-NEXT: ) + (func $calls-dropped (param $x i32) (result i32) + ;; This call is dropped, and marked with the hint, so we can remove it. + (drop + (@binaryen.removable.if.unused) + (call $calls-dropped + (i32.const 0) + ) + ) + ;; As above, but a parameter has effects. We must keep that around, without + ;; the call. + (drop + (@binaryen.removable.if.unused) + (call $calls-dropped + (local.tee $x + (i32.const 1) + ) + ) + ) + ;; Now we lack the hint, so we keep the call. + (drop + (call $calls-dropped + (i32.const 2) + ) + ) + (i32.const 3) + ) + + ;; CHECK: (func $calls-used (type $0) (param $x i32) (result i32) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (@binaryen.removable.if.unused) + ;; CHECK-NEXT: (call $calls-used + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (@binaryen.removable.if.unused) + ;; CHECK-NEXT: (call $calls-used + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (call $calls-used + ;; CHECK-NEXT: (i32.const 2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 3) + ;; CHECK-NEXT: ) + (func $calls-used (param $x i32) (result i32) + ;; As above, but the calls are not dropped, so we keep them. + (local.set $x + (@binaryen.removable.if.unused) + (call $calls-used + (i32.const 0) + ) + ) + (local.set $x + (@binaryen.removable.if.unused) + (call $calls-used + (local.tee $x + (i32.const 1) + ) + ) + ) + (local.set $x + (call $calls-used + (i32.const 2) + ) + ) + (i32.const 3) + ) +) + diff --git a/test/lit/removable-if-unused-func.wast b/test/lit/removable-if-unused-func.wast new file mode 100644 index 00000000000..1500327345a --- /dev/null +++ b/test/lit/removable-if-unused-func.wast @@ -0,0 +1,23 @@ +;; RUN: wasm-opt -all %s -S -o - | filecheck %s +;; RUN: wasm-opt -all --roundtrip %s -S -o - | filecheck %s + +(module + (@binaryen.removable.if.unused) + (func $func-annotation + ;; The annotation here is on the function. + (drop + (i32.const 0) + ) + ) +) + +;; CHECK: (module +;; CHECK-NEXT: (type $0 (func)) +;; CHECK-NEXT: (@binaryen.removable.if.unused) +;; CHECK-NEXT: (func $func-annotation +;; CHECK-NEXT: (drop +;; CHECK-NEXT: (i32.const 0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) + diff --git a/test/lit/removable-if-unused.wast b/test/lit/removable-if-unused.wast new file mode 100644 index 00000000000..9aa3398ae85 --- /dev/null +++ b/test/lit/removable-if-unused.wast @@ -0,0 +1,31 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: wasm-opt -all %s -S -o - | filecheck %s + +;; RUN: wasm-opt -all --roundtrip %s -S -o - | filecheck %s --check-prefix=RTRIP + +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (func $func (type $0) + ;; CHECK-NEXT: (call $func) + ;; CHECK-NEXT: (@binaryen.removable.if.unused) + ;; CHECK-NEXT: (call $func) + ;; CHECK-NEXT: (call $func) + ;; CHECK-NEXT: ) + ;; RTRIP: (type $0 (func)) + + ;; RTRIP: (func $func (type $0) + ;; RTRIP-NEXT: (call $func) + ;; RTRIP-NEXT: (@binaryen.removable.if.unused) + ;; RTRIP-NEXT: (call $func) + ;; RTRIP-NEXT: (call $func) + ;; RTRIP-NEXT: ) + (func $func + ;; Three calls, one annotated in the middle. + (call $func) + (@binaryen.removable.if.unused) + (call $func) + (call $func) + ) +)