From 83db657dcf197279fe1bf808556b065254da8dce Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 1 Sep 2026 14:24:44 -0700 Subject: [PATCH 1/4] go --- src/passes/OptimizeInstructions.cpp | 11 ++++ .../passes/optimize-instructions-desc.wast | 65 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index aa093530543..c3afb3e512b 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -2552,6 +2552,17 @@ struct OptimizeInstructions bool notWeaker = Type::isSubType(curr->type, child->type); bool safe = !child->desc || getPassOptions().trapsNeverHappen; if (notWeaker && safe) { + if (curr->desc) { + // There is another child here, whose effects we must consider (the + // same ordering situation as in skipNonNullCast: we want to move a + // trap past later children). + auto& options = getPassOptions(); + EffectAnalyzer descEffects(options, *getModule(), curr->desc); + ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref); + if (movingEffects.orderedBefore(descEffects)) { + return; + } + } if (child->desc) { // Reorder the child's reference past its dropped descriptor if // necessary. diff --git a/test/lit/passes/optimize-instructions-desc.wast b/test/lit/passes/optimize-instructions-desc.wast index c06764dccb1..d25fa2dbe44 100644 --- a/test/lit/passes/optimize-instructions-desc.wast +++ b/test/lit/passes/optimize-instructions-desc.wast @@ -1459,4 +1459,69 @@ ) ) ) + + ;; CHECK: (func $ref.cast_desc_eq-ref.cast (type $22) (param $x anyref) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.cast_desc_eq (ref $struct) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (block (result (ref null $desc)) + ;; CHECK-NEXT: (return) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.cast_desc_eq (ref (exact $struct)) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (struct.new_default $desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; NTRAP: (func $ref.cast_desc_eq-ref.cast (type $22) (param $x anyref) + ;; NTRAP-NEXT: (local $1 (ref $struct)) + ;; NTRAP-NEXT: (local $2 (ref null $desc)) + ;; NTRAP-NEXT: (drop + ;; NTRAP-NEXT: (block (result (ref $struct)) + ;; NTRAP-NEXT: (local.set $1 + ;; NTRAP-NEXT: (ref.cast (ref $struct) + ;; NTRAP-NEXT: (local.get $x) + ;; 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: ) + ;; NTRAP-NEXT: ) + ;; NTRAP-NEXT: (drop + ;; NTRAP-NEXT: (ref.cast_desc_eq (ref (exact $struct)) + ;; NTRAP-NEXT: (local.get $x) + ;; NTRAP-NEXT: (struct.new_default $desc) + ;; NTRAP-NEXT: ) + ;; NTRAP-NEXT: ) + ;; NTRAP-NEXT: ) + (func $ref.cast_desc_eq-ref.cast (param $x anyref) + ;; As above with ref.as_non_null, removing the inner ref.cast would allow + ;; reaching the return before the cast check. + (drop + (ref.cast_desc_eq (ref $struct) + (ref.cast (ref $struct) + (local.get $x) + ) + (block (result (ref null $desc)) + (return) + ) + ) + ) + ;; Without dangerous effects we can remove the inner cast. + (drop + (ref.cast_desc_eq (ref $struct) + (ref.cast (ref $struct) + (local.get $x) + ) + (struct.new $desc) + ) + ) + ) ) From d8211a3b485dff81e18d04dfa8c2e17fb863cb0c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 1 Sep 2026 14:31:42 -0700 Subject: [PATCH 2/4] fix --- test/lit/passes/optimize-instructions-desc.wast | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/lit/passes/optimize-instructions-desc.wast b/test/lit/passes/optimize-instructions-desc.wast index d25fa2dbe44..3586f230dc6 100644 --- a/test/lit/passes/optimize-instructions-desc.wast +++ b/test/lit/passes/optimize-instructions-desc.wast @@ -1463,7 +1463,9 @@ ;; CHECK: (func $ref.cast_desc_eq-ref.cast (type $22) (param $x anyref) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (ref.cast_desc_eq (ref $struct) - ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: (ref.cast (ref $struct) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (block (result (ref null $desc)) ;; CHECK-NEXT: (return) ;; CHECK-NEXT: ) @@ -1503,7 +1505,8 @@ ;; NTRAP-NEXT: ) (func $ref.cast_desc_eq-ref.cast (param $x anyref) ;; As above with ref.as_non_null, removing the inner ref.cast would allow - ;; reaching the return before the cast check. + ;; reaching the return before the cast check, so we do not optimize. (In + ;; NTRAP mode we end up removing the outer cast, separately.) (drop (ref.cast_desc_eq (ref $struct) (ref.cast (ref $struct) @@ -1520,7 +1523,7 @@ (ref.cast (ref $struct) (local.get $x) ) - (struct.new $desc) + (struct.new $desc) ;; this has no effects ) ) ) From 5c0b0a4a13e81783ad70c84b542d1f4b5a254384 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 1 Sep 2026 15:59:01 -0700 Subject: [PATCH 3/4] expand comment --- src/passes/OptimizeInstructions.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index c3afb3e512b..5796149d31a 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -2555,7 +2555,9 @@ struct OptimizeInstructions if (curr->desc) { // There is another child here, whose effects we must consider (the // same ordering situation as in skipNonNullCast: we want to move a - // trap past later children). + // trap past later children. That is, we are removing a cast, with the + // result that if it trapped, the later cast will still trap, so the + // trap is moving. And it must not move past effectful things). auto& options = getPassOptions(); EffectAnalyzer descEffects(options, *getModule(), curr->desc); ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref); From 4e09eb00e668c75bd3aa829361147bf0694e6772 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 1 Sep 2026 16:07:38 -0700 Subject: [PATCH 4/4] reorder as suggested --- src/passes/OptimizeInstructions.cpp | 45 +++++++++++++++++++---------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp index 5796149d31a..dd3491a669c 100644 --- a/src/passes/OptimizeInstructions.cpp +++ b/src/passes/OptimizeInstructions.cpp @@ -2550,26 +2550,41 @@ struct OptimizeInstructions // traps are allowed, then we cannot remove the potentially-trapping // child, though. bool notWeaker = Type::isSubType(curr->type, child->type); - bool safe = !child->desc || getPassOptions().trapsNeverHappen; - if (notWeaker && safe) { - if (curr->desc) { - // There is another child here, whose effects we must consider (the - // same ordering situation as in skipNonNullCast: we want to move a - // trap past later children. That is, we are removing a cast, with the - // result that if it trapped, the later cast will still trap, so the - // trap is moving. And it must not move past effectful things). - auto& options = getPassOptions(); - EffectAnalyzer descEffects(options, *getModule(), curr->desc); - ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref); - if (movingEffects.orderedBefore(descEffects)) { - return; - } + auto& options = getPassOptions(); + auto canTrap = !options.trapsNeverHappen; + bool safe = !child->desc || !canTrap; + bool canOptimize = notWeaker && safe; + if (canOptimize && curr->desc && canTrap) { + // There is another child here, which might trap, and we need to + // consider that in this situation: + // + // (outer.cast + // (inner.cast (inner.ref)) + // (descriptor with effects) + // ) + // + // => + // + // (outer.cast + // (inner.ref) ;; inner cast was removed + // (descriptor with effects) + // ) + // + // It is safe to remove the inner cast, as if it trapped, the outer one + // would still trap. But if there is a descriptor, then we are moving + // the trap across the descriptor, and shouldn't cross effects there. + EffectAnalyzer descEffects(options, *getModule(), curr->desc); + ShallowEffectAnalyzer movingEffects(options, *getModule(), curr->ref); + if (movingEffects.orderedBefore(descEffects)) { + canOptimize = false; } + } + if (canOptimize) { if (child->desc) { // Reorder the child's reference past its dropped descriptor if // necessary. auto* block = - ChildLocalizer(child, getFunction(), *getModule(), getPassOptions()) + ChildLocalizer(child, getFunction(), *getModule(), options) .getChildrenReplacement(); block->list.push_back(child->ref); block->type = child->ref->type;