From 73b958fefdebc93fc1128b781c821611acb852c8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 11:08:22 -0700 Subject: [PATCH 1/6] code --- src/passes/ConstraintAnalysis.cpp | 33 +++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/passes/ConstraintAnalysis.cpp b/src/passes/ConstraintAnalysis.cpp index fff4e4b8ed3..68823089952 100644 --- a/src/passes/ConstraintAnalysis.cpp +++ b/src/passes/ConstraintAnalysis.cpp @@ -339,7 +339,6 @@ struct ConstraintAnalysis while (!work.empty()) { auto* block = work.pop(); - // Start at the top of the block, then go through, applying things. BasicBlockConstraintMap constraints = block->contents.startConstraints; @@ -570,7 +569,12 @@ struct ConstraintAnalysis // See above on binary action counting limits. if (auto* binary = set->value->dynCast()) { - if (binaryActionCounts[binary]++ >= MaxBinaryActions) { + // The count may exceed the limit sometimes, but add a hard assert on + // never going up so high it is likely doing an unbounded computation. + auto& count = binaryActionCounts[binary]; + assert(count < MaxBinaryActions * 10); + count++; + if (count >= MaxBinaryActions) { constraints.setProvesNothing(set->index); return; } @@ -600,8 +604,29 @@ struct ConstraintAnalysis // opportunity to write any other value while falling through. (And, any // local.tee appearing here would have been reached earlier in the // traversal, and handled.) - auto* value = - Properties::getFallthrough(set->value, getPassOptions(), *getModule()); + // + // We find the first tee in the fallthrough and apply that. This is both + // more efficient - we don't need to look any further - and also it avoids + // a problem where MaxBinaryActions is not properly applied. Imagine that + // we have an increment with a tee in the middle: + // + // (local.set $y (local.tee $x (i32.add (local.get $y) (i32.const 1)))) + // + // If this executes too many times, we will stop calculating $x (see the + // above code). Then $y should just copy $x's state. + auto* value = set->value; + while (1) { + if (value->is()) { + break; + } + auto* next = Properties::getImmediateFallthrough( + value, getPassOptions(), *getModule()); + if (value == next) { + break; + } else { + value = next; + } + } constraints.set(set->index, value); } } From 6aa47b2db7e142c46720e6cc2429247e815648fe Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 11:10:37 -0700 Subject: [PATCH 2/6] test --- .../lit/passes/constraint-analysis-loops.wast | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/lit/passes/constraint-analysis-loops.wast b/test/lit/passes/constraint-analysis-loops.wast index 9c1e594f18e..7c66ee84429 100644 --- a/test/lit/passes/constraint-analysis-loops.wast +++ b/test/lit/passes/constraint-analysis-loops.wast @@ -1905,4 +1905,59 @@ ) ) ) + + ;; CHECK: (func $increment-tee (type $0) + ;; CHECK-NEXT: (local $x i32) + ;; CHECK-NEXT: (local $y i32) + ;; CHECK-NEXT: (loop $label1 + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.gt_s + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $y + ;; CHECK-NEXT: (local.tee $x + ;; CHECK-NEXT: (i32.add + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br $label1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $increment-tee + (local $x i32) + (local $y i32) + ;; A loop, where $y is incremented but there is a tee in the middle. The loop + ;; is unbounded (the exit condition is never hit), so we must be careful to + ;; not keep calculating 1,2,3, without limit. The tee in the middle should not + ;; confuse us: we apply the +=1 operation to x directly, but y reads it + ;; through the tee. We should stop calculating anything about both rather than + ;; hang for a long time. + (loop $label1 + (if + (i32.gt_s + (local.get $y) + (local.get $x) + ) + (then + (unreachable) + ) + ) + (local.set $y + (local.tee $x + (i32.add + (local.get $y) + (i32.const 1) + ) + ) + ) + (br $label1) + ) + ) ) From 1c9db385a6ecc677b6278e77fa7ad777d3405735 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 11:12:21 -0700 Subject: [PATCH 3/6] test --- src/passes/ConstraintAnalysis.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/passes/ConstraintAnalysis.cpp b/src/passes/ConstraintAnalysis.cpp index 68823089952..179eb1256ec 100644 --- a/src/passes/ConstraintAnalysis.cpp +++ b/src/passes/ConstraintAnalysis.cpp @@ -339,6 +339,7 @@ struct ConstraintAnalysis while (!work.empty()) { auto* block = work.pop(); + // Start at the top of the block, then go through, applying things. BasicBlockConstraintMap constraints = block->contents.startConstraints; From 84a669c774c6b07ed050b84998368ef586220360 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 12:31:21 -0700 Subject: [PATCH 4/6] simpler: do fallthrough first --- src/passes/ConstraintAnalysis.cpp | 39 ++++++++++++++----------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/passes/ConstraintAnalysis.cpp b/src/passes/ConstraintAnalysis.cpp index 179eb1256ec..86dd68afdfd 100644 --- a/src/passes/ConstraintAnalysis.cpp +++ b/src/passes/ConstraintAnalysis.cpp @@ -568,19 +568,6 @@ struct ConstraintAnalysis return; } - // See above on binary action counting limits. - if (auto* binary = set->value->dynCast()) { - // The count may exceed the limit sometimes, but add a hard assert on - // never going up so high it is likely doing an unbounded computation. - auto& count = binaryActionCounts[binary]; - assert(count < MaxBinaryActions * 10); - count++; - if (count >= MaxBinaryActions) { - constraints.setProvesNothing(set->index); - return; - } - } - // Look at the fallthrough. It is valid to do so, because our constraints // only track two things, constants and locals. For a constant, it does // not change while falling through. For a local, the only way for the @@ -606,15 +593,8 @@ struct ConstraintAnalysis // local.tee appearing here would have been reached earlier in the // traversal, and handled.) // - // We find the first tee in the fallthrough and apply that. This is both - // more efficient - we don't need to look any further - and also it avoids - // a problem where MaxBinaryActions is not properly applied. Imagine that - // we have an increment with a tee in the middle: - // - // (local.set $y (local.tee $x (i32.add (local.get $y) (i32.const 1)))) - // - // If this executes too many times, we will stop calculating $x (see the - // above code). Then $y should just copy $x's state. + // We find the first tee in the fallthrough and apply that. This is more + // efficient - we don't need to look any further. auto* value = set->value; while (1) { if (value->is()) { @@ -628,6 +608,21 @@ struct ConstraintAnalysis value = next; } } + + // Now that we know the value, check binary action counting limits (see + // above). + if (auto* binary = set->value->dynCast()) { + // The count may exceed the limit sometimes, but add a hard assert on + // never going up so high it is likely doing an unbounded computation. + auto& count = binaryActionCounts[binary]; + assert(count < MaxBinaryActions * 10); + count++; + if (count >= MaxBinaryActions) { + constraints.setProvesNothing(set->index); + return; + } + } + constraints.set(set->index, value); } } From 33f5e6e19cf46bafbb53a15d8245f49594288dac Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 12:32:30 -0700 Subject: [PATCH 5/6] comment --- src/passes/ConstraintAnalysis.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/passes/ConstraintAnalysis.cpp b/src/passes/ConstraintAnalysis.cpp index 86dd68afdfd..20dc7870172 100644 --- a/src/passes/ConstraintAnalysis.cpp +++ b/src/passes/ConstraintAnalysis.cpp @@ -592,12 +592,12 @@ struct ConstraintAnalysis // opportunity to write any other value while falling through. (And, any // local.tee appearing here would have been reached earlier in the // traversal, and handled.) - // - // We find the first tee in the fallthrough and apply that. This is more - // efficient - we don't need to look any further. auto* value = set->value; while (1) { if (value->is()) { + // We stop at the first tee: we don't need to look any further, and + // will just apply that local's values to ourselves, saving repeated + // work. break; } auto* next = Properties::getImmediateFallthrough( From 028c34f08767068e1ceaaee9e25b738a2e22dfc7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 31 Aug 2026 12:33:29 -0700 Subject: [PATCH 6/6] oops --- src/passes/ConstraintAnalysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/ConstraintAnalysis.cpp b/src/passes/ConstraintAnalysis.cpp index 20dc7870172..89a3584487c 100644 --- a/src/passes/ConstraintAnalysis.cpp +++ b/src/passes/ConstraintAnalysis.cpp @@ -611,7 +611,7 @@ struct ConstraintAnalysis // Now that we know the value, check binary action counting limits (see // above). - if (auto* binary = set->value->dynCast()) { + if (auto* binary = value->dynCast()) { // The count may exceed the limit sometimes, but add a hard assert on // never going up so high it is likely doing an unbounded computation. auto& count = binaryActionCounts[binary];