From 24a570cbc07c826b7a3fa2a331e965240a3149eb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 10:34:06 -0700 Subject: [PATCH 1/8] fix --- src/passes/Precompute.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index f3a6e52e8c2..a48827157eb 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -260,6 +260,10 @@ class PrecomputingExpressionRunner struct Precompute : public WalkerPass< PostWalker>> { + + using Super = WalkerPass< + PostWalker>>; + bool isFunctionParallel() override { return true; } std::unique_ptr create() override { @@ -275,6 +279,24 @@ struct Precompute bool canPartiallyPrecompute; + static void scan(Precompute* self, Expression** currp) { + if ((*currp)->is()) { + // On loop entries, clear effectful sets. That mechanism in + // wasm-interpreter is not aware of control flow merges, so a normal + // walk over + // + // x = 10; + // loop { + // use(x); + // x = 20; + // + // would see 10, then apply the 10 in the use. + self->getValues.clear(); + } + + Super::scan(self, currp); + } + void doWalkFunction(Function* func) { // Perform partial precomputing only when the optimization level is non- // trivial, as it is slower and less likely to help. From 0ab709001323800e80150426857fec9153205e49 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 10:34:13 -0700 Subject: [PATCH 2/8] format --- src/passes/Precompute.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index a48827157eb..7da2814ec88 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -261,8 +261,8 @@ struct Precompute : public WalkerPass< PostWalker>> { - using Super = WalkerPass< - PostWalker>>; + using Super = + WalkerPass>>; bool isFunctionParallel() override { return true; } From 9b1582448a87f507c52d6fe0c8bbcdfafd0754f4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 10:36:58 -0700 Subject: [PATCH 3/8] comment --- src/passes/Precompute.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 7da2814ec88..29b924c751a 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -291,6 +291,9 @@ struct Precompute // x = 20; // // would see 10, then apply the 10 in the use. + // TODO: Deprecate the old getValues mechanism, as |propagate| does the + // same thing properly - though we run propagation less, so this + // might regress unless we run the (slower) propagation more. self->getValues.clear(); } From c8c7cce9318a5321fc24085febbe5e2555d83cff Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 10:38:14 -0700 Subject: [PATCH 4/8] test --- test/lit/passes/precompute-gc-loop.wast | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 test/lit/passes/precompute-gc-loop.wast diff --git a/test/lit/passes/precompute-gc-loop.wast b/test/lit/passes/precompute-gc-loop.wast new file mode 100644 index 00000000000..30f1248ecc2 --- /dev/null +++ b/test/lit/passes/precompute-gc-loop.wast @@ -0,0 +1,46 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. + +;; RUN: wasm-opt %s --remove-unused-names --precompute-propagate --fuzz-exec -all -S -o - \ +;; RUN: | filecheck %s + +(module + (type $struct (sub (struct (field i32)))) + + (func $test + (local $struct (ref null $struct)) + (local $int i32) + (local.set $int + (i32.const 0) + ) + (loop $loop + (local.set $struct + (struct.new $struct + (local.get $int) + ) + ) + (local.set $int + (i32.const 1) + ) + (br_if $loop + (i32.eqz + (block $block1 (result i32) + (block $block + (drop + (br_on_null $block + (local.get $struct) + ) + ) + (br $block1 + (struct.get $struct 0 + (local.get $struct) + ) + ) + ) + (i32.const 0) + ) + ) + ) + ) + ) + ) +) From 45e066d58f29ca2a6ca07a41f5eef8905e4b2903 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 11:38:08 -0700 Subject: [PATCH 5/8] fix --- src/passes/Precompute.cpp | 29 ++------ test/lit/passes/precompute-gc-loop.wast | 91 ++++++++++++++++++------- 2 files changed, 74 insertions(+), 46 deletions(-) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 29b924c751a..459b05f037f 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -210,7 +210,13 @@ class PrecomputingExpressionRunner if (hasEffects) { // Visit, so we recompute the effects. (This is rare, see comment // above.) - visitFunc(); + auto flow = visitFunc(); + // Also check the result of the effects - if it is non-constant, we + // cannot use it. (This can happen during propagation, when we see that + // other inputs exist to something we depend on.) + if (flow.breaking()) { + return flow; + } } // Refer to the same canonical GCData that we already created. return Literal(data, curr->type.getHeapType()); @@ -279,27 +285,6 @@ struct Precompute bool canPartiallyPrecompute; - static void scan(Precompute* self, Expression** currp) { - if ((*currp)->is()) { - // On loop entries, clear effectful sets. That mechanism in - // wasm-interpreter is not aware of control flow merges, so a normal - // walk over - // - // x = 10; - // loop { - // use(x); - // x = 20; - // - // would see 10, then apply the 10 in the use. - // TODO: Deprecate the old getValues mechanism, as |propagate| does the - // same thing properly - though we run propagation less, so this - // might regress unless we run the (slower) propagation more. - self->getValues.clear(); - } - - Super::scan(self, currp); - } - void doWalkFunction(Function* func) { // Perform partial precomputing only when the optimization level is non- // trivial, as it is slower and less likely to help. diff --git a/test/lit/passes/precompute-gc-loop.wast b/test/lit/passes/precompute-gc-loop.wast index 30f1248ecc2..d1752f23f5c 100644 --- a/test/lit/passes/precompute-gc-loop.wast +++ b/test/lit/passes/precompute-gc-loop.wast @@ -4,40 +4,83 @@ ;; RUN: | filecheck %s (module + ;; CHECK: (type $struct (sub (struct (field i32)))) (type $struct (sub (struct (field i32)))) - (func $test + ;; CHECK: (func $loop-different (type $1) + ;; CHECK-NEXT: (local $struct (ref null $struct)) + ;; CHECK-NEXT: (local $int i32) + ;; CHECK-NEXT: (local.set $int + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (loop $loop + ;; CHECK-NEXT: (local.set $struct + ;; CHECK-NEXT: (struct.new $struct + ;; CHECK-NEXT: (local.get $int) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $int + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br_if $loop + ;; CHECK-NEXT: (i32.eqz + ;; CHECK-NEXT: (block $block1 (result i32) + ;; CHECK-NEXT: (block $block + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (br_on_null $block + ;; CHECK-NEXT: (local.get $struct) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br $block1 + ;; CHECK-NEXT: (struct.get $struct 0 + ;; CHECK-NEXT: (local.get $struct) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $loop-different (local $struct (ref null $struct)) (local $int i32) + ;; This int is set before the loop, and read at the loop top. The value there + ;; is *not* 0, because we set a new value in the loop, which takes effect in + ;; later iterations. + (local.set $int + (i32.const 0) + ) + (loop $loop + (local.set $struct + (struct.new $struct + (local.get $int) + ) + ) (local.set $int - (i32.const 0) + (i32.const 1) ) - (loop $loop - (local.set $struct - (struct.new $struct - (local.get $int) - ) - ) - (local.set $int - (i32.const 1) - ) - (br_if $loop - (i32.eqz - (block $block1 (result i32) - (block $block - (drop - (br_on_null $block - (local.get $struct) - ) + + ;; The rest of the code is needed to prevent the testcase from getting + ;; optimized trivially away. This br should not be optimized away or + ;; simplfied - it executes in the first loop iteration but not the second. + (br_if $loop + (i32.eqz + (block $block1 (result i32) + (block $block + (drop + (br_on_null $block + (local.get $struct) ) - (br $block1 - (struct.get $struct 0 - (local.get $struct) - ) + ) + (br $block1 + (struct.get $struct 0 + (local.get $struct) ) ) - (i32.const 0) ) + (i32.const 0) ) ) ) From 7bdce2db68a2d5632d0a46c20512e0760d65b5ca Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 11:41:13 -0700 Subject: [PATCH 6/8] fix --- src/passes/Precompute.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 459b05f037f..5c9be05a75b 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -93,9 +93,6 @@ struct HeapValues { // of GetValues computed during the precompute pass. class PrecomputingExpressionRunner : public ConstantExpressionRunner { - - using Super = ConstantExpressionRunner; - // Concrete values of gets computed during the pass, which the runner does not // know about since it only records values of sets it visits. const GetValues& getValues; From 89dcb5ba15a1b7b4ad1124e8741a92244cc368a0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 11:42:45 -0700 Subject: [PATCH 7/8] oop --- src/passes/Precompute.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 5c9be05a75b..459b05f037f 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -93,6 +93,9 @@ struct HeapValues { // of GetValues computed during the precompute pass. class PrecomputingExpressionRunner : public ConstantExpressionRunner { + + using Super = ConstantExpressionRunner; + // Concrete values of gets computed during the pass, which the runner does not // know about since it only records values of sets it visits. const GetValues& getValues; From 8215e83a620ff36c34c47566dbe42c29c641526c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 31 Oct 2025 11:42:55 -0700 Subject: [PATCH 8/8] oop --- src/passes/Precompute.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 459b05f037f..02cf27a26b6 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -266,10 +266,6 @@ class PrecomputingExpressionRunner struct Precompute : public WalkerPass< PostWalker>> { - - using Super = - WalkerPass>>; - bool isFunctionParallel() override { return true; } std::unique_ptr create() override {