From 15aecadfd15c1a31ce47490e4baeeab84f6ebe07 Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Wed, 25 Mar 2026 23:52:25 +0000 Subject: [PATCH 1/3] Fix for traps never happens --- src/ir/effects.h | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index ee26be2841c..28e850d4c7d 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -287,9 +287,7 @@ class EffectAnalyzer { // // TODO: Go through the optimizer and use this in all places that do not move // code around. - bool hasUnremovableSideEffects() const { - return hasNonTrapSideEffects() || (trap && !trapsNeverHappen); - } + bool hasUnremovableSideEffects() const { return hasSideEffects(); } bool hasAnything() const { return hasSideEffects() || accessesLocal() || readsMutableGlobalState(); @@ -418,17 +416,10 @@ class EffectAnalyzer { // anything. assert(!((trap && other.throws()) || (throws() && other.trap))); // We can't reorder an implicit trap in a way that could alter what global - // state is modified. However, in trapsNeverHappen mode we assume traps do - // not occur in practice, which lets us ignore this, at least in the case - // that the code executes. As mentioned above, we assume that there is no - // transfer of control flow between the things we are comparing, so all we - // need to do is check for such transfers in them. - if (!trapsNeverHappen || transfersControlFlow() || - other.transfersControlFlow()) { - if ((trap && other.writesGlobalState()) || - (other.trap && writesGlobalState())) { - return true; - } + // state is modified. + if ((trap && other.writesGlobalState()) || + (other.trap && writesGlobalState())) { + return true; } return false; } @@ -466,6 +457,12 @@ class EffectAnalyzer { trap = trap || other.trap; implicitTrap = implicitTrap || other.implicitTrap; trapsNeverHappen = trapsNeverHappen || other.trapsNeverHappen; + if (trapsNeverHappen) { + trap = false; + implicitTrap = false; + } else if (ignoreImplicitTraps) { + implicitTrap = false; + } throws_ = throws_ || other.throws_; danglingPop = danglingPop || other.danglingPop; mayNotReturn = mayNotReturn || other.mayNotReturn; @@ -1441,11 +1438,15 @@ class EffectAnalyzer { void post() { assert(tryDepth == 0); - if (ignoreImplicitTraps) { + if (ignoreImplicitTraps || trapsNeverHappen) { implicitTrap = false; } else if (implicitTrap) { trap = true; } + + if (trapsNeverHappen) { + trap = false; + } } }; From b3570abeebaccbab430aca3c8ff7388ee78a6dbf Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Wed, 25 Mar 2026 23:55:11 +0000 Subject: [PATCH 2/3] Update tests --- .../passes/optimize-instructions-desc.wast | 20 +++++++------------ .../passes/optimize-instructions-gc-tnh.wast | 6 +----- test/lit/passes/vacuum-tnh.wast | 6 ++---- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/test/lit/passes/optimize-instructions-desc.wast b/test/lit/passes/optimize-instructions-desc.wast index 151f9e9d32b..66265fa0b7c 100644 --- a/test/lit/passes/optimize-instructions-desc.wast +++ b/test/lit/passes/optimize-instructions-desc.wast @@ -513,11 +513,9 @@ ;; NTRAP: (func $cast-desc-eq-weaker-nondesc-child-effects (type $14) (param $ref anyref) (param $desc (ref $sub.desc)) ;; NTRAP-NEXT: (drop ;; NTRAP-NEXT: (ref.cast_desc_eq (ref $sub) - ;; NTRAP-NEXT: (ref.as_non_null - ;; NTRAP-NEXT: (block (result anyref) - ;; NTRAP-NEXT: (call $effect) - ;; NTRAP-NEXT: (local.get $ref) - ;; NTRAP-NEXT: ) + ;; NTRAP-NEXT: (block (result anyref) + ;; NTRAP-NEXT: (call $effect) + ;; NTRAP-NEXT: (local.get $ref) ;; NTRAP-NEXT: ) ;; NTRAP-NEXT: (block (result (ref $sub.desc)) ;; NTRAP-NEXT: (call $effect) @@ -1407,21 +1405,17 @@ ;; CHECK-NEXT: ) ;; NTRAP: (func $ref.cast_desc_eq-ref.as_non_null (type $4) ;; NTRAP-NEXT: (local $null (ref null $struct)) - ;; NTRAP-NEXT: (local $1 (ref $struct)) - ;; NTRAP-NEXT: (local $2 (ref null $desc)) + ;; NTRAP-NEXT: (local $1 (ref null $desc)) ;; NTRAP-NEXT: (drop ;; NTRAP-NEXT: (block (result (ref $struct)) ;; NTRAP-NEXT: (local.set $1 - ;; NTRAP-NEXT: (ref.as_non_null - ;; NTRAP-NEXT: (local.get $null) - ;; NTRAP-NEXT: ) - ;; NTRAP-NEXT: ) - ;; NTRAP-NEXT: (local.set $2 ;; NTRAP-NEXT: (block (result (ref null $desc)) ;; NTRAP-NEXT: (return) ;; NTRAP-NEXT: ) ;; NTRAP-NEXT: ) - ;; NTRAP-NEXT: (local.get $1) + ;; NTRAP-NEXT: (ref.as_non_null + ;; NTRAP-NEXT: (local.get $null) + ;; NTRAP-NEXT: ) ;; NTRAP-NEXT: ) ;; NTRAP-NEXT: ) ;; NTRAP-NEXT: (drop diff --git a/test/lit/passes/optimize-instructions-gc-tnh.wast b/test/lit/passes/optimize-instructions-gc-tnh.wast index 9323488e336..fce9c147f79 100644 --- a/test/lit/passes/optimize-instructions-gc-tnh.wast +++ b/test/lit/passes/optimize-instructions-gc-tnh.wast @@ -879,11 +879,7 @@ ) ;; TNH: (func $select.unreachable.child.flip (type $6) (param $x (ref $struct)) (result (ref $struct)) - ;; TNH-NEXT: (select - ;; TNH-NEXT: (local.get $x) - ;; TNH-NEXT: (unreachable) - ;; TNH-NEXT: (i32.const 1) - ;; TNH-NEXT: ) + ;; TNH-NEXT: (local.get $x) ;; TNH-NEXT: ) ;; NO_TNH: (func $select.unreachable.child.flip (type $6) (param $x (ref $struct)) (result (ref $struct)) ;; NO_TNH-NEXT: (select diff --git a/test/lit/passes/vacuum-tnh.wast b/test/lit/passes/vacuum-tnh.wast index 89d96a562df..cbd6c193eda 100644 --- a/test/lit/passes/vacuum-tnh.wast +++ b/test/lit/passes/vacuum-tnh.wast @@ -19,9 +19,7 @@ (type $struct (struct (field (mut i32)))) ;; YESTNH: (func $drop (type $4) (param $x i32) (param $y anyref) - ;; YESTNH-NEXT: (drop - ;; YESTNH-NEXT: (unreachable) - ;; YESTNH-NEXT: ) + ;; YESTNH-NEXT: (nop) ;; YESTNH-NEXT: ) ;; NO_TNH: (func $drop (type $4) (param $x i32) (param $y anyref) ;; NO_TNH-NEXT: (drop @@ -184,7 +182,7 @@ ) ;; YESTNH: (func $toplevel (type $0) - ;; YESTNH-NEXT: (unreachable) + ;; YESTNH-NEXT: (nop) ;; YESTNH-NEXT: ) ;; NO_TNH: (func $toplevel (type $0) ;; NO_TNH-NEXT: (unreachable) From c730be13ce1d78450e96dad95bd450cc7eaef1eb Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Thu, 26 Mar 2026 22:28:08 +0000 Subject: [PATCH 3/3] Try to restore earlier logic --- src/ir/effects.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index 28e850d4c7d..0d3e69277b0 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -417,9 +417,11 @@ class EffectAnalyzer { assert(!((trap && other.throws()) || (throws() && other.trap))); // We can't reorder an implicit trap in a way that could alter what global // state is modified. - if ((trap && other.writesGlobalState()) || - (other.trap && writesGlobalState())) { - return true; + if (transfersControlFlow() || other.transfersControlFlow()) { + if ((trap && other.writesGlobalState()) || + (other.trap && writesGlobalState())) { + return true; + } } return false; }