From 623a0bf58a362d33df3878b18243065ab18eb7ab Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Tue, 20 Oct 2026 10:58:41 -0700 Subject: [PATCH 1/5] [CFP][GSI] Fix catch pops after optimizing reads When ConstantFieldPropagation or GlobalStructInference optimizes a struct.get or ref.get_desc, it either wraps the reference operand in a Block or places it into the condition (third child) of a Select. Any pop instruction inside the reference from a legacy exception-handling catch body then becomes either nested inside a block or displaced out of the catch body's first-descendant chain. Update EHUtils::handleBlockNestedPop to also find and lift pops that have been moved out of the first-descendant chain, and call EHUtils::handleBlockNestedPops in both passes when reads are optimized. --- src/ir/eh-utils.cpp | 32 +++++++ src/passes/ConstantFieldPropagation.cpp | 4 +- src/passes/GlobalStructInference.cpp | 8 ++ test/lit/passes/cfp-reftest.wast | 83 ++++++++++++++++++ test/lit/passes/cfp.wast | 46 ++++++++++ test/lit/passes/gsi.wast | 107 ++++++++++++++++++++++++ 6 files changed, 279 insertions(+), 1 deletion(-) diff --git a/src/ir/eh-utils.cpp b/src/ir/eh-utils.cpp index 19f48477a70..d60883dd6f8 100644 --- a/src/ir/eh-utils.cpp +++ b/src/ir/eh-utils.cpp @@ -106,6 +106,28 @@ bool containsValidDanglingPop(Expression* catchBody) { return pop != nullptr && !isPopNested; } +static Expression** findPopPtr(Expression*& expr) { + SmallVector work; + work.push_back(&expr); + while (!work.empty()) { + auto* currPtr = work.back(); + work.pop_back(); + auto* curr = *currPtr; + if (curr->is()) { + return currPtr; + } else if (auto* try_ = curr->dynCast()) { + // We don't go into inner catch bodies; pops in inner catch bodies + // belong to the inner catches + work.push_back(&try_->body); + } else { + for (auto& child : ChildIterator(curr)) { + work.push_back(&child); + } + } + } + return nullptr; +} + void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { Builder builder(wasm); for (Index i = 0; i < try_->catchTags.size(); i++) { @@ -119,6 +141,16 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { bool isPopNested = false; Expression** popPtr = nullptr; Expression* pop = getFirstPop(catchBody, isPopNested, popPtr); + if (!pop) { + // The pop may have been moved out of the first-descendant line, e.g. + // into the condition of a select. Find it and fix it up just like a + // block-nested pop. + popPtr = findPopPtr(catchBody); + if (popPtr) { + pop = *popPtr; + isPopNested = true; + } + } assert(pop && "Pop has not been found in this catch"); // Change code like diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 66f45bb6047..96fd6b7186b 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -54,6 +54,7 @@ #include #include "ir/bits.h" +#include "ir/eh-utils.h" #include "ir/gc-type-utils.h" #include "ir/possible-constant.h" #include "ir/struct-utils.h" @@ -453,9 +454,10 @@ struct FunctionOptimizer : public WalkerPass> { WalkerPass>::doWalkFunction(func); // If we changed anything, we need to update parent types as types may have - // changed. + // changed, and fix up any pops that we enclosed in a block. if (changed) { ReFinalize().walkFunctionInModule(func, getModule()); + EHUtils::handleBlockNestedPops(func, *getModule()); } } diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp index 744baee5e69..32b225f9b06 100644 --- a/src/passes/GlobalStructInference.cpp +++ b/src/passes/GlobalStructInference.cpp @@ -59,6 +59,7 @@ #include "ir/bits.h" #include "ir/debuginfo.h" +#include "ir/eh-utils.h" #include "ir/find_all.h" #include "ir/module-utils.h" #include "ir/names.h" @@ -353,6 +354,7 @@ struct GlobalStructInference : public Pass { : parent(parent), globalsToUnnest(globalsToUnnest) {} bool refinalize = false; + bool needEHFixups = false; // As we prepare to un-nest globals, we create global.gets of the global // that we will un-nest the content to. That global does not yet exist, @@ -465,6 +467,7 @@ struct GlobalStructInference : public Pass { // to synchronize with in the first place. ref = builder.makeSequence( builder.makeDrop(builder.makeRefAs(RefAsNonNull, ref)), get); + needEHFixups = true; return; } @@ -514,6 +517,7 @@ struct GlobalStructInference : public Pass { replaceCurrent(builder.makeSequence( builder.makeDrop(builder.makeRefAs(RefAsNonNull, ref)), getReadValue(values[0], fieldIndex, field, curr))); + needEHFixups = true; return; } assert(values.size() == 2); @@ -547,6 +551,7 @@ struct GlobalStructInference : public Pass { builder.makeRefEq(builder.makeRefAs(RefAsNonNull, ref), getGlobal), left, right)); + needEHFixups = true; } void visitRefCast(RefCast* curr) { @@ -603,6 +608,9 @@ struct GlobalStructInference : public Pass { if (refinalize) { ReFinalize().walkFunctionInModule(func, getModule()); } + if (needEHFixups) { + EHUtils::handleBlockNestedPops(func, *getModule()); + } } Value readFromStructNew(StructNew* structNew, diff --git a/test/lit/passes/cfp-reftest.wast b/test/lit/passes/cfp-reftest.wast index 48cda2bc226..8163e632ded 100644 --- a/test/lit/passes/cfp-reftest.wast +++ b/test/lit/passes/cfp-reftest.wast @@ -1669,3 +1669,86 @@ ) ) ) + +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $super (sub (struct (field i32)))) + (type $super (sub (struct (field i32)))) + ;; CHECK: (type $sub1 (sub final $super (struct (field i32)))) + (type $sub1 (sub final $super (struct (field i32)))) + ;; CHECK: (type $sub2 (sub final $super (struct (field i32) (field i64)))) + (type $sub2 (sub final $super (struct (field i32) (field i64)))) + ) + ;; CHECK: (type $3 (func)) + + ;; CHECK: (type $4 (func (param (ref $super)))) + + ;; CHECK: (tag $e (type $4) (param (ref $super))) + (tag $e (param (ref $super))) + + ;; CHECK: (func $make (type $3) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new $sub1 + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new $sub2 + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: (i64.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $make + (drop + (struct.new $sub1 + (i32.const 10) + ) + ) + (drop + (struct.new $sub2 + (i32.const 20) + (i64.const 0) + ) + ) + ) + + ;; CHECK: (func $catch-pop (type $3) + ;; CHECK-NEXT: (local $0 (ref $super)) + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $e + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (pop (ref $super)) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (select + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: (i32.const 20) + ;; CHECK-NEXT: (ref.test (ref $sub1) + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $catch-pop + ;; Optimizing this struct.get into a select moves its reference into the + ;; select condition (third child), requiring the enclosed pop to be fixed up. + (try + (do) + (catch $e + (drop + (struct.get $super 0 + (pop (ref $super)) + ) + ) + ) + ) + ) +) diff --git a/test/lit/passes/cfp.wast b/test/lit/passes/cfp.wast index fa7fb9277d6..a6d051ad7e5 100644 --- a/test/lit/passes/cfp.wast +++ b/test/lit/passes/cfp.wast @@ -3366,3 +3366,49 @@ ) ) ) + +(module + ;; CHECK: (type $struct (struct (field i32))) + (type $struct (struct (field i32))) + ;; CHECK: (type $1 (func (param (ref $struct)))) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (tag $e (type $1) (param (ref $struct))) + (tag $e (param (ref $struct))) + + ;; CHECK: (func $catch-pop (type $2) + ;; CHECK-NEXT: (local $0 (ref $struct)) + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $e + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (pop (ref $struct)) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $catch-pop + ;; Optimizing this struct.get wraps its reference in a block, requiring the + ;; enclosed pop to be fixed up. + (try + (do) + (catch $e + (drop + (struct.get $struct 0 + (pop (ref $struct)) + ) + ) + ) + ) + ) +) diff --git a/test/lit/passes/gsi.wast b/test/lit/passes/gsi.wast index 7d5702cc523..2b20719df19 100644 --- a/test/lit/passes/gsi.wast +++ b/test/lit/passes/gsi.wast @@ -2415,3 +2415,110 @@ ) ) ) + +(module + ;; CHECK: (type $one (struct (field i32))) + (type $one (struct (field i32))) + ;; CHECK: (type $two (struct (field i64))) + (type $two (struct (field i64))) + ;; CHECK: (type $2 (func)) + + ;; CHECK: (type $3 (func (param (ref $one)))) + + ;; CHECK: (type $4 (func (param (ref $two)))) + + ;; CHECK: (global $one (ref $one) (struct.new $one + ;; CHECK-NEXT: (i32.const 10) + ;; CHECK-NEXT: )) + (global $one (ref $one) (struct.new $one (i32.const 10))) + ;; CHECK: (global $two-a (ref $two) (struct.new $two + ;; CHECK-NEXT: (i64.const 10) + ;; CHECK-NEXT: )) + (global $two-a (ref $two) (struct.new $two (i64.const 10))) + ;; CHECK: (global $two-b (ref $two) (struct.new $two + ;; CHECK-NEXT: (i64.const 20) + ;; CHECK-NEXT: )) + (global $two-b (ref $two) (struct.new $two (i64.const 20))) + ;; CHECK: (tag $e-one (type $3) (param (ref $one))) + (tag $e-one (param (ref $one))) + ;; CHECK: (tag $e-two (type $4) (param (ref $two))) + (tag $e-two (param (ref $two))) + + ;; CHECK: (func $catch-pop-block (type $2) + ;; CHECK-NEXT: (local $0 (ref $one)) + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $e-one + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (pop (ref $one)) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.get $one 0 + ;; CHECK-NEXT: (block (result (ref $one)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $one) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $catch-pop-block + ;; Optimizing this struct.get wraps its reference in a block, requiring the + ;; enclosed pop to be fixed up. + (try + (do) + (catch $e-one + (drop + (struct.get $one 0 + (pop (ref $one)) + ) + ) + ) + ) + ) + + ;; CHECK: (func $catch-pop-select (type $2) + ;; CHECK-NEXT: (local $0 (ref $two)) + ;; CHECK-NEXT: (try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch $e-two + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (pop (ref $two)) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (select + ;; CHECK-NEXT: (i64.const 10) + ;; CHECK-NEXT: (i64.const 20) + ;; CHECK-NEXT: (ref.eq + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $two-a) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $catch-pop-select + ;; Optimizing this struct.get into a select moves its reference into the + ;; select condition (third child), requiring the enclosed pop to be fixed up. + (try + (do) + (catch $e-two + (drop + (struct.get $two 0 + (pop (ref $two)) + ) + ) + ) + ) + ) +) From 6f9cac0ff6ce6679591e8e88f006d696b0ab8536 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 21 Oct 2026 17:37:38 -0700 Subject: [PATCH 2/5] replace getFirstPop, use PostWalker, depend on #9292 --- src/ir/eh-utils.cpp | 211 ++++++++++++++++++++------------------------ 1 file changed, 98 insertions(+), 113 deletions(-) diff --git a/src/ir/eh-utils.cpp b/src/ir/eh-utils.cpp index d60883dd6f8..1c07f6126fa 100644 --- a/src/ir/eh-utils.cpp +++ b/src/ir/eh-utils.cpp @@ -15,117 +15,113 @@ */ #include "ir/eh-utils.h" -#include "ir/branch-utils.h" #include "ir/find_all.h" +#include "ir/iteration.h" +#include "ir/properties.h" +#include "wasm-builder.h" namespace wasm { namespace EHUtils { -// This returns three values, some of them as output parameters: -// - Return value: 'pop' expression (Expression*), when there is one in -// first-descendant line. If there's no such pop, it returns null. -// - isPopNested: Whether the discovered 'pop' is nested within a block -// - popPtr: 'pop' expression's pointer (Expression**), when there is one found -// -// When 'catchBody' itself is a 'pop', 'pop''s pointer is null, because there is -// no way to get the given expression's address. But that's fine because pop's -// pointer is only necessary (in handleBlockNestedPops) to fix it up when it is -// nested, and if 'catchBody' itself is a pop, we don't need to fix it up. -static Expression* -getFirstPop(Expression* catchBody, bool& isPopNested, Expression**& popPtr) { - Expression* firstChild = catchBody; - isPopNested = false; - popPtr = nullptr; - // When there are multiple expressions within a catch body, an implicit - // block is created within it for convenience purposes. - auto* implicitBlock = catchBody->dynCast(); - - // Go down the line for the first child until we reach a leaf. A pop should be - // in that first-descendant line. - Expression** firstChildPtr = nullptr; - while (true) { - if (firstChild->is()) { - popPtr = firstChildPtr; - return firstChild; +// Finds the first 'pop' in the given catch body that does not belong to an +// inner try's catch, and sets `isInvalid` to true if the pop is enclosed in a +// control flow structure (other than an unnamed block) or is not in the catch +// body's first-descendant line. +static Expression** findPopPtr(Expression*& catchBody, + bool& isInvalid, + bool stopIfInvalid = false) { + struct PopFinder : PostWalker { + bool isInvalid = false; + bool stopIfInvalid = false; + bool pushedTask = false; + Expression** popPtr = nullptr; + + PopFinder(Expression*& catchBody, bool stopIfInvalid) + : stopIfInvalid(stopIfInvalid) { + walk(catchBody); } - if (Properties::isControlFlowStructure(firstChild)) { - if (auto* iff = firstChild->dynCast()) { - // If's condition is a value child who comes before an 'if' instruction - // in binary, it is fine if a 'pop' is in there. We don't allow a 'pop' - // to be in an 'if''s then or else body because they are not first - // descendants. - firstChild = iff->condition; - firstChildPtr = &iff->condition; - continue; - } else if (firstChild->is()) { - // We don't allow the pop to be included in a loop, because it cannot be - // run more than once - return nullptr; + void pushTask(TaskFunc func, Expression** currp) { + pushedTask = true; + PostWalker::pushTask(func, currp); + } + + void maybePushTask(TaskFunc func, Expression** currp) { + if (*currp) { + pushTask(func, currp); + } + } + + static void scan(PopFinder* self, Expression** currp) { + if (self->popPtr || (self->stopIfInvalid && self->isInvalid)) { + return; + } + + Expression* curr = *currp; + if (curr->is()) { + self->popPtr = currp; + return; } - if (firstChild->is()) { - // If there are no branches that targets the implicit block, it will be - // removed when written back. But if there are branches that target the - // implicit block, - // (catch $e - // (block $l0 - // (pop i32) ;; within a block! - // (br $l0) - // ... - // ) - // This cannot be removed, so this is considered a nested pop (which we - // should fix). - if (firstChild == implicitBlock) { - if (BranchUtils::BranchSeeker::has(implicitBlock, - implicitBlock->name)) { - isPopNested = true; + + if (auto* try_ = curr->dynCast()) { + self->isInvalid = true; + if (!self->stopIfInvalid) { + // Do not walk into inner catch bodies; pops in inner catch bodies + // belong to the inner catches. + self->pushTask(scan, &try_->body); + } + return; + } + + if (!self->isInvalid && Properties::isControlFlowStructure(curr)) { + if (curr->is()) { + // If's condition is a value child who comes before an 'if' + // instruction in binary, so it is fine if a 'pop' is in there. We + // don't allow a 'pop' to be in an 'if''s then or else body because + // they are not first descendants. PostWalker scans the condition + // first, and if the pop is not in the condition, reaching the end of + // the condition's first-descendant line will set `isInvalid` before + // the arms are scanned. + } else if (auto* block = curr->dynCast()) { + // Unnamed blocks are not emitted in the binary format, so a 'pop' at + // the beginning of an unnamed block is valid. Named blocks may be + // emitted if targeted by branches, so treat any 'pop' inside a named + // block as invalid. + if (block->name.is()) { + self->isInvalid = true; } } else { - isPopNested = true; + assert(curr->is() || curr->is()); + self->isInvalid = true; } - } else if (firstChild->is() || firstChild->is()) { - isPopNested = true; - } else { - WASM_UNREACHABLE("Unexpected control flow expression"); } - } - ChildIterator it(firstChild); - if (it.getNumChildren() == 0) { - return nullptr; - } - firstChildPtr = &*it.begin(); - firstChild = *firstChildPtr; - } -} -bool containsValidDanglingPop(Expression* catchBody) { - bool isPopNested = false; - Expression** popPtr = nullptr; - auto* pop = getFirstPop(catchBody, isPopNested, popPtr); - return pop != nullptr && !isPopNested; -} + if (self->stopIfInvalid && self->isInvalid) { + return; + } -static Expression** findPopPtr(Expression*& expr) { - SmallVector work; - work.push_back(&expr); - while (!work.empty()) { - auto* currPtr = work.back(); - work.pop_back(); - auto* curr = *currPtr; - if (curr->is()) { - return currPtr; - } else if (auto* try_ = curr->dynCast()) { - // We don't go into inner catch bodies; pops in inner catch bodies - // belong to the inner catches - work.push_back(&try_->body); - } else { - for (auto& child : ChildIterator(curr)) { - work.push_back(&child); + // If PostWalker::scan doesn't push a task to scan any children, then we + // must have reached a leaf node. The first leaf reached is the leftmost + // leaf, and the only valid place for a Pop to appear. We already know + // this is not a Pop, so if this is a leaf, then any later Pop will be + // invalid. + self->pushedTask = false; + PostWalker::scan(self, currp); + if (!self->pushedTask) { + self->isInvalid = true; } } - } - return nullptr; + } finder(catchBody, stopIfInvalid); + + isInvalid = finder.isInvalid; + return finder.popPtr; +} + +bool containsValidDanglingPop(Expression* catchBody) { + bool isInvalid = false; + auto* popPtr = findPopPtr(catchBody, isInvalid, /*stopIfInvalid=*/true); + return popPtr != nullptr && !isInvalid; } void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { @@ -137,21 +133,10 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { continue; } - auto* catchBody = try_->catchBodies[i]; - bool isPopNested = false; - Expression** popPtr = nullptr; - Expression* pop = getFirstPop(catchBody, isPopNested, popPtr); - if (!pop) { - // The pop may have been moved out of the first-descendant line, e.g. - // into the condition of a select. Find it and fix it up just like a - // block-nested pop. - popPtr = findPopPtr(catchBody); - if (popPtr) { - pop = *popPtr; - isPopNested = true; - } - } - assert(pop && "Pop has not been found in this catch"); + auto*& catchBody = try_->catchBodies[i]; + bool isInvalid = false; + Expression** popPtr = findPopPtr(catchBody, isInvalid); + assert(popPtr && "Pop has not been found in this catch"); // Change code like // (catch $e @@ -170,12 +155,12 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { // (local.get $new) // ) // ) - if (isPopNested) { - assert(popPtr); + if (isInvalid) { + Expression* pop = *popPtr; Index newLocal = builder.addVar(func, pop->type); - try_->catchBodies[i] = - builder.makeSequence(builder.makeLocalSet(newLocal, pop), catchBody); *popPtr = builder.makeLocalGet(newLocal, pop->type); + catchBody = + builder.makeSequence(builder.makeLocalSet(newLocal, pop), catchBody); } } } From f71a5ae4f1bfe870c556a3d941bab0e8dfc09ecb Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 28 Oct 2026 15:47:08 -0700 Subject: [PATCH 3/5] Remove unnecessary block-nested pop fixups and tests after #9292 --- src/passes/ConstantFieldPropagation.cpp | 2 +- src/passes/GlobalStructInference.cpp | 2 - test/lit/passes/cfp.wast | 46 --------------------- test/lit/passes/gsi.wast | 55 ++----------------------- 4 files changed, 4 insertions(+), 101 deletions(-) diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 96fd6b7186b..420cd0dd364 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -454,7 +454,7 @@ struct FunctionOptimizer : public WalkerPass> { WalkerPass>::doWalkFunction(func); // If we changed anything, we need to update parent types as types may have - // changed, and fix up any pops that we enclosed in a block. + // changed, and fix up any pops that we moved into a select condition. if (changed) { ReFinalize().walkFunctionInModule(func, getModule()); EHUtils::handleBlockNestedPops(func, *getModule()); diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp index 5dc19d75c9d..3723381f64e 100644 --- a/src/passes/GlobalStructInference.cpp +++ b/src/passes/GlobalStructInference.cpp @@ -467,7 +467,6 @@ struct GlobalStructInference : public Pass { // to synchronize with in the first place. ref = builder.makeSequence( builder.makeDrop(builder.makeRefAs(RefAsNonNull, ref)), get); - needEHFixups = true; return; } @@ -517,7 +516,6 @@ struct GlobalStructInference : public Pass { replaceCurrent(builder.makeSequence( builder.makeDrop(builder.makeRefAs(RefAsNonNull, ref)), getReadValue(values[0], fieldIndex, field, curr))); - needEHFixups = true; return; } assert(values.size() == 2); diff --git a/test/lit/passes/cfp.wast b/test/lit/passes/cfp.wast index a6d051ad7e5..fa7fb9277d6 100644 --- a/test/lit/passes/cfp.wast +++ b/test/lit/passes/cfp.wast @@ -3366,49 +3366,3 @@ ) ) ) - -(module - ;; CHECK: (type $struct (struct (field i32))) - (type $struct (struct (field i32))) - ;; CHECK: (type $1 (func (param (ref $struct)))) - - ;; CHECK: (type $2 (func)) - - ;; CHECK: (tag $e (type $1) (param (ref $struct))) - (tag $e (param (ref $struct))) - - ;; CHECK: (func $catch-pop (type $2) - ;; CHECK-NEXT: (local $0 (ref $struct)) - ;; CHECK-NEXT: (try - ;; CHECK-NEXT: (do - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (catch $e - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (pop (ref $struct)) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (block - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (unreachable) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $catch-pop - ;; Optimizing this struct.get wraps its reference in a block, requiring the - ;; enclosed pop to be fixed up. - (try - (do) - (catch $e - (drop - (struct.get $struct 0 - (pop (ref $struct)) - ) - ) - ) - ) - ) -) diff --git a/test/lit/passes/gsi.wast b/test/lit/passes/gsi.wast index 2b20719df19..401bf374cc7 100644 --- a/test/lit/passes/gsi.wast +++ b/test/lit/passes/gsi.wast @@ -2417,20 +2417,12 @@ ) (module - ;; CHECK: (type $one (struct (field i32))) - (type $one (struct (field i32))) ;; CHECK: (type $two (struct (field i64))) (type $two (struct (field i64))) - ;; CHECK: (type $2 (func)) - - ;; CHECK: (type $3 (func (param (ref $one)))) + ;; CHECK: (type $1 (func (param (ref $two)))) - ;; CHECK: (type $4 (func (param (ref $two)))) + ;; CHECK: (type $2 (func)) - ;; CHECK: (global $one (ref $one) (struct.new $one - ;; CHECK-NEXT: (i32.const 10) - ;; CHECK-NEXT: )) - (global $one (ref $one) (struct.new $one (i32.const 10))) ;; CHECK: (global $two-a (ref $two) (struct.new $two ;; CHECK-NEXT: (i64.const 10) ;; CHECK-NEXT: )) @@ -2439,50 +2431,9 @@ ;; CHECK-NEXT: (i64.const 20) ;; CHECK-NEXT: )) (global $two-b (ref $two) (struct.new $two (i64.const 20))) - ;; CHECK: (tag $e-one (type $3) (param (ref $one))) - (tag $e-one (param (ref $one))) - ;; CHECK: (tag $e-two (type $4) (param (ref $two))) + ;; CHECK: (tag $e-two (type $1) (param (ref $two))) (tag $e-two (param (ref $two))) - ;; CHECK: (func $catch-pop-block (type $2) - ;; CHECK-NEXT: (local $0 (ref $one)) - ;; CHECK-NEXT: (try - ;; CHECK-NEXT: (do - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (catch $e-one - ;; CHECK-NEXT: (local.set $0 - ;; CHECK-NEXT: (pop (ref $one)) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.get $one 0 - ;; CHECK-NEXT: (block (result (ref $one)) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (ref.as_non_null - ;; CHECK-NEXT: (local.get $0) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (global.get $one) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $catch-pop-block - ;; Optimizing this struct.get wraps its reference in a block, requiring the - ;; enclosed pop to be fixed up. - (try - (do) - (catch $e-one - (drop - (struct.get $one 0 - (pop (ref $one)) - ) - ) - ) - ) - ) - ;; CHECK: (func $catch-pop-select (type $2) ;; CHECK-NEXT: (local $0 (ref $two)) ;; CHECK-NEXT: (try From cfd588848e09f4b9acd8e3ffa0dcad431ffc3b7c Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 28 Oct 2026 16:46:58 -0700 Subject: [PATCH 4/5] updates --- src/ir/eh-utils.cpp | 4 ++-- src/passes/ConstantFieldPropagation.cpp | 8 +++++++- src/passes/GlobalStructInference.cpp | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ir/eh-utils.cpp b/src/ir/eh-utils.cpp index 1c07f6126fa..8cefecf856d 100644 --- a/src/ir/eh-utils.cpp +++ b/src/ir/eh-utils.cpp @@ -141,7 +141,7 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { // Change code like // (catch $e // ... - // (block + // (block $name // (pop i32) // ) // ) @@ -151,7 +151,7 @@ void handleBlockNestedPop(Try* try_, Function* func, Module& wasm) { // (pop i32) // ) // ... - // (block + // (block $name // (local.get $new) // ) // ) diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 420cd0dd364..a2fb360edc0 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -448,15 +448,20 @@ struct FunctionOptimizer : public WalkerPass> { makeExpression(values[1 - testIndex].constant, refHeapType, curr))); changed = true; + // Placing `ref` into the select condition (the third child) moves any + // enclosed `pop` out of the catch body's first-descendant line. + needEHFixups = true; } void doWalkFunction(Function* func) { WalkerPass>::doWalkFunction(func); // If we changed anything, we need to update parent types as types may have - // changed, and fix up any pops that we moved into a select condition. + // changed. if (changed) { ReFinalize().walkFunctionInModule(func, getModule()); + } + if (needEHFixups) { EHUtils::handleBlockNestedPops(func, *getModule()); } } @@ -468,6 +473,7 @@ struct FunctionOptimizer : public WalkerPass> { const bool refTest; bool changed = false; + bool needEHFixups = false; }; struct PCVScanner diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp index 3723381f64e..b568f231afe 100644 --- a/src/passes/GlobalStructInference.cpp +++ b/src/passes/GlobalStructInference.cpp @@ -549,6 +549,8 @@ struct GlobalStructInference : public Pass { builder.makeRefEq(builder.makeRefAs(RefAsNonNull, ref), getGlobal), left, right)); + // Placing `ref` into the select condition (the third child) moves any + // enclosed `pop` out of the catch body's first-descendant line. needEHFixups = true; } From 1056e8531a901f259bd1b61e4ed82f2c3301f618 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 29 Oct 2026 10:10:35 -0700 Subject: [PATCH 5/5] comment --- src/ir/eh-utils.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ir/eh-utils.cpp b/src/ir/eh-utils.cpp index 8cefecf856d..1e8c3c17cbb 100644 --- a/src/ir/eh-utils.cpp +++ b/src/ir/eh-utils.cpp @@ -65,6 +65,9 @@ static Expression** findPopPtr(Expression*& catchBody, } if (auto* try_ = curr->dynCast()) { + // It is neither valid for a pop to appear inside a try body (or any + // other control flow structure that will be emitted in the binary), nor + // after the try. self->isInvalid = true; if (!self->stopIfInvalid) { // Do not walk into inner catch bodies; pops in inner catch bodies