From 5daee4c23a8da306780237eb2baaabf5627c003f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:12:58 -0800 Subject: [PATCH 01/35] go --- src/ir/effects.h | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index b9e07a87bf8..ae55b99e728 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -171,6 +171,16 @@ class EffectAnalyzer { // more here.) bool hasReturnCallThrow = false; + // Similar to calls (an effect where anything can happen), but only if this is + // moved. That is, this has no effects where it is presently located, but if + // moved, it will have effects. As a result, this can be removed from its + // current location, but if we want to move it around, we must consider it as + // having the same effects as a call. + // + // Concretely, this does not influence has*Effects(), but it does influence + // invalidates(), which compares effects between things that might move. + bool callsIfMoved = false; + // Helper functions to check for various effect types bool accessesLocal() const { @@ -246,7 +256,8 @@ class EffectAnalyzer { bool hasExternalBreakTargets() const { return !breakTargets.empty(); } // Checks if these effects would invalidate another set of effects (e.g., if - // we write, we invalidate someone that reads). + // we write, we invalidate someone that reads). This checks if we can move one + // set of effects past another, and similar situations. // // This assumes the things whose effects we are comparing will both execute, // at least if neither of them transfers control flow away. That is, we assume @@ -279,16 +290,22 @@ class EffectAnalyzer { // can reorder them even if B traps (even if A has a global effect like a // global.set, since we assume B does not trap in traps-never-happen). bool invalidates(const EffectAnalyzer& other) { + // For purposes of the following comparisons, callsIfMoved is the same as + // calls: We are comparing one set of effects to another, possibly because + // we are moving them across each other, and callsIfMoved has effects if we + // move it. + auto thisCalls = calls || callsIfMoved; + auto otherCalls = other.calls || other.callsIfMoved; if ((transfersControlFlow() && other.hasSideEffects()) || (other.transfersControlFlow() && hasSideEffects()) || - ((writesMemory || calls) && other.accessesMemory()) || - ((other.writesMemory || other.calls) && accessesMemory()) || - ((writesTable || calls) && other.accessesTable()) || - ((other.writesTable || other.calls) && accessesTable()) || - ((writesStruct || calls) && other.accessesMutableStruct()) || - ((other.writesStruct || other.calls) && accessesMutableStruct()) || - ((writesArray || calls) && other.accessesArray()) || - ((other.writesArray || other.calls) && accessesArray()) || + ((writesMemory || thisCalls) && other.accessesMemory()) || + ((other.writesMemory || otherCalls) && accessesMemory()) || + ((writesTable || thisCalls) && other.accessesTable()) || + ((other.writesTable || otherCalls) && accessesTable()) || + ((writesStruct || thisCalls) && other.accessesMutableStruct()) || + ((other.writesStruct || otherCalls) && accessesMutableStruct()) || + ((writesArray || thisCalls) && other.accessesArray()) || + ((other.writesArray || otherCalls) && accessesArray()) || (danglingPop || other.danglingPop)) { return true; } @@ -308,8 +325,8 @@ class EffectAnalyzer { return true; } } - if ((other.calls && accessesMutableGlobal()) || - (calls && other.accessesMutableGlobal())) { + if ((otherCalls && accessesMutableGlobal()) || + (thisCalls && other.accessesMutableGlobal())) { return true; } for (auto global : globalsWritten) { @@ -363,6 +380,8 @@ class EffectAnalyzer { throws_ = throws_ || other.throws_; danglingPop = danglingPop || other.danglingPop; mayNotReturn = mayNotReturn || other.mayNotReturn; + hasReturnCallThrow = hasReturnCallThrow || other.hasReturnCallThrow; + callsIfMoved = callsIfMoved || other.callsIfMoved; for (auto i : other.localsRead) { localsRead.insert(i); } From 7ce31517c0bb1fd0fb0be7882f4da7f504d44197 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:20:53 -0800 Subject: [PATCH 02/35] work --- src/wasm.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wasm.h b/src/wasm.h index ab6323b5f35..92567cbb0c3 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2296,8 +2296,13 @@ class Function : public Importable { static const uint8_t AlwaysInline = 127; std::optional inline_; + // Binaryen intrinsic: Mark as having side effects if moved, but having no + // effects in the current position. See |callsIfMoved| in effects.h. + std::optional effectsIfMoved; + bool operator==(const CodeAnnotation& other) const { - return branchLikely == other.branchLikely && inline_ == other.inline_; + return branchLikely == other.branchLikely && inline_ == other.inline_ && + effectsIfMoved == other.effectsIfMoved; } }; From 477e77b43a66fa632d97d182f53b6988889802d1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:47:10 -0800 Subject: [PATCH 03/35] work --- src/ir/effects.h | 7 +++++++ src/passes/Print.cpp | 6 ++++++ src/wasm-annotations.h | 1 + src/wasm-binary.h | 5 +++++ src/wasm-ir-builder.h | 8 ++++++-- src/wasm.h | 3 ++- src/wasm/wasm-binary.cpp | 36 +++++++++++++++++++++++++++++++++++- src/wasm/wasm-ir-builder.cpp | 13 +++++++++++-- src/wasm/wasm.cpp | 1 + 9 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index ae55b99e728..80bb74004fc 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -179,6 +179,13 @@ class EffectAnalyzer { // // Concretely, this does not influence has*Effects(), but it does influence // invalidates(), which compares effects between things that might move. + // + // This implements effectsIfMoved from wasm.h. The concrete effect we apply is + // a call. (The difference in name is to avoid ambiguity: callsIfMoved in + // wasm.h might suggest that the actual call does not happen - but it does, + // just it is marked as having no effects unless moved. Put another way, we + // implement the side effects from the user-facing spec using a call effect; + // we could call the wasm.h field "callEffectsIfMoved", but prefer brevity.) bool callsIfMoved = false; // Helper functions to check for various effect types diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index e54d3eebdff..526915a159f 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -2787,6 +2787,12 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) { restoreNormalColor(o); doIndent(o, indent); } + if (annotation.effectsIfMoved) { + Colors::grey(o); + o << "(@" << Annotations::EffectsIfMoved << ""\")\n"; + restoreNormalColor(o); + doIndent(o, indent); + } } } diff --git a/src/wasm-annotations.h b/src/wasm-annotations.h index 888c356dcb9..cc4ee5083d9 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 EffectsIfMovedHint; } // namespace wasm::Annotations diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 5a1f618da3d..673ce408cde 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 getEffectsIfMovedHintsBuffer(); // helpers void writeInlineString(std::string_view name); @@ -1732,6 +1733,10 @@ class WasmBinaryReader { size_t inlineHintsLen = 0; void readInlineHints(size_t payloadLen); + size_t callsIfMovedHintsPos = 0; + size_t callsIfMovedHintsLen = 0; + void readCallsIfMovedHints(size_t payloadLen); + std::tuple readMemoryAccess(bool isAtomic, bool isRMW); std::tuple getAtomicMemarg(); diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index bbc21afb083..b35263c20a6 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -138,7 +138,8 @@ class IRBuilder : public UnifiedExpressionVisitor> { // Unlike Builder::makeCall, this assumes the function already exists. Result<> makeCall(Name func, bool isReturn, - std::optional inline_ = std::nullopt); + std::optional inline_ = std::nullopt, + std::optional effectsIfMoved = std::nullopt); Result<> makeCallIndirect(Name table, HeapType type, bool isReturn, @@ -734,9 +735,12 @@ class IRBuilder : public UnifiedExpressionVisitor> { // Add a branch hint, if |likely| is present. void addBranchHint(Expression* expr, std::optional likely); - // Add an inlining hint, if |inline_| is present. + // Add an inlining hint, if present. void addInlineHint(Expression* expr, std::optional inline_); + // Add an effectsIfMoved hint, if present. + void addEffectsIfMovedHint(Expression* expr, std::optional effectsIfMoved); + void dump(); }; diff --git a/src/wasm.h b/src/wasm.h index 92567cbb0c3..3a6f41e7793 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2298,7 +2298,8 @@ class Function : public Importable { // Binaryen intrinsic: Mark as having side effects if moved, but having no // effects in the current position. See |callsIfMoved| in effects.h. - std::optional effectsIfMoved; + // TODO: link to spec + std::optional effectsIfMoved; bool operator==(const CodeAnnotation& other) const { return branchLikely == other.branchLikely && inline_ == other.inline_ && diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 96fd205f908..f7e3ee67a96 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1764,6 +1764,19 @@ std::optional WasmBinaryWriter::getInlineHintsBuffer() { }); } +std::optional WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { + return writeExpressionHints( + Annotations::EffectsIfMovedHint, + [](const Function::CodeAnnotation& annotation) { + return annotation.effectsIfMoved; + }, + [](const Function::CodeAnnotation& annotation, + BufferWithRandomAccess& buffer) { + // Hint size, always 0 for now. + 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]); @@ -2035,7 +2048,8 @@ void WasmBinaryReader::preScan() { auto sectionName = getInlineString(); if (sectionName == Annotations::BranchHint || - sectionName == Annotations::InlineHint) { + sectionName == Annotations::InlineHint || + sectionName == Annotations::EffectsIfMovedHint) { // Code annotations require code locations. // TODO: We could note which functions require code locations, as an // optimization. @@ -2167,6 +2181,10 @@ void WasmBinaryReader::read() { pos = inlineHintsPos; readInlineHints(inlineHintsLen); } + if (effectsIfMovedHintsPos) { + pos = effectsIfMovedHintsPos; + readEffectsIfMovedHints(effectsIfMovedHintsLen); + } validateBinary(); } @@ -2195,6 +2213,9 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::InlineHint) { inlineHintsPos = pos; inlineHintsLen = payloadLen; + } else if (sectionName == Annotations::EffectsIfMovedHint) { + effectsIfMovedHintPos = pos; + effectsIfMovedHintLen = payloadLen; } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { @@ -5484,6 +5505,19 @@ void WasmBinaryReader::readInlineHints(size_t payloadLen) { }); } +void WasmBinaryReader::readEffectsIfMovedHints(size_t payloadLen) { + readExpressionHints(Annotations::EffectsIfMovedHint, + payloadLen, + [&](Function::CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 0) { + throwError("bad EffectsIfMovedHint size"); + } + + annotation.effectsIfMoved.emplace(); + }); +} + 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 57a2469dce0..aa57ae3d154 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1376,7 +1376,8 @@ Result<> IRBuilder::makeSwitch(const std::vector& labels, Result<> IRBuilder::makeCall(Name func, bool isReturn, - std::optional inline_) { + std::optional inline_, + std::optional effectsIfMoved) { auto sig = wasm.getFunction(func)->getSig(); Call curr(wasm.allocator); curr.target = func; @@ -1386,6 +1387,7 @@ Result<> IRBuilder::makeCall(Name func, builder.makeCall(curr.target, curr.operands, sig.results, isReturn); push(call); addInlineHint(call, inline_); + addEffectsIfMovedHint(call, effectsIfMoved); return Ok{}; } @@ -2649,10 +2651,17 @@ void IRBuilder::addBranchHint(Expression* expr, std::optional likely) { void IRBuilder::addInlineHint(Expression* expr, std::optional inline_) { if (inline_) { - // Branches are only possible inside functions. + // Only possible inside functions. assert(func); func->codeAnnotations[expr].inline_ = inline_; } } +void IRBuilder::addEffectsIfMovedHint(Expression* expr, + std::optional effectsIfMoved) { + // Only possible inside functions. + assert(func); + func->codeAnnotations[expr].effectsIfMoved.emplace(); +} + } // namespace wasm diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index 447847c18d7..c3edb53fc98 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 EffectsIfMovedHint = "binaryen.effects_if_moved"; } // namespace Annotations From 83583cb398666f14697be47de33dd2044e79c240 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:51:14 -0800 Subject: [PATCH 04/35] work --- src/parser/contexts.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 1ac3a7c525f..6e1e1c22453 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1330,6 +1330,18 @@ struct AnnotationParserCtx { return value; } + + // Return the inline hint for a call instruction, if there is one. + std::optional + getEffectsIfMovedHint(const std::vector& annotations) { + std::optional hint; + for (auto& a : annotations) { + if (a.kind == Annotations::EffectsIfMovedHint) { + hint.emplace(); + } + } + return hint; + } }; // Phase 4: Parse and set the types of module elements. @@ -2421,7 +2433,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Name func, bool isReturn) { auto inline_ = getInlineHint(annotations); - return withLoc(pos, irBuilder.makeCall(func, isReturn, inline_)); + auto effectsIfMoved = getEffectsIfMovedHint(annotations); + return withLoc(pos, irBuilder.makeCall(func, isReturn, inline_, effectsIfMoved)); } Result<> makeCallIndirect(Index pos, From 073b39e1fef4f8930dfaf12c3f0528e28c12fc1a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:51:23 -0800 Subject: [PATCH 05/35] fmt --- src/parser/contexts.h | 3 ++- src/wasm-ir-builder.h | 12 +++++++----- src/wasm/wasm-binary.cpp | 3 ++- src/wasm/wasm-ir-builder.cpp | 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 6e1e1c22453..cd8e6a4003d 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -2434,7 +2434,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { bool isReturn) { auto inline_ = getInlineHint(annotations); auto effectsIfMoved = getEffectsIfMovedHint(annotations); - return withLoc(pos, irBuilder.makeCall(func, isReturn, inline_, effectsIfMoved)); + return withLoc(pos, + irBuilder.makeCall(func, isReturn, inline_, effectsIfMoved)); } Result<> makeCallIndirect(Index pos, diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index b35263c20a6..acdc1aced95 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -136,10 +136,11 @@ class IRBuilder : public UnifiedExpressionVisitor> { std::optional likely = std::nullopt); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. - Result<> makeCall(Name func, - bool isReturn, - std::optional inline_ = std::nullopt, - std::optional effectsIfMoved = std::nullopt); + Result<> + makeCall(Name func, + bool isReturn, + std::optional inline_ = std::nullopt, + std::optional effectsIfMoved = std::nullopt); Result<> makeCallIndirect(Name table, HeapType type, bool isReturn, @@ -739,7 +740,8 @@ class IRBuilder : public UnifiedExpressionVisitor> { void addInlineHint(Expression* expr, std::optional inline_); // Add an effectsIfMoved hint, if present. - void addEffectsIfMovedHint(Expression* expr, std::optional effectsIfMoved); + void addEffectsIfMovedHint(Expression* expr, + std::optional effectsIfMoved); void dump(); }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index f7e3ee67a96..29cfdf3a88b 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1764,7 +1764,8 @@ std::optional WasmBinaryWriter::getInlineHintsBuffer() { }); } -std::optional WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { +std::optional +WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { return writeExpressionHints( Annotations::EffectsIfMovedHint, [](const Function::CodeAnnotation& annotation) { diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index aa57ae3d154..737db370847 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2658,7 +2658,7 @@ void IRBuilder::addInlineHint(Expression* expr, } void IRBuilder::addEffectsIfMovedHint(Expression* expr, - std::optional effectsIfMoved) { + std::optional effectsIfMoved) { // Only possible inside functions. assert(func); func->codeAnnotations[expr].effectsIfMoved.emplace(); From f129e510f2b1d11a058d39ca8c8e009777de42ba Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:53:19 -0800 Subject: [PATCH 06/35] test --- test/lit/effects-if-moved-hints.wast | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 test/lit/effects-if-moved-hints.wast diff --git a/test/lit/effects-if-moved-hints.wast b/test/lit/effects-if-moved-hints.wast new file mode 100644 index 00000000000..eb14ce9994a --- /dev/null +++ b/test/lit/effects-if-moved-hints.wast @@ -0,0 +1,14 @@ +;; 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 + (func $func + ;; Three calls, one annotated. + (call $func) + (@binaryen.efects.if.moved) + (call $func) + ) +) From 075fd1569a80cbceb369570c48b8ac9e0962580c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:53:50 -0800 Subject: [PATCH 07/35] work --- src/passes/Print.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 526915a159f..c089dedcafb 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -2789,7 +2789,7 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) { } if (annotation.effectsIfMoved) { Colors::grey(o); - o << "(@" << Annotations::EffectsIfMoved << ""\")\n"; + o << "(@" << Annotations::EffectsIfMovedHint << ""\")\n"; restoreNormalColor(o); doIndent(o, indent); } From d540a9643da8df49f36ef7aac2240fe8bd435d9f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 20 Jan 2026 16:54:26 -0800 Subject: [PATCH 08/35] work --- src/passes/Print.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index c089dedcafb..b6bf7eb190d 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -2789,7 +2789,7 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) { } if (annotation.effectsIfMoved) { Colors::grey(o); - o << "(@" << Annotations::EffectsIfMovedHint << ""\")\n"; + o << "(@" << Annotations::EffectsIfMovedHint << ")\n"; restoreNormalColor(o); doIndent(o, indent); } From 4d8b7a512c2798b5a357d4d88e3f4cc03a8ba0d1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 09:45:21 -0800 Subject: [PATCH 09/35] builds --- src/wasm-binary.h | 6 +++--- src/wasm/wasm-binary.cpp | 4 ++-- src/wasm/wasm-ir-builder.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 673ce408cde..449fa3fc09c 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1733,9 +1733,9 @@ class WasmBinaryReader { size_t inlineHintsLen = 0; void readInlineHints(size_t payloadLen); - size_t callsIfMovedHintsPos = 0; - size_t callsIfMovedHintsLen = 0; - void readCallsIfMovedHints(size_t payloadLen); + size_t effectsIfMovedHintsPos = 0; + size_t effectsIfMovedHintsLen = 0; + void readEffectsIfMovedHints(size_t payloadLen); std::tuple readMemoryAccess(bool isAtomic, bool isRMW); diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 29cfdf3a88b..f776d35b153 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2215,8 +2215,8 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { inlineHintsPos = pos; inlineHintsLen = payloadLen; } else if (sectionName == Annotations::EffectsIfMovedHint) { - effectsIfMovedHintPos = pos; - effectsIfMovedHintLen = payloadLen; + effectsIfMovedHintsPos = pos; + effectsIfMovedHintsLen = payloadLen; } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 737db370847..00a791c72f7 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2658,7 +2658,7 @@ void IRBuilder::addInlineHint(Expression* expr, } void IRBuilder::addEffectsIfMovedHint(Expression* expr, - std::optional effectsIfMoved) { + std::optional effectsIfMoved) { // Only possible inside functions. assert(func); func->codeAnnotations[expr].effectsIfMoved.emplace(); From e38e29c39d66997d4837c9c9d8297897fcb1bb80 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:05:30 -0800 Subject: [PATCH 10/35] progress --- src/parser/contexts.h | 1 + src/wasm/wasm-ir-builder.cpp | 8 +++++--- src/wasm/wasm.cpp | 2 +- test/lit/effects-if-moved-hints.wast | 20 ++++++++++++++++++-- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index cd8e6a4003d..3012a7b51e1 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1338,6 +1338,7 @@ struct AnnotationParserCtx { for (auto& a : annotations) { if (a.kind == Annotations::EffectsIfMovedHint) { hint.emplace(); + break; } } return hint; diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 00a791c72f7..735b8b23bf2 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2659,9 +2659,11 @@ void IRBuilder::addInlineHint(Expression* expr, void IRBuilder::addEffectsIfMovedHint(Expression* expr, std::optional effectsIfMoved) { - // Only possible inside functions. - assert(func); - func->codeAnnotations[expr].effectsIfMoved.emplace(); + if (effectsIfMoved) { + // Only possible inside functions. + assert(func); + func->codeAnnotations[expr].effectsIfMoved.emplace(); + } } } // namespace wasm diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index c3edb53fc98..e5b7aea7919 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -69,7 +69,7 @@ namespace Annotations { const Name BranchHint = "metadata.code.branch_hint"; const Name InlineHint = "metadata.code.inline"; -const Name EffectsIfMovedHint = "binaryen.effects_if_moved"; +const Name EffectsIfMovedHint = "binaryen.effects.if.moved"; } // namespace Annotations diff --git a/test/lit/effects-if-moved-hints.wast b/test/lit/effects-if-moved-hints.wast index eb14ce9994a..7b8dae8ac44 100644 --- a/test/lit/effects-if-moved-hints.wast +++ b/test/lit/effects-if-moved-hints.wast @@ -5,10 +5,26 @@ ;; 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.effects.if.moved) + ;; 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: (call $func) + ;; RTRIP-NEXT: (call $func) + ;; RTRIP-NEXT: ) (func $func - ;; Three calls, one annotated. + ;; Three calls, one annotated in the middle. + (call $func) + (@binaryen.effects.if.moved) (call $func) - (@binaryen.efects.if.moved) (call $func) ) ) From c78dc413dc3c995b37f1ac67fee140f964e4ce85 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:07:31 -0800 Subject: [PATCH 11/35] finish --- src/wasm/wasm-binary.cpp | 1 + test/lit/effects-if-moved-hints.wast | 1 + 2 files changed, 2 insertions(+) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index f776d35b153..e6582b307e0 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1623,6 +1623,7 @@ std::optional WasmBinaryWriter::writeCodeAnnotations() { append(getBranchHintsBuffer()); append(getInlineHintsBuffer()); + append(getEffectsIfMovedHintsBuffer()); return ret; } diff --git a/test/lit/effects-if-moved-hints.wast b/test/lit/effects-if-moved-hints.wast index 7b8dae8ac44..eadde670a4b 100644 --- a/test/lit/effects-if-moved-hints.wast +++ b/test/lit/effects-if-moved-hints.wast @@ -17,6 +17,7 @@ ;; RTRIP: (func $func (type $0) ;; RTRIP-NEXT: (call $func) + ;; RTRIP-NEXT: (@binaryen.effects.if.moved) ;; RTRIP-NEXT: (call $func) ;; RTRIP-NEXT: (call $func) ;; RTRIP-NEXT: ) From 8ad0110fa8f9eaf45faee23cc73773c4432cd06d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:07:38 -0800 Subject: [PATCH 12/35] works --- src/wasm/wasm-ir-builder.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 735b8b23bf2..7d013f57070 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2657,8 +2657,8 @@ void IRBuilder::addInlineHint(Expression* expr, } } -void IRBuilder::addEffectsIfMovedHint(Expression* expr, - std::optional effectsIfMoved) { +void IRBuilder::addEffectsIfMovedHint( + Expression* expr, std::optional effectsIfMoved) { if (effectsIfMoved) { // Only possible inside functions. assert(func); From d876266599b58141443738f7c5b8a059e8d062d7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:09:50 -0800 Subject: [PATCH 13/35] UNDO effects.h --- src/ir/effects.h | 48 +++++++++++------------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index 80bb74004fc..b9e07a87bf8 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -171,23 +171,6 @@ class EffectAnalyzer { // more here.) bool hasReturnCallThrow = false; - // Similar to calls (an effect where anything can happen), but only if this is - // moved. That is, this has no effects where it is presently located, but if - // moved, it will have effects. As a result, this can be removed from its - // current location, but if we want to move it around, we must consider it as - // having the same effects as a call. - // - // Concretely, this does not influence has*Effects(), but it does influence - // invalidates(), which compares effects between things that might move. - // - // This implements effectsIfMoved from wasm.h. The concrete effect we apply is - // a call. (The difference in name is to avoid ambiguity: callsIfMoved in - // wasm.h might suggest that the actual call does not happen - but it does, - // just it is marked as having no effects unless moved. Put another way, we - // implement the side effects from the user-facing spec using a call effect; - // we could call the wasm.h field "callEffectsIfMoved", but prefer brevity.) - bool callsIfMoved = false; - // Helper functions to check for various effect types bool accessesLocal() const { @@ -263,8 +246,7 @@ class EffectAnalyzer { bool hasExternalBreakTargets() const { return !breakTargets.empty(); } // Checks if these effects would invalidate another set of effects (e.g., if - // we write, we invalidate someone that reads). This checks if we can move one - // set of effects past another, and similar situations. + // we write, we invalidate someone that reads). // // This assumes the things whose effects we are comparing will both execute, // at least if neither of them transfers control flow away. That is, we assume @@ -297,22 +279,16 @@ class EffectAnalyzer { // can reorder them even if B traps (even if A has a global effect like a // global.set, since we assume B does not trap in traps-never-happen). bool invalidates(const EffectAnalyzer& other) { - // For purposes of the following comparisons, callsIfMoved is the same as - // calls: We are comparing one set of effects to another, possibly because - // we are moving them across each other, and callsIfMoved has effects if we - // move it. - auto thisCalls = calls || callsIfMoved; - auto otherCalls = other.calls || other.callsIfMoved; if ((transfersControlFlow() && other.hasSideEffects()) || (other.transfersControlFlow() && hasSideEffects()) || - ((writesMemory || thisCalls) && other.accessesMemory()) || - ((other.writesMemory || otherCalls) && accessesMemory()) || - ((writesTable || thisCalls) && other.accessesTable()) || - ((other.writesTable || otherCalls) && accessesTable()) || - ((writesStruct || thisCalls) && other.accessesMutableStruct()) || - ((other.writesStruct || otherCalls) && accessesMutableStruct()) || - ((writesArray || thisCalls) && other.accessesArray()) || - ((other.writesArray || otherCalls) && accessesArray()) || + ((writesMemory || calls) && other.accessesMemory()) || + ((other.writesMemory || other.calls) && accessesMemory()) || + ((writesTable || calls) && other.accessesTable()) || + ((other.writesTable || other.calls) && accessesTable()) || + ((writesStruct || calls) && other.accessesMutableStruct()) || + ((other.writesStruct || other.calls) && accessesMutableStruct()) || + ((writesArray || calls) && other.accessesArray()) || + ((other.writesArray || other.calls) && accessesArray()) || (danglingPop || other.danglingPop)) { return true; } @@ -332,8 +308,8 @@ class EffectAnalyzer { return true; } } - if ((otherCalls && accessesMutableGlobal()) || - (thisCalls && other.accessesMutableGlobal())) { + if ((other.calls && accessesMutableGlobal()) || + (calls && other.accessesMutableGlobal())) { return true; } for (auto global : globalsWritten) { @@ -387,8 +363,6 @@ class EffectAnalyzer { throws_ = throws_ || other.throws_; danglingPop = danglingPop || other.danglingPop; mayNotReturn = mayNotReturn || other.mayNotReturn; - hasReturnCallThrow = hasReturnCallThrow || other.hasReturnCallThrow; - callsIfMoved = callsIfMoved || other.callsIfMoved; for (auto i : other.localsRead) { localsRead.insert(i); } From 31f6e5c2703a3cae5b52be53d9fc52395a482a09 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:18:38 -0800 Subject: [PATCH 14/35] yolo --- src/passes/Vacuum.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index b5e223d9f21..f648f0ad26e 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -92,9 +92,7 @@ struct Vacuum : public WalkerPass> { return curr; } // Check if this expression itself has side effects, ignoring children. - EffectAnalyzer self(getPassOptions(), *getModule()); - self.visit(curr); - if (self.hasUnremovableSideEffects()) { + if (hasShallowEffectsAssumingResultUnused(curr)) { return curr; } // The result isn't used, and this has no side effects itself, so we can @@ -130,6 +128,25 @@ struct Vacuum : public WalkerPass> { } } + // Given an expression whose result is unused, see if it has effects (ignoring + // children, so only a shallow check). This is basically just a call to + // ShallowEffectAnalyzer, except that the knowledge of the result being unused + // lets us check the relevant hint. + bool hasShallowEffectsAssumingResultUnused(Expression* curr) { // canberemoved, to justifiy hasUnremovable + if (auto* call = curr->dynCast()) { + auto iter = getFunction()->codeAnnotations.find(call); + if iter != currFunction->codeAnnotations.end()) { + auto& annotation = iter->second; + if (annotation.effectsIfMoved) { + // No need to check effects, this can be removed. + return false; + } + } + } + ShallowEffectAnalyzer self(getPassOptions(), *getModule(), curr); + return self.hasUnremovableSideEffects(); + } + void visitBlock(Block* curr) { auto& list = curr->list; From 879dc79852a6446c39c2e69d9b6afdc7ce3d7267 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:27:18 -0800 Subject: [PATCH 15/35] works --- src/passes/Vacuum.cpp | 5 +- test/lit/passes/vacuum-effects-if-moved.wast | 89 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 test/lit/passes/vacuum-effects-if-moved.wast diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index f648f0ad26e..f6aae6b6cb8 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -134,8 +134,9 @@ struct Vacuum : public WalkerPass> { // lets us check the relevant hint. bool hasShallowEffectsAssumingResultUnused(Expression* curr) { // canberemoved, to justifiy hasUnremovable if (auto* call = curr->dynCast()) { - auto iter = getFunction()->codeAnnotations.find(call); - if iter != currFunction->codeAnnotations.end()) { + auto& annotations = getFunction()->codeAnnotations; + auto iter = annotations.find(call); + if (iter != annotations.end()) { auto& annotation = iter->second; if (annotation.effectsIfMoved) { // No need to check effects, this can be removed. diff --git a/test/lit/passes/vacuum-effects-if-moved.wast b/test/lit/passes/vacuum-effects-if-moved.wast new file mode 100644 index 00000000000..19766013c2d --- /dev/null +++ b/test/lit/passes/vacuum-effects-if-moved.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.effects.if.moved) + (call $calls-dropped + (i32.const 0) + ) + ) + ;; As above, but a parameter has effects. We must keep that around, without + ;; the call. + (drop + (@binaryen.effects.if.moved) + (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.effects.if.moved) + ;; CHECK-NEXT: (call $calls-used + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (@binaryen.effects.if.moved) + ;; CHECK-NEXT: (call $calls-used + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; 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.effects.if.moved) + (call $calls-used + (i32.const 0) + ) + ) + (local.set $x + (@binaryen.effects.if.moved) + (call $calls-used + (local.tee $x + (i32.const 1) + ) + ) + ) + (drop + (call $calls-used + (i32.const 2) + ) + ) + (i32.const 3) + ) +) + From eadd3dc6c1273a0cc145ec3bdd1dd5eb2e22ef33 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:29:14 -0800 Subject: [PATCH 16/35] rename --- src/passes/Vacuum.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index f6aae6b6cb8..dcdd48de68a 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -91,8 +91,8 @@ struct Vacuum : public WalkerPass> { curr->is() || curr->is() || curr->is()) { return curr; } - // Check if this expression itself has side effects, ignoring children. - if (hasShallowEffectsAssumingResultUnused(curr)) { + // 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 @@ -128,11 +128,11 @@ struct Vacuum : public WalkerPass> { } } - // Given an expression whose result is unused, see if it has effects (ignoring - // children, so only a shallow check). This is basically just a call to - // ShallowEffectAnalyzer, except that the knowledge of the result being unused - // lets us check the relevant hint. - bool hasShallowEffectsAssumingResultUnused(Expression* curr) { // canberemoved, to justifiy hasUnremovable + // 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. + bool mustKeepUnusedParent(Expression* curr) { if (auto* call = curr->dynCast()) { auto& annotations = getFunction()->codeAnnotations; auto iter = annotations.find(call); From a49fe2643d687596159564ed10f08dfd08ef7df1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Jan 2026 10:29:42 -0800 Subject: [PATCH 17/35] rename --- src/passes/Vacuum.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index dcdd48de68a..7d29d1d3ee9 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -131,7 +131,7 @@ 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. + // result is unused, the relevant hint may help us. bool mustKeepUnusedParent(Expression* curr) { if (auto* call = curr->dynCast()) { auto& annotations = getFunction()->codeAnnotations; From dce83d867d84f86edcabc386332f05f3971de795 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 12:13:29 -0800 Subject: [PATCH 18/35] refactor --- src/parser/contexts.h | 82 ++++++++++++++++-------------------- src/wasm-ir-builder.h | 27 ++++-------- src/wasm.h | 46 ++++++++++---------- src/wasm/wasm-ir-builder.cpp | 50 ++++++++++------------ 4 files changed, 92 insertions(+), 113 deletions(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 69818043bef..510d7b46185 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1297,52 +1297,45 @@ struct ParseImplicitTypeDefsCtx : TypeParserCtx { }; struct AnnotationParserCtx { - // Return the inline hint for a call instruction, if there is one. - std::optional - getInlineHint(const std::vector& annotations) { - // Find and apply (the last) inline hint. - const Annotation* hint = nullptr; + // Parse annotations into IR. + CodeAnnotation parseAnnotations(const std::vector& annotations) { + CodeAnnotation ret; + + // Find the hints. For hints with content we must find the last one, which + // overrides the others. + const Annotation* inlineHint = nullptr; for (auto& a : annotations) { if (a.kind == Annotations::InlineHint) { - hint = &a; + inlineHint = &a; + } else if (a.kind == Annotations::EffectsIfMovedHint) { + ret.effectsIfMovedHint.emplace(); } } - if (!hint) { - return std::nullopt; - } - - Lexer lexer(hint->contents); - if (lexer.empty()) { - std::cerr << "warning: empty InlineHint\n"; - return std::nullopt; - } - - auto str = lexer.takeString(); - if (!str || str->size() != 1) { - std::cerr << "warning: invalid InlineHint string\n"; - return std::nullopt; - } - uint8_t value = (*str)[0]; - if (value > 127) { - std::cerr << "warning: invalid InlineHint value\n"; - return std::nullopt; - } + // Apply the last inline hint, if any. + if (inlineHint) { + Lexer lexer(inlineHint->contents); + if (lexer.empty()) { + std::cerr << "warning: empty InlineHint\n"; + return std::nullopt; + } - return value; - } + auto str = lexer.takeString(); + if (!str || str->size() != 1) { + std::cerr << "warning: invalid InlineHint string\n"; + return std::nullopt; + } - // Return the inline hint for a call instruction, if there is one. - std::optional - getEffectsIfMovedHint(const std::vector& annotations) { - std::optional hint; - for (auto& a : annotations) { - if (a.kind == Annotations::EffectsIfMovedHint) { - hint.emplace(); - break; + uint8_t value = (*str)[0]; + if (value > 127) { + std::cerr << "warning: invalid InlineHint value\n"; + return std::nullopt; } + + ret.inline_ = value; } - return hint; + + return ret; } }; @@ -2435,10 +2428,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { const std::vector& annotations, Name func, bool isReturn) { - auto inline_ = getInlineHint(annotations); - auto effectsIfMoved = getEffectsIfMovedHint(annotations); - return withLoc(pos, - irBuilder.makeCall(func, isReturn, inline_, effectsIfMoved)); + return withLoc( + pos, irBuilder.makeCall(func, isReturn, parseAnnotations(annotations))); } Result<> makeCallIndirect(Index pos, @@ -2448,9 +2439,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { bool isReturn) { auto t = getTable(pos, table); CHECK_ERR(t); - auto inline_ = getInlineHint(annotations); return withLoc(pos, - irBuilder.makeCallIndirect(*t, type, isReturn, inline_)); + irBuilder.makeCallIndirect( + *t, type, isReturn, parseAnnotations(annotations))); } // Return the branch hint for a branching instruction, if there is one. @@ -2632,8 +2623,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { const std::vector& annotations, HeapType type, bool isReturn) { - auto inline_ = getInlineHint(annotations); - return withLoc(pos, irBuilder.makeCallRef(type, isReturn, inline_)); + return withLoc( + pos, + irBuilder.makeCallRef(type, isReturn, parseAnnotations(annotations))); } Result<> makeRefI31(Index pos, diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index c6f5254ded9..952d22a7257 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -136,15 +136,14 @@ class IRBuilder : public UnifiedExpressionVisitor> { std::optional likely = std::nullopt); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. + Result<> makeCall(Name func, + bool isReturn, + const CodeAnnotations& annotations = CodeAnnotations()); Result<> - makeCall(Name func, - bool isReturn, - std::optional inline_ = std::nullopt, - std::optional effectsIfMoved = std::nullopt); - Result<> makeCallIndirect(Name table, - HeapType type, - bool isReturn, - std::optional inline_ = std::nullopt); + makeCallIndirect(Name table, + HeapType type, + bool isReturn, + const CodeAnnotations& annotations = CodeAnnotations()); Result<> makeLocalGet(Index local); Result<> makeLocalSet(Index local); Result<> makeLocalTee(Index local); @@ -228,7 +227,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeI31Get(bool signed_); Result<> makeCallRef(HeapType type, bool isReturn, - std::optional inline_ = std::nullopt); + const CodeAnnotations& annotations = CodeAnnotations()); Result<> makeRefTest(Type type); Result<> makeRefCast(Type type, bool isDesc); Result<> makeRefGetDesc(HeapType type); @@ -737,15 +736,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Expression* fixExtraOutput(ScopeCtx& scope, Name label, Expression* expr); void fixLoopWithInput(Loop* loop, Type inputType, Index scratch); - // Add a branch hint, if |likely| is present. - void addBranchHint(Expression* expr, std::optional likely); - - // Add an inlining hint, if present. - void addInlineHint(Expression* expr, std::optional inline_); - - // Add an effectsIfMoved hint, if present. - void addEffectsIfMovedHint(Expression* expr, - std::optional effectsIfMoved); + void applyAnnotations(Expression* expr, const CodeAnnotations& annotations); void dump(); }; diff --git a/src/wasm.h b/src/wasm.h index d2456187c12..c239ab1e737 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2232,6 +2232,27 @@ struct BinaryLocations { // Forward declaration for FuncEffectsMap. class EffectAnalyzer; +// Code annotations for VMs. +struct CodeAnnotation { + // Branch Hinting proposal: Whether the branch is likely, or unlikely. + std::optional branchLikely; + + // Compilation Hints proposal. + static const uint8_t NeverInline = 0; + static const uint8_t AlwaysInline = 127; + std::optional inline_; + + // Binaryen intrinsic: Mark as having side effects if moved, but having no + // effects in the current position. See |callsIfMoved| in effects.h. + // TODO: link to spec + std::optional effectsIfMoved; + + bool operator==(const CodeAnnotation& other) const { + return branchLikely == other.branchLikely && inline_ == other.inline_ && + effectsIfMoved == other.effectsIfMoved; + } +}; + class Function : public Importable { public: // A non-nullable reference to a function type. Exact for defined functions. @@ -2285,31 +2306,10 @@ class Function : public Importable { delimiterLocations; BinaryLocations::FunctionLocations funcLocation; - // Code annotations for VMs. As with debug info, we do not store these on + // Function-level annotations are implemented with a key of nullptr, matching + // the 0 byte offset in the spec. As with debug info, we do not store these on // Expressions as we assume most instances are unannotated, and do not want to // add constant memory overhead. - struct CodeAnnotation { - // Branch Hinting proposal: Whether the branch is likely, or unlikely. - std::optional branchLikely; - - // Compilation Hints proposal. - static const uint8_t NeverInline = 0; - static const uint8_t AlwaysInline = 127; - std::optional inline_; - - // Binaryen intrinsic: Mark as having side effects if moved, but having no - // effects in the current position. See |callsIfMoved| in effects.h. - // TODO: link to spec - std::optional effectsIfMoved; - - bool operator==(const CodeAnnotation& other) const { - return branchLikely == other.branchLikely && inline_ == other.inline_ && - effectsIfMoved == other.effectsIfMoved; - } - }; - - // Function-level annotations are implemented with a key of nullptr, matching - // the 0 byte offset in the spec. std::unordered_map codeAnnotations; // The effects for this function, if they have been computed. We use a shared diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index a801d71db97..9c8af4ac3d4 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1374,10 +1374,10 @@ Result<> IRBuilder::makeSwitch(const std::vector& labels, return Ok{}; } -Result<> IRBuilder::makeCall(Name func, - bool isReturn, - std::optional inline_, - std::optional effectsIfMoved) { +Result<> +IRBuilder::makeCall(Name func, + bool isReturn, + const CodeAnnotations& annotations = CodeAnnotations()) { auto sig = wasm.getFunction(func)->getSig(); Call curr(wasm.allocator); curr.target = func; @@ -1386,15 +1386,15 @@ Result<> IRBuilder::makeCall(Name func, auto* call = builder.makeCall(curr.target, curr.operands, sig.results, isReturn); push(call); - addInlineHint(call, inline_); - addEffectsIfMovedHint(call, effectsIfMoved); + applyAnnotations(call, annotations); return Ok{}; } -Result<> IRBuilder::makeCallIndirect(Name table, - HeapType type, - bool isReturn, - std::optional inline_) { +Result<> IRBuilder::makeCallIndirect( + Name table, + HeapType type, + bool isReturn, + const CodeAnnotations& annotations = CodeAnnotations()) { if (!type.isSignature()) { return Err{"expected function type annotation on call_indirect"}; } @@ -1405,7 +1405,7 @@ Result<> IRBuilder::makeCallIndirect(Name table, auto* call = builder.makeCallIndirect(table, curr.target, curr.operands, type, isReturn); push(call); - addInlineHint(call, inline_); + applyAnnotations(call, annotations); return Ok{}; } @@ -1917,9 +1917,10 @@ Result<> IRBuilder::makeI31Get(bool signed_) { return Ok{}; } -Result<> IRBuilder::makeCallRef(HeapType type, - bool isReturn, - std::optional inline_) { +Result<> +IRBuilder::makeCallRef(HeapType type, + bool isReturn, + const CodeAnnotations& annotations = CodeAnnotations()) { if (!type.isSignature()) { return Err{"expected function type annotation on call_ref"}; } @@ -1934,7 +1935,7 @@ Result<> IRBuilder::makeCallRef(HeapType type, auto* call = builder.makeCallRef(curr.target, curr.operands, sig.results, isReturn); push(call); - addInlineHint(call, inline_); + applyAnnotations(call, annotations); return Ok{}; } @@ -2644,26 +2645,21 @@ Result<> IRBuilder::makeStackSwitch(HeapType ct, Name tag) { return Ok{}; } -void IRBuilder::addBranchHint(Expression* expr, std::optional likely) { - if (likely) { +void IRBuilder::applyAnnotations(Expression* expr, + const CodeAnnotations& annotations) { + if (annotations.branchLikely) { // Branches are only possible inside functions. assert(func); - func->codeAnnotations[expr].branchLikely = likely; + func->codeAnnotations[expr].branchLikely = annotations.branchLikely; } -} -void IRBuilder::addInlineHint(Expression* expr, - std::optional inline_) { - if (inline_) { + if (annotations.inline_) { // Only possible inside functions. assert(func); - func->codeAnnotations[expr].inline_ = inline_; + func->codeAnnotations[expr].inline_ = annotations.inline_; } -} -void IRBuilder::addEffectsIfMovedHint( - Expression* expr, std::optional effectsIfMoved) { - if (effectsIfMoved) { + if (annotations.effectsIfMoved) { // Only possible inside functions. assert(func); func->codeAnnotations[expr].effectsIfMoved.emplace(); From 93234e2f8fcfb6cbe7bbca9040875fb30c059553 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 12:49:53 -0800 Subject: [PATCH 19/35] fix --- src/wasm-ir-builder.h | 8 ++++---- src/wasm/wasm-ir-builder.cpp | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 952d22a7257..479ce58338d 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -138,12 +138,12 @@ class IRBuilder : public UnifiedExpressionVisitor> { // Unlike Builder::makeCall, this assumes the function already exists. Result<> makeCall(Name func, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()); + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeCallIndirect(Name table, HeapType type, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()); + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeLocalGet(Index local); Result<> makeLocalSet(Index local); Result<> makeLocalTee(Index local); @@ -227,7 +227,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeI31Get(bool signed_); Result<> makeCallRef(HeapType type, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()); + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeRefTest(Type type); Result<> makeRefCast(Type type, bool isDesc); Result<> makeRefGetDesc(HeapType type); @@ -736,7 +736,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Expression* fixExtraOutput(ScopeCtx& scope, Name label, Expression* expr); void fixLoopWithInput(Loop* loop, Type inputType, Index scratch); - void applyAnnotations(Expression* expr, const CodeAnnotations& annotations); + void applyAnnotations(Expression* expr, const CodeAnnotation& annotation); void dump(); }; diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 9c8af4ac3d4..5b6d2864864 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1377,7 +1377,7 @@ Result<> IRBuilder::makeSwitch(const std::vector& labels, Result<> IRBuilder::makeCall(Name func, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()) { + const CodeAnnotation& annotations) { auto sig = wasm.getFunction(func)->getSig(); Call curr(wasm.allocator); curr.target = func; @@ -1394,7 +1394,7 @@ Result<> IRBuilder::makeCallIndirect( Name table, HeapType type, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()) { + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_indirect"}; } @@ -1920,7 +1920,7 @@ Result<> IRBuilder::makeI31Get(bool signed_) { Result<> IRBuilder::makeCallRef(HeapType type, bool isReturn, - const CodeAnnotations& annotations = CodeAnnotations()) { + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_ref"}; } @@ -2646,20 +2646,20 @@ Result<> IRBuilder::makeStackSwitch(HeapType ct, Name tag) { } void IRBuilder::applyAnnotations(Expression* expr, - const CodeAnnotations& annotations) { - if (annotations.branchLikely) { + const CodeAnnotation& annotation) { + if (annotation.branchLikely) { // Branches are only possible inside functions. assert(func); - func->codeAnnotations[expr].branchLikely = annotations.branchLikely; + func->codeAnnotations[expr].branchLikely = annotation.branchLikely; } - if (annotations.inline_) { + if (annotation.inline_) { // Only possible inside functions. assert(func); - func->codeAnnotations[expr].inline_ = annotations.inline_; + func->codeAnnotations[expr].inline_ = annotation.inline_; } - if (annotations.effectsIfMoved) { + if (annotation.effectsIfMoved) { // Only possible inside functions. assert(func); func->codeAnnotations[expr].effectsIfMoved.emplace(); From eeba29786e005bba73a7b719b5f11b3f11495ee4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 12:50:02 -0800 Subject: [PATCH 20/35] fmrt --- src/wasm/wasm-ir-builder.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 5b6d2864864..567ef67290c 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1374,10 +1374,9 @@ Result<> IRBuilder::makeSwitch(const std::vector& labels, return Ok{}; } -Result<> -IRBuilder::makeCall(Name func, - bool isReturn, - const CodeAnnotation& annotations) { +Result<> IRBuilder::makeCall(Name func, + bool isReturn, + const CodeAnnotation& annotations) { auto sig = wasm.getFunction(func)->getSig(); Call curr(wasm.allocator); curr.target = func; @@ -1390,11 +1389,10 @@ IRBuilder::makeCall(Name func, return Ok{}; } -Result<> IRBuilder::makeCallIndirect( - Name table, - HeapType type, - bool isReturn, - const CodeAnnotation& annotations) { +Result<> IRBuilder::makeCallIndirect(Name table, + HeapType type, + bool isReturn, + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_indirect"}; } @@ -1917,10 +1915,9 @@ Result<> IRBuilder::makeI31Get(bool signed_) { return Ok{}; } -Result<> -IRBuilder::makeCallRef(HeapType type, - bool isReturn, - const CodeAnnotation& annotations) { +Result<> IRBuilder::makeCallRef(HeapType type, + bool isReturn, + const CodeAnnotation& annotations) { if (!type.isSignature()) { return Err{"expected function type annotation on call_ref"}; } From 902a233ddb31bffd116906de094fa38e23f450c4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 16:45:40 -0800 Subject: [PATCH 21/35] work --- src/ir/metadata.cpp | 2 +- src/parser/contexts.h | 9 +++++---- src/wasm-ir-builder.h | 6 +++--- src/wasm/wasm-ir-builder.cpp | 14 +++++++------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/ir/metadata.cpp b/src/ir/metadata.cpp index f69927ecc10..61c5da9bc06 100644 --- a/src/ir/metadata.cpp +++ b/src/ir/metadata.cpp @@ -130,7 +130,7 @@ bool equal(Function* a, Function* b) { bList.list[i], a->codeAnnotations, b->codeAnnotations, - Function::CodeAnnotation())) { + CodeAnnotation())) { return false; } } diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 510d7b46185..82327d2c3c3 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1298,6 +1298,7 @@ struct ParseImplicitTypeDefsCtx : TypeParserCtx { struct AnnotationParserCtx { // Parse annotations into IR. + // TODO add branch hints CodeAnnotation parseAnnotations(const std::vector& annotations) { CodeAnnotation ret; @@ -1308,7 +1309,7 @@ struct AnnotationParserCtx { if (a.kind == Annotations::InlineHint) { inlineHint = &a; } else if (a.kind == Annotations::EffectsIfMovedHint) { - ret.effectsIfMovedHint.emplace(); + ret.effectsIfMoved.emplace(); } } @@ -1317,19 +1318,19 @@ struct AnnotationParserCtx { Lexer lexer(inlineHint->contents); if (lexer.empty()) { std::cerr << "warning: empty InlineHint\n"; - return std::nullopt; + return ret; } auto str = lexer.takeString(); if (!str || str->size() != 1) { std::cerr << "warning: invalid InlineHint string\n"; - return std::nullopt; + return ret; } uint8_t value = (*str)[0]; if (value > 127) { std::cerr << "warning: invalid InlineHint value\n"; - return std::nullopt; + return ret; } ret.inline_ = value; diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 479ce58338d..b270f746506 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -129,11 +129,11 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeNop(); Result<> makeBlock(Name label, Signature sig); Result<> - makeIf(Name label, Signature sig, std::optional likely = std::nullopt); + makeIf(Name label, Signature sig, const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeLoop(Name label, Signature sig); Result<> makeBreak(Index label, bool isConditional, - std::optional likely = std::nullopt); + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. Result<> makeCall(Name func, @@ -235,7 +235,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { BrOnOp op, Type in = Type::none, Type out = Type::none, - std::optional likely = std::nullopt); + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeStructNew(HeapType type, bool isDesc); Result<> makeStructNewDefault(HeapType type, bool isDesc); Result<> diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 567ef67290c..30fc2ae93a2 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1314,10 +1314,10 @@ Result<> IRBuilder::makeBlock(Name label, Signature sig) { } Result<> -IRBuilder::makeIf(Name label, Signature sig, std::optional likely) { +IRBuilder::makeIf(Name label, Signature sig, const CodeAnnotation& annotations) { auto* iff = wasm.allocator.alloc(); iff->type = sig.results; - addBranchHint(iff, likely); + applyAnnotations(annotations); return visitIfStart(iff, label, sig.params); } @@ -1330,7 +1330,7 @@ Result<> IRBuilder::makeLoop(Name label, Signature sig) { Result<> IRBuilder::makeBreak(Index label, bool isConditional, - std::optional likely) { + const CodeAnnotation& annotations) { auto name = getLabelName(label); CHECK_ERR(name); auto labelType = getLabelType(label); @@ -1342,7 +1342,7 @@ Result<> IRBuilder::makeBreak(Index label, curr.condition = isConditional ? &curr : nullptr; CHECK_ERR(ChildPopper{*this}.visitBreak(&curr, *labelType)); auto* br = builder.makeBreak(curr.name, curr.value, curr.condition); - addBranchHint(br, likely); + applyAnnotations(annotations); push(br); return Ok{}; @@ -1981,7 +1981,7 @@ Result<> IRBuilder::makeRefGetDesc(HeapType type) { } Result<> IRBuilder::makeBrOn( - Index label, BrOnOp op, Type in, Type out, std::optional likely) { + Index label, BrOnOp op, Type in, Type out, const CodeAnnotation& annotations) { std::optional descriptor; if (op == BrOnCastDesc || op == BrOnCastDescFail) { assert(out.isRef()); @@ -2062,7 +2062,7 @@ Result<> IRBuilder::makeBrOn( CHECK_ERR(name); auto* br = builder.makeBrOn(op, *name, curr.ref, out, curr.desc); - addBranchHint(br, likely); + applyAnnotations(annotations); push(br); return Ok{}; } @@ -2086,7 +2086,7 @@ Result<> IRBuilder::makeBrOn( // Perform the branch. CHECK_ERR(visitBrOn(&curr)); auto* br = builder.makeBrOn(op, extraLabel, curr.ref, out, curr.desc); - addBranchHint(br, likely); + applyAnnotations(annotations); push(br); // If the branch wasn't taken, we need to leave the extra values on the From 2e287a2c27c64fa59b83f5fcf759661884920730 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 16:51:50 -0800 Subject: [PATCH 22/35] work --- src/parser/contexts.h | 99 +++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 59 deletions(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 82327d2c3c3..48c36394a1a 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1298,42 +1298,61 @@ struct ParseImplicitTypeDefsCtx : TypeParserCtx { struct AnnotationParserCtx { // Parse annotations into IR. - // TODO add branch hints CodeAnnotation parseAnnotations(const std::vector& annotations) { CodeAnnotation ret; // Find the hints. For hints with content we must find the last one, which // overrides the others. + const Annotation* branchHint = nullptr; const Annotation* inlineHint = nullptr; for (auto& a : annotations) { - if (a.kind == Annotations::InlineHint) { + if (a.kind == Annotations::BranchHint) { + branchHint = &a; + } else if (a.kind == Annotations::InlineHint) { inlineHint = &a; } else if (a.kind == Annotations::EffectsIfMovedHint) { ret.effectsIfMoved.emplace(); } } - // Apply the last inline hint, if any. + // Apply the last branch hint, if valid. + if (branchHint) { + Lexer lexer(branchHint->contents); + if (lexer.empty()) { + std::cerr << "warning: empty BranchHint\n"; + } else { + auto str = lexer.takeString(); + if (!str || str->size() != 1) { + std::cerr << "warning: invalid BranchHint string\n"; + } else { + auto value = (*str)[0]; + if (value != 0 && value != 1) { + std::cerr << "warning: invalid BranchHint value\n"; + } else { + return ret.branchLikely = bool(value); + } + } + } + } + + // Apply the last inline hint, if valid. if (inlineHint) { Lexer lexer(inlineHint->contents); if (lexer.empty()) { std::cerr << "warning: empty InlineHint\n"; - return ret; - } - - auto str = lexer.takeString(); - if (!str || str->size() != 1) { - std::cerr << "warning: invalid InlineHint string\n"; - return ret; - } - - uint8_t value = (*str)[0]; - if (value > 127) { - std::cerr << "warning: invalid InlineHint value\n"; - return ret; + } else { + auto str = lexer.takeString(); + if (!str || str->size() != 1) { + std::cerr << "warning: invalid InlineHint string\n"; + } else { + uint8_t value = (*str)[0]; + if (value > 127) { + std::cerr << "warning: invalid InlineHint value\n"; + } else { + ret.inline_ = value; + } + } } - - ret.inline_ = value; } return ret; @@ -2007,10 +2026,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { if (!type.isSignature()) { return in.err(pos, "expected function type"); } - auto likely = getBranchHint(annotations); return withLoc( pos, - irBuilder.makeIf(label ? *label : Name{}, type.getSignature(), likely)); + irBuilder.makeIf(label ? *label : Name{}, type.getSignature(), parseAnnotations(annotations))); } Result<> visitElse() { return withLoc(irBuilder.visitElse()); } @@ -2445,47 +2463,11 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { *t, type, isReturn, parseAnnotations(annotations))); } - // Return the branch hint for a branching instruction, if there is one. - std::optional - getBranchHint(const std::vector& annotations) { - // Find and apply (the last) branch hint. - const Annotation* hint = nullptr; - for (auto& a : annotations) { - if (a.kind == Annotations::BranchHint) { - hint = &a; - } - } - if (!hint) { - return std::nullopt; - } - - Lexer lexer(hint->contents); - if (lexer.empty()) { - std::cerr << "warning: empty BranchHint\n"; - return std::nullopt; - } - - auto str = lexer.takeString(); - if (!str || str->size() != 1) { - std::cerr << "warning: invalid BranchHint string\n"; - return std::nullopt; - } - - auto value = (*str)[0]; - if (value != 0 && value != 1) { - std::cerr << "warning: invalid BranchHint value\n"; - return std::nullopt; - } - - return bool(value); - } - Result<> makeBreak(Index pos, const std::vector& annotations, Index label, bool isConditional) { - auto likely = getBranchHint(annotations); - return withLoc(pos, irBuilder.makeBreak(label, isConditional, likely)); + return withLoc(pos, irBuilder.makeBreak(label, isConditional, parseAnnotations(annotations))); } Result<> makeSwitch(Index pos, @@ -2666,8 +2648,7 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { BrOnOp op, Type in = Type::none, Type out = Type::none) { - auto likely = getBranchHint(annotations); - return withLoc(pos, irBuilder.makeBrOn(label, op, in, out, likely)); + return withLoc(pos, irBuilder.makeBrOn(label, op, in, out, parseAnnotations(annotations))); } Result<> makeStructNew(Index pos, From 45650e17de968245a6d8cef4ce4da03d08e7ce04 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 16:52:25 -0800 Subject: [PATCH 23/35] work --- src/parser/contexts.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 48c36394a1a..30fb29429d9 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1329,7 +1329,7 @@ struct AnnotationParserCtx { if (value != 0 && value != 1) { std::cerr << "warning: invalid BranchHint value\n"; } else { - return ret.branchLikely = bool(value); + ret.branchLikely = bool(value); } } } From 017f8e5689049d101e2c82731b56ece964c4ea1e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 16:53:24 -0800 Subject: [PATCH 24/35] work --- src/wasm/wasm-ir-builder.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index 30fc2ae93a2..a610642ae61 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1317,7 +1317,7 @@ Result<> IRBuilder::makeIf(Name label, Signature sig, const CodeAnnotation& annotations) { auto* iff = wasm.allocator.alloc(); iff->type = sig.results; - applyAnnotations(annotations); + applyAnnotations(iff, annotations); return visitIfStart(iff, label, sig.params); } @@ -1342,7 +1342,7 @@ Result<> IRBuilder::makeBreak(Index label, curr.condition = isConditional ? &curr : nullptr; CHECK_ERR(ChildPopper{*this}.visitBreak(&curr, *labelType)); auto* br = builder.makeBreak(curr.name, curr.value, curr.condition); - applyAnnotations(annotations); + applyAnnotations(br, annotations); push(br); return Ok{}; @@ -2062,7 +2062,7 @@ Result<> IRBuilder::makeBrOn( CHECK_ERR(name); auto* br = builder.makeBrOn(op, *name, curr.ref, out, curr.desc); - applyAnnotations(annotations); + applyAnnotations(br, annotations); push(br); return Ok{}; } @@ -2086,7 +2086,7 @@ Result<> IRBuilder::makeBrOn( // Perform the branch. CHECK_ERR(visitBrOn(&curr)); auto* br = builder.makeBrOn(op, extraLabel, curr.ref, out, curr.desc); - applyAnnotations(annotations); + applyAnnotations(br, annotations); push(br); // If the branch wasn't taken, we need to leave the extra values on the From 4097eeceab67d859ec5cad5cd60ffd00dcecffb2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 16:54:45 -0800 Subject: [PATCH 25/35] work --- src/wasm/wasm-binary.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 6f0c3190c0e..2c898cec6f5 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1635,7 +1635,7 @@ std::optional WasmBinaryWriter::writeExpressionHints( Expression* expr; // The offset we will write in the custom section. BinaryLocation offset; - Function::CodeAnnotation* hint; + CodeAnnotation* hint; }; struct FuncHints { @@ -1727,10 +1727,10 @@ std::optional WasmBinaryWriter::writeExpressionHints( std::optional WasmBinaryWriter::getBranchHintsBuffer() { return writeExpressionHints( Annotations::BranchHint, - [](const Function::CodeAnnotation& annotation) { + [](const CodeAnnotation& annotation) { return annotation.branchLikely; }, - [](const Function::CodeAnnotation& annotation, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -1746,10 +1746,10 @@ std::optional WasmBinaryWriter::getBranchHintsBuffer() { std::optional WasmBinaryWriter::getInlineHintsBuffer() { return writeExpressionHints( Annotations::InlineHint, - [](const Function::CodeAnnotation& annotation) { + [](const CodeAnnotation& annotation) { return annotation.inline_; }, - [](const Function::CodeAnnotation& annotation, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -1769,10 +1769,10 @@ std::optional WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { return writeExpressionHints( Annotations::EffectsIfMovedHint, - [](const Function::CodeAnnotation& annotation) { + [](const CodeAnnotation& annotation) { return annotation.effectsIfMoved; }, - [](const Function::CodeAnnotation& annotation, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 0 for now. buffer << U32LEB(0); @@ -5481,7 +5481,7 @@ void WasmBinaryReader::readExpressionHints(Name sectionName, void WasmBinaryReader::readBranchHints(size_t payloadLen) { readExpressionHints(Annotations::BranchHint, payloadLen, - [&](Function::CodeAnnotation& annotation) { + [&](CodeAnnotation& annotation) { auto size = getU32LEB(); if (size != 1) { throwError("bad BranchHint size"); @@ -5499,7 +5499,7 @@ void WasmBinaryReader::readBranchHints(size_t payloadLen) { void WasmBinaryReader::readInlineHints(size_t payloadLen) { readExpressionHints(Annotations::InlineHint, payloadLen, - [&](Function::CodeAnnotation& annotation) { + [&](CodeAnnotation& annotation) { auto size = getU32LEB(); if (size != 1) { throwError("bad InlineHint size"); @@ -5517,7 +5517,7 @@ void WasmBinaryReader::readInlineHints(size_t payloadLen) { void WasmBinaryReader::readEffectsIfMovedHints(size_t payloadLen) { readExpressionHints(Annotations::EffectsIfMovedHint, payloadLen, - [&](Function::CodeAnnotation& annotation) { + [&](CodeAnnotation& annotation) { auto size = getU32LEB(); if (size != 0) { throwError("bad EffectsIfMovedHint size"); From 694e4f99354a302106fdbe8ff975035dc045cbc0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 27 Jan 2026 17:00:39 -0800 Subject: [PATCH 26/35] work --- src/parser/contexts.h | 15 +++++--- src/wasm-ir-builder.h | 5 ++- src/wasm/wasm-binary.cpp | 71 +++++++++++++++--------------------- src/wasm/wasm-ir-builder.cpp | 12 ++++-- 4 files changed, 51 insertions(+), 52 deletions(-) diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 30fb29429d9..e486de94002 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -2026,9 +2026,10 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { if (!type.isSignature()) { return in.err(pos, "expected function type"); } - return withLoc( - pos, - irBuilder.makeIf(label ? *label : Name{}, type.getSignature(), parseAnnotations(annotations))); + return withLoc(pos, + irBuilder.makeIf(label ? *label : Name{}, + type.getSignature(), + parseAnnotations(annotations))); } Result<> visitElse() { return withLoc(irBuilder.visitElse()); } @@ -2467,7 +2468,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { const std::vector& annotations, Index label, bool isConditional) { - return withLoc(pos, irBuilder.makeBreak(label, isConditional, parseAnnotations(annotations))); + return withLoc( + pos, + irBuilder.makeBreak(label, isConditional, parseAnnotations(annotations))); } Result<> makeSwitch(Index pos, @@ -2648,7 +2651,9 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { BrOnOp op, Type in = Type::none, Type out = Type::none) { - return withLoc(pos, irBuilder.makeBrOn(label, op, in, out, parseAnnotations(annotations))); + return withLoc( + pos, + irBuilder.makeBrOn(label, op, in, out, parseAnnotations(annotations))); } Result<> makeStructNew(Index pos, diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index b270f746506..b54b40e8933 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -128,8 +128,9 @@ class IRBuilder : public UnifiedExpressionVisitor> { // signatures ensure that there are no missing fields. Result<> makeNop(); Result<> makeBlock(Name label, Signature sig); - Result<> - makeIf(Name label, Signature sig, const CodeAnnotation& annotations = CodeAnnotation()); + Result<> makeIf(Name label, + Signature sig, + const CodeAnnotation& annotations = CodeAnnotation()); Result<> makeLoop(Name label, Signature sig); Result<> makeBreak(Index label, bool isConditional, diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 2c898cec6f5..43bf2d667df 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1727,11 +1727,8 @@ std::optional WasmBinaryWriter::writeExpressionHints( std::optional WasmBinaryWriter::getBranchHintsBuffer() { return writeExpressionHints( Annotations::BranchHint, - [](const CodeAnnotation& annotation) { - return annotation.branchLikely; - }, - [](const CodeAnnotation& annotation, - BufferWithRandomAccess& buffer) { + [](const CodeAnnotation& annotation) { return annotation.branchLikely; }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -1746,11 +1743,8 @@ std::optional WasmBinaryWriter::getBranchHintsBuffer() { std::optional WasmBinaryWriter::getInlineHintsBuffer() { return writeExpressionHints( Annotations::InlineHint, - [](const CodeAnnotation& annotation) { - return annotation.inline_; - }, - [](const CodeAnnotation& annotation, - BufferWithRandomAccess& buffer) { + [](const CodeAnnotation& annotation) { return annotation.inline_; }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 1 for now. buffer << U32LEB(1); @@ -1769,11 +1763,8 @@ std::optional WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { return writeExpressionHints( Annotations::EffectsIfMovedHint, - [](const CodeAnnotation& annotation) { - return annotation.effectsIfMoved; - }, - [](const CodeAnnotation& annotation, - BufferWithRandomAccess& buffer) { + [](const CodeAnnotation& annotation) { return annotation.effectsIfMoved; }, + [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { // Hint size, always 0 for now. buffer << U32LEB(0); }); @@ -5479,39 +5470,37 @@ void WasmBinaryReader::readExpressionHints(Name sectionName, } void WasmBinaryReader::readBranchHints(size_t payloadLen) { - readExpressionHints(Annotations::BranchHint, - payloadLen, - [&](CodeAnnotation& annotation) { - auto size = getU32LEB(); - if (size != 1) { - throwError("bad BranchHint size"); - } + readExpressionHints( + Annotations::BranchHint, payloadLen, [&](CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 1) { + throwError("bad BranchHint size"); + } - auto likely = getU32LEB(); - if (likely != 0 && likely != 1) { - throwError("bad BranchHint value"); - } + auto likely = getU32LEB(); + if (likely != 0 && likely != 1) { + throwError("bad BranchHint value"); + } - annotation.branchLikely = likely; - }); + annotation.branchLikely = likely; + }); } void WasmBinaryReader::readInlineHints(size_t payloadLen) { - readExpressionHints(Annotations::InlineHint, - payloadLen, - [&](CodeAnnotation& annotation) { - auto size = getU32LEB(); - if (size != 1) { - throwError("bad InlineHint size"); - } + readExpressionHints( + Annotations::InlineHint, payloadLen, [&](CodeAnnotation& annotation) { + auto size = getU32LEB(); + if (size != 1) { + throwError("bad InlineHint size"); + } - uint8_t inline_ = getInt8(); - if (inline_ > 127) { - throwError("bad InlineHint value"); - } + uint8_t inline_ = getInt8(); + if (inline_ > 127) { + throwError("bad InlineHint value"); + } - annotation.inline_ = inline_; - }); + annotation.inline_ = inline_; + }); } void WasmBinaryReader::readEffectsIfMovedHints(size_t payloadLen) { diff --git a/src/wasm/wasm-ir-builder.cpp b/src/wasm/wasm-ir-builder.cpp index a610642ae61..139fdbca1dc 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -1313,8 +1313,9 @@ Result<> IRBuilder::makeBlock(Name label, Signature sig) { return visitBlockStart(block, sig.params); } -Result<> -IRBuilder::makeIf(Name label, Signature sig, const CodeAnnotation& annotations) { +Result<> IRBuilder::makeIf(Name label, + Signature sig, + const CodeAnnotation& annotations) { auto* iff = wasm.allocator.alloc(); iff->type = sig.results; applyAnnotations(iff, annotations); @@ -1980,8 +1981,11 @@ Result<> IRBuilder::makeRefGetDesc(HeapType type) { return Ok{}; } -Result<> IRBuilder::makeBrOn( - Index label, BrOnOp op, Type in, Type out, const CodeAnnotation& annotations) { +Result<> IRBuilder::makeBrOn(Index label, + BrOnOp op, + Type in, + Type out, + const CodeAnnotation& annotations) { std::optional descriptor; if (op == BrOnCastDesc || op == BrOnCastDescFail) { assert(out.isRef()); From a815fb7180ef12ee72ba1c3c54d137ad48775b97 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 11:49:12 -0800 Subject: [PATCH 27/35] simpl --- src/wasm-binary.h | 23 +++++++++++------------ src/wasm/wasm-binary.cpp | 31 ++++++++++++------------------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 449fa3fc09c..f324e533d17 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1720,21 +1720,20 @@ class WasmBinaryReader { void readDylink(size_t payloadLen); void readDylink0(size_t payloadLen); - // We read branch hints *after* the code section, even though they appear + // We read code annotations *after* the code section, even though they appear // earlier. That is simpler for us as we note expression locations as we scan - // code, and then just need to match them up. To do this, we note the branch - // hint position and size in the first pass, and handle it later. - size_t branchHintsPos = 0; - size_t branchHintsLen = 0; - void readBranchHints(size_t payloadLen); + // code, and then just need to match them up. To do this, we note the + // positions of annotation sections in the first pass, and handle them later. + struct AnnotationSectionInfo { + // The start position of the section. We will rewind to there to read it. + size_t pos; + // A lambda that will read the section, from that position. + std::function read; + }; + std::vector deferredAnnotationSections; - // Like branch hints, we note where the section is to read it later. - size_t inlineHintsPos = 0; - size_t inlineHintsLen = 0; + void readBranchHints(size_t payloadLen); void readInlineHints(size_t payloadLen); - - size_t effectsIfMovedHintsPos = 0; - size_t effectsIfMovedHintsLen = 0; void readEffectsIfMovedHints(size_t payloadLen); std::tuple diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 43bf2d667df..17d1a512ba6 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2165,18 +2165,11 @@ void WasmBinaryReader::read() { } } - // Go back and parse things we deferred. - if (branchHintsPos) { - pos = branchHintsPos; - readBranchHints(branchHintsLen); - } - if (inlineHintsPos) { - pos = inlineHintsPos; - readInlineHints(inlineHintsLen); - } - if (effectsIfMovedHintsPos) { - pos = effectsIfMovedHintsPos; - readEffectsIfMovedHints(effectsIfMovedHintsLen); + // Go back and parse annotations we deferred. + for (auto& [annotationPos, read] : deferredAnnotationSections) { + // Rewind to the right position, and read. + pos = annotationPos; + read(); } validateBinary(); @@ -2200,15 +2193,15 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName.equals(BinaryConsts::CustomSections::Dylink0)) { readDylink0(payloadLen); } else if (sectionName == Annotations::BranchHint) { - // Only note the position and length, we read this later. - branchHintsPos = pos; - branchHintsLen = payloadLen; + // Deferred. + deferredAnnotationSections.push_back( + AnnotationSectionInfo{pos, [=]() { readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { - inlineHintsPos = pos; - inlineHintsLen = payloadLen; + deferredAnnotationSections.push_back( + AnnotationSectionInfo{pos, [=]() { readInlineHints(payloadLen); }}); } else if (sectionName == Annotations::EffectsIfMovedHint) { - effectsIfMovedHintsPos = pos; - effectsIfMovedHintsLen = payloadLen; + deferredAnnotationSections.push_back(AnnotationSectionInfo{ + pos, [=]() { readEffectsIfMovedHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From 937b8734106a79b7dbb8b1b0362de6965a019519 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 12:15:59 -0800 Subject: [PATCH 28/35] UNDO.ifMoved --- src/parser/contexts.h | 2 - src/passes/Print.cpp | 6 -- src/passes/Vacuum.cpp | 26 +----- src/wasm-annotations.h | 1 - src/wasm-binary.h | 2 - src/wasm.h | 8 +- src/wasm/wasm-binary.cpp | 31 +------ src/wasm/wasm-ir-builder.cpp | 6 -- src/wasm/wasm.cpp | 1 - test/lit/effects-if-moved-hints.wast | 31 ------- test/lit/passes/vacuum-effects-if-moved.wast | 89 -------------------- 11 files changed, 6 insertions(+), 197 deletions(-) delete mode 100644 test/lit/effects-if-moved-hints.wast delete mode 100644 test/lit/passes/vacuum-effects-if-moved.wast diff --git a/src/parser/contexts.h b/src/parser/contexts.h index e486de94002..a077b4e2918 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1310,8 +1310,6 @@ struct AnnotationParserCtx { branchHint = &a; } else if (a.kind == Annotations::InlineHint) { inlineHint = &a; - } else if (a.kind == Annotations::EffectsIfMovedHint) { - ret.effectsIfMoved.emplace(); } } diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 7fd67cf822f..380c2db4629 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -2788,12 +2788,6 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) { restoreNormalColor(o); doIndent(o, indent); } - if (annotation.effectsIfMoved) { - Colors::grey(o); - o << "(@" << Annotations::EffectsIfMovedHint << ")\n"; - restoreNormalColor(o); - doIndent(o, indent); - } } } diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index 7d29d1d3ee9..b5e223d9f21 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -91,8 +91,10 @@ struct Vacuum : public WalkerPass> { curr->is() || curr->is() || curr->is()) { return curr; } - // Check if this expression itself must be kept. - if (mustKeepUnusedParent(curr)) { + // Check if this expression itself has side effects, ignoring children. + EffectAnalyzer self(getPassOptions(), *getModule()); + self.visit(curr); + if (self.hasUnremovableSideEffects()) { return curr; } // The result isn't used, and this has no side effects itself, so we can @@ -128,26 +130,6 @@ 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. - bool mustKeepUnusedParent(Expression* curr) { - if (auto* call = curr->dynCast()) { - auto& annotations = getFunction()->codeAnnotations; - auto iter = annotations.find(call); - if (iter != annotations.end()) { - auto& annotation = iter->second; - if (annotation.effectsIfMoved) { - // No need to check effects, this can be removed. - 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 cc4ee5083d9..888c356dcb9 100644 --- a/src/wasm-annotations.h +++ b/src/wasm-annotations.h @@ -27,7 +27,6 @@ namespace wasm::Annotations { extern const Name BranchHint; extern const Name InlineHint; -extern const Name EffectsIfMovedHint; } // namespace wasm::Annotations diff --git a/src/wasm-binary.h b/src/wasm-binary.h index f324e533d17..d9175f5d3c3 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1441,7 +1441,6 @@ class WasmBinaryWriter { std::optional getBranchHintsBuffer(); std::optional getInlineHintsBuffer(); - std::optional getEffectsIfMovedHintsBuffer(); // helpers void writeInlineString(std::string_view name); @@ -1734,7 +1733,6 @@ class WasmBinaryReader { void readBranchHints(size_t payloadLen); void readInlineHints(size_t payloadLen); - void readEffectsIfMovedHints(size_t payloadLen); std::tuple readMemoryAccess(bool isAtomic, bool isRMW); diff --git a/src/wasm.h b/src/wasm.h index c239ab1e737..3d016f80385 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2242,14 +2242,8 @@ struct CodeAnnotation { static const uint8_t AlwaysInline = 127; std::optional inline_; - // Binaryen intrinsic: Mark as having side effects if moved, but having no - // effects in the current position. See |callsIfMoved| in effects.h. - // TODO: link to spec - std::optional effectsIfMoved; - bool operator==(const CodeAnnotation& other) const { - return branchLikely == other.branchLikely && inline_ == other.inline_ && - effectsIfMoved == other.effectsIfMoved; + return branchLikely == other.branchLikely && inline_ == other.inline_; } }; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 17d1a512ba6..d3805c56210 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1623,7 +1623,6 @@ std::optional WasmBinaryWriter::writeCodeAnnotations() { append(getBranchHintsBuffer()); append(getInlineHintsBuffer()); - append(getEffectsIfMovedHintsBuffer()); return ret; } @@ -1759,17 +1758,6 @@ std::optional WasmBinaryWriter::getInlineHintsBuffer() { }); } -std::optional -WasmBinaryWriter::getEffectsIfMovedHintsBuffer() { - return writeExpressionHints( - Annotations::EffectsIfMovedHint, - [](const CodeAnnotation& annotation) { return annotation.effectsIfMoved; }, - [](const CodeAnnotation& annotation, BufferWithRandomAccess& buffer) { - // Hint size, always 0 for now. - 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]); @@ -2041,8 +2029,7 @@ void WasmBinaryReader::preScan() { auto sectionName = getInlineString(); if (sectionName == Annotations::BranchHint || - sectionName == Annotations::InlineHint || - sectionName == Annotations::EffectsIfMovedHint) { + sectionName == Annotations::InlineHint) { // Code annotations require code locations. // TODO: We could note which functions require code locations, as an // optimization. @@ -2199,9 +2186,6 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back( AnnotationSectionInfo{pos, [=]() { readInlineHints(payloadLen); }}); - } else if (sectionName == Annotations::EffectsIfMovedHint) { - deferredAnnotationSections.push_back(AnnotationSectionInfo{ - pos, [=]() { readEffectsIfMovedHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { @@ -5496,19 +5480,6 @@ void WasmBinaryReader::readInlineHints(size_t payloadLen) { }); } -void WasmBinaryReader::readEffectsIfMovedHints(size_t payloadLen) { - readExpressionHints(Annotations::EffectsIfMovedHint, - payloadLen, - [&](CodeAnnotation& annotation) { - auto size = getU32LEB(); - if (size != 0) { - throwError("bad EffectsIfMovedHint size"); - } - - annotation.effectsIfMoved.emplace(); - }); -} - 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 139fdbca1dc..96a0f639d51 100644 --- a/src/wasm/wasm-ir-builder.cpp +++ b/src/wasm/wasm-ir-builder.cpp @@ -2659,12 +2659,6 @@ void IRBuilder::applyAnnotations(Expression* expr, assert(func); func->codeAnnotations[expr].inline_ = annotation.inline_; } - - if (annotation.effectsIfMoved) { - // Only possible inside functions. - assert(func); - func->codeAnnotations[expr].effectsIfMoved.emplace(); - } } } // namespace wasm diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index e5b7aea7919..447847c18d7 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -69,7 +69,6 @@ namespace Annotations { const Name BranchHint = "metadata.code.branch_hint"; const Name InlineHint = "metadata.code.inline"; -const Name EffectsIfMovedHint = "binaryen.effects.if.moved"; } // namespace Annotations diff --git a/test/lit/effects-if-moved-hints.wast b/test/lit/effects-if-moved-hints.wast deleted file mode 100644 index eadde670a4b..00000000000 --- a/test/lit/effects-if-moved-hints.wast +++ /dev/null @@ -1,31 +0,0 @@ -;; 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.effects.if.moved) - ;; 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.effects.if.moved) - ;; RTRIP-NEXT: (call $func) - ;; RTRIP-NEXT: (call $func) - ;; RTRIP-NEXT: ) - (func $func - ;; Three calls, one annotated in the middle. - (call $func) - (@binaryen.effects.if.moved) - (call $func) - (call $func) - ) -) diff --git a/test/lit/passes/vacuum-effects-if-moved.wast b/test/lit/passes/vacuum-effects-if-moved.wast deleted file mode 100644 index 19766013c2d..00000000000 --- a/test/lit/passes/vacuum-effects-if-moved.wast +++ /dev/null @@ -1,89 +0,0 @@ -;; 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.effects.if.moved) - (call $calls-dropped - (i32.const 0) - ) - ) - ;; As above, but a parameter has effects. We must keep that around, without - ;; the call. - (drop - (@binaryen.effects.if.moved) - (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.effects.if.moved) - ;; CHECK-NEXT: (call $calls-used - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.set $x - ;; CHECK-NEXT: (@binaryen.effects.if.moved) - ;; CHECK-NEXT: (call $calls-used - ;; CHECK-NEXT: (local.tee $x - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (drop - ;; 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.effects.if.moved) - (call $calls-used - (i32.const 0) - ) - ) - (local.set $x - (@binaryen.effects.if.moved) - (call $calls-used - (local.tee $x - (i32.const 1) - ) - ) - ) - (drop - (call $calls-used - (i32.const 2) - ) - ) - (i32.const 3) - ) -) - From 1e499ab5e40c3407b5226a8b1206afb6033ee62b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 13:21:38 -0800 Subject: [PATCH 29/35] avoid ci warning --- src/wasm/wasm-binary.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index d3805c56210..e5f8f1967ca 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2182,10 +2182,10 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::BranchHint) { // Deferred. deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=]() { readBranchHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=,this]() { readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=]() { readInlineHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=,this]() { readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From a0e11836b47ac54e553306eba41af959252a3c4e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 13:31:31 -0800 Subject: [PATCH 30/35] fmrt --- src/wasm/wasm-binary.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index e5f8f1967ca..0669f5eecda 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2182,10 +2182,10 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::BranchHint) { // Deferred. deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=,this]() { readBranchHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=, this]() { readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=,this]() { readInlineHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=, this]() { readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From 2364fb8df585c495525c2500328cb038120d1ce6 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 13:33:02 -0800 Subject: [PATCH 31/35] avoid c++20 feature warning --- src/wasm/wasm-binary.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 0669f5eecda..618c905228e 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2182,10 +2182,10 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::BranchHint) { // Deferred. deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=, this]() { readBranchHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=]() { this->readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=, this]() { readInlineHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=]() { this->readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From 0212940b76cd8b58c367b46a46ae39d56208214a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 13:45:45 -0800 Subject: [PATCH 32/35] try the only remaining possibility and cross fingers --- src/wasm/wasm-binary.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 618c905228e..5ef8d9435e0 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2182,10 +2182,10 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { } else if (sectionName == Annotations::BranchHint) { // Deferred. deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=]() { this->readBranchHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=, this]() { this->readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=]() { this->readInlineHints(payloadLen); }}); + AnnotationSectionInfo{pos, [=, this]() { this->readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From be7922c4771081b3deb0c69ef41f7316ce25dec3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 28 Jan 2026 15:45:47 -0800 Subject: [PATCH 33/35] try yet another way to appease the c++ compiler's warnings --- src/wasm/wasm-binary.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 5ef8d9435e0..230ab8af0c2 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -2181,11 +2181,11 @@ void WasmBinaryReader::readCustomSection(size_t payloadLen) { readDylink0(payloadLen); } else if (sectionName == Annotations::BranchHint) { // Deferred. - deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=, this]() { this->readBranchHints(payloadLen); }}); + deferredAnnotationSections.push_back(AnnotationSectionInfo{ + pos, [this, payloadLen]() { this->readBranchHints(payloadLen); }}); } else if (sectionName == Annotations::InlineHint) { - deferredAnnotationSections.push_back( - AnnotationSectionInfo{pos, [=, this]() { this->readInlineHints(payloadLen); }}); + deferredAnnotationSections.push_back(AnnotationSectionInfo{ + pos, [this, payloadLen]() { this->readInlineHints(payloadLen); }}); } else { // an unfamiliar custom section if (sectionName.equals(BinaryConsts::CustomSections::Linking)) { From d83c9138e78765699c631df0ac5b2cf417906789 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 29 Jan 2026 11:38:36 -0800 Subject: [PATCH 34/35] simplify to ={} --- src/wasm-ir-builder.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index b54b40e8933..1564fcfcf0f 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -130,21 +130,21 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeBlock(Name label, Signature sig); Result<> makeIf(Name label, Signature sig, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeLoop(Name label, Signature sig); Result<> makeBreak(Index label, bool isConditional, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. Result<> makeCall(Name func, bool isReturn, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeCallIndirect(Name table, HeapType type, bool isReturn, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeLocalGet(Index local); Result<> makeLocalSet(Index local); Result<> makeLocalTee(Index local); @@ -228,7 +228,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { Result<> makeI31Get(bool signed_); Result<> makeCallRef(HeapType type, bool isReturn, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeRefTest(Type type); Result<> makeRefCast(Type type, bool isDesc); Result<> makeRefGetDesc(HeapType type); @@ -236,7 +236,7 @@ class IRBuilder : public UnifiedExpressionVisitor> { BrOnOp op, Type in = Type::none, Type out = Type::none, - const CodeAnnotation& annotations = CodeAnnotation()); + const CodeAnnotation& annotations = {}); Result<> makeStructNew(HeapType type, bool isDesc); Result<> makeStructNewDefault(HeapType type, bool isDesc); Result<> From 35df9048d76e26d7f57e148d3966953a6fce3179 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 29 Jan 2026 11:43:25 -0800 Subject: [PATCH 35/35] format --- src/wasm-ir-builder.h | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/wasm-ir-builder.h b/src/wasm-ir-builder.h index 1564fcfcf0f..11f43b46df6 100644 --- a/src/wasm-ir-builder.h +++ b/src/wasm-ir-builder.h @@ -128,23 +128,20 @@ class IRBuilder : public UnifiedExpressionVisitor> { // signatures ensure that there are no missing fields. Result<> makeNop(); Result<> makeBlock(Name label, Signature sig); - Result<> makeIf(Name label, - Signature sig, - const CodeAnnotation& annotations = {}); + Result<> + makeIf(Name label, Signature sig, const CodeAnnotation& annotations = {}); Result<> makeLoop(Name label, Signature sig); Result<> makeBreak(Index label, bool isConditional, const CodeAnnotation& annotations = {}); Result<> makeSwitch(const std::vector& labels, Index defaultLabel); // Unlike Builder::makeCall, this assumes the function already exists. - Result<> makeCall(Name func, - bool isReturn, - const CodeAnnotation& annotations = {}); Result<> - makeCallIndirect(Name table, - HeapType type, - bool isReturn, - const CodeAnnotation& annotations = {}); + makeCall(Name func, bool isReturn, const CodeAnnotation& annotations = {}); + Result<> makeCallIndirect(Name table, + HeapType type, + bool isReturn, + const CodeAnnotation& annotations = {}); Result<> makeLocalGet(Index local); Result<> makeLocalSet(Index local); Result<> makeLocalTee(Index local);