From eac1f8dddc0563875bc7edffb1c94d078b7e67d0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 12:32:10 -0700 Subject: [PATCH 01/10] start --- src/ir/constraint.cpp | 5 +++++ src/ir/constraint.h | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/ir/constraint.cpp b/src/ir/constraint.cpp index 6332f8a5224..787313750a9 100644 --- a/src/ir/constraint.cpp +++ b/src/ir/constraint.cpp @@ -148,6 +148,7 @@ void AndedConstraintSet::approximateAnd(const Constraint& c) { if (size() < MaxConstraints) { push_back(c); + sort(); return; } @@ -184,6 +185,10 @@ void AndedConstraintSet::approximateOr(const AndedConstraintSet& other) { clear(); } +void AndedConstraintSet::sort() { + std::sort(begin(), end()); +} + std::optional LocalConstraint::parse(Expression* curr) { auto parseEqZArgument = [&](Expression* value) -> std::optional { diff --git a/src/ir/constraint.h b/src/ir/constraint.h index 1c91e0d1632..567c53200f7 100644 --- a/src/ir/constraint.h +++ b/src/ir/constraint.h @@ -165,6 +165,12 @@ struct AndedConstraintSet : inplace_vector { setProvesNothing(); push_back(c); } + +private: + // While we are a vector, the order of constraints does not logically matter. + // We keep ourselves sorted in a canonical form, so that simple ==, != etc. + // comparisons work. The canonical order also makes debug printing nicer. + void sort(); }; // A local plus a constraint on it. From 0ed4896276d40d92d037aba51c5679d957c67cd7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 12:49:53 -0700 Subject: [PATCH 02/10] fix --- src/ir/constraint.h | 15 ++++++ src/literal.h | 1 + src/support/parent_index_iterator.h | 16 +++++++ src/wasm-interpreter.h | 3 ++ src/wasm/literal.cpp | 58 ++++++++++++++++++++++++ test/lit/passes/constraint-analysis.wast | 32 +++++++++++++ 6 files changed, 125 insertions(+) diff --git a/src/ir/constraint.h b/src/ir/constraint.h index 567c53200f7..26d11633c59 100644 --- a/src/ir/constraint.h +++ b/src/ir/constraint.h @@ -35,6 +35,15 @@ namespace wasm::constraint { // A term in a constraint, either a local index or literal value. struct Term : public std::variant { bool operator==(const Term&) const = default; + bool operator<(const Term& other) const { + if (index() != other.index()) { + return index() < other.index(); + } + if (index() == 0) { + return std::get(*this) < std::get(other); + } + return std::get(*this) < std::get(other); + } }; // A constraint: some operation and some value, like "is equal to 17" or "is @@ -44,6 +53,12 @@ struct Constraint { Term term; bool operator==(const Constraint&) const = default; + bool operator<(const Constraint& other) const { + if (op != other.op) { + return op < other.op; + } + return term < other.term; + } Constraint negate() const { return Constraint{Abstract::negateRelational(op), term}; diff --git a/src/literal.h b/src/literal.h index e982dae68aa..6f8415aeeb4 100644 --- a/src/literal.h +++ b/src/literal.h @@ -356,6 +356,7 @@ class Literal { // would be equal to itself, if the bits are equal). bool operator==(const Literal& other) const; bool operator!=(const Literal& other) const; + bool operator<(const Literal& other) const; bool isNaN(); bool isCanonicalNaN(); diff --git a/src/support/parent_index_iterator.h b/src/support/parent_index_iterator.h index e19aacfdcfc..8495356eed7 100644 --- a/src/support/parent_index_iterator.h +++ b/src/support/parent_index_iterator.h @@ -55,6 +55,22 @@ template struct ParentIndexIterator { bool operator!=(const ParentIndexIterator& other) const { return !(*this == other); } + bool operator<(const ParentIndexIterator& other) const { + assert(parent == other.parent); + return index < other.index; + } + bool operator<=(const ParentIndexIterator& other) const { + assert(parent == other.parent); + return index <= other.index; + } + bool operator>(const ParentIndexIterator& other) const { + assert(parent == other.parent); + return index > other.index; + } + bool operator>=(const ParentIndexIterator& other) const { + assert(parent == other.parent); + return index >= other.index; + } Iterator& operator++() { ++index; return self(); diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 120fc78bea5..c5c1a33ef17 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -159,6 +159,9 @@ struct FuncData { bool operator==(const FuncData& other) const { return name == other.name && self == other.self; } + bool operator<(const FuncData& other) const { + return std::tie(name, self) < std::tie(other.name, other.self); + } Flow doCall(const Literals& arguments) { assert(call); diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index d20921bccbf..e57103a7608 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -520,6 +520,64 @@ bool Literal::operator!=(const Literal& other) const { return !(*this == other); } +bool Literal::operator<(const Literal& other) const { + if (type != other.type) { + return type.getID() < other.type.getID(); + } + if (type.isBasic()) { + switch (type.getBasic()) { + case Type::none: + return false; + case Type::i32: + case Type::f32: + return i32 < other.i32; + case Type::i64: + case Type::f64: + return i64 < other.i64; + case Type::v128: + return memcmp(v128, other.v128, 16) < 0; + case Type::unreachable: + return false; + } + } else if (type.isRef()) { + assert(type.isRef()); + if (type.isNull()) { + return false; + } + if (type.isFunction()) { + return *funcData < *other.funcData; + } + if (type.isString()) { + return std::lexicographical_compare(gcData->values.begin(), + gcData->values.end(), + other.gcData->values.begin(), + other.gcData->values.end()); + } + if (type.isData()) { + return gcData < other.gcData; + } + auto heapType = type.getHeapType(); + assert(heapType.isBasic()); + if (heapType.isMaybeShared(HeapType::i31)) { + return i32 < other.i32; + } + if (heapType.isMaybeShared(HeapType::ext)) { + if (hasExternPayload() != other.hasExternPayload()) { + return hasExternPayload() < other.hasExternPayload(); + } + if (hasExternPayload()) { + return getExternPayload() < other.getExternPayload(); + } + return internalize() < other.internalize(); + } + if (heapType.isMaybeShared(HeapType::any)) { + return externalize() < other.externalize(); + } + WASM_UNREACHABLE("unexpected type"); + } + WASM_UNREACHABLE("unexpected type"); +} + bool Literal::isNaN() { if (type == Type::f32 && std::isnan(getf32())) { return true; diff --git a/test/lit/passes/constraint-analysis.wast b/test/lit/passes/constraint-analysis.wast index b04c2b2a3e6..dfc32e7ee0d 100644 --- a/test/lit/passes/constraint-analysis.wast +++ b/test/lit/passes/constraint-analysis.wast @@ -3529,5 +3529,37 @@ (then) ) ) + + (func $iloop (param $0 f32) + ;; Regression test for an infinite loop: the specific cfg here + the + ;; constraints lead to a situation where, if we were not careful, we would + ;; think we have an infinite stream of updates in flow(). Specifically, we + ;; end up updating a location to a combination of two constraints {A, B} and + ;; then end up finding {B, A} in the next cycle, and then alternate those + ;; two forever. This is fixed by sorting the constraints. + (local $1 f32) + (local.set $0 + (local.get $1) + ) + (if + (i32.const 0) + (then + (loop + (local.set $1 + (local.get $0) + ) + ) + ) + (else + ) + ) + (loop $label + (loop + ) + (br_if $label + (i32.const 0) + ) + ) + ) ) From ebaf4ce2fe5a0c88a4ab821cb312f4bc9233d27a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 12:50:07 -0700 Subject: [PATCH 03/10] fix --- src/ir/constraint.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ir/constraint.cpp b/src/ir/constraint.cpp index 787313750a9..432d5212bda 100644 --- a/src/ir/constraint.cpp +++ b/src/ir/constraint.cpp @@ -185,9 +185,7 @@ void AndedConstraintSet::approximateOr(const AndedConstraintSet& other) { clear(); } -void AndedConstraintSet::sort() { - std::sort(begin(), end()); -} +void AndedConstraintSet::sort() { std::sort(begin(), end()); } std::optional LocalConstraint::parse(Expression* curr) { auto parseEqZArgument = From efec239c82f01ea024918da7843bce6a078c9154 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 12:54:49 -0700 Subject: [PATCH 04/10] fix --- src/wasm/literal.cpp | 69 ++++++++++++------------ test/lit/passes/constraint-analysis.wast | 52 ++++++++++++++++++ 2 files changed, 86 insertions(+), 35 deletions(-) diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index e57103a7608..6c590076598 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -524,6 +524,7 @@ bool Literal::operator<(const Literal& other) const { if (type != other.type) { return type.getID() < other.type.getID(); } + if (type.isBasic()) { switch (type.getBasic()) { case Type::none: @@ -537,45 +538,43 @@ bool Literal::operator<(const Literal& other) const { case Type::v128: return memcmp(v128, other.v128, 16) < 0; case Type::unreachable: - return false; - } - } else if (type.isRef()) { - assert(type.isRef()); - if (type.isNull()) { - return false; - } - if (type.isFunction()) { - return *funcData < *other.funcData; - } - if (type.isString()) { - return std::lexicographical_compare(gcData->values.begin(), - gcData->values.end(), - other.gcData->values.begin(), - other.gcData->values.end()); - } - if (type.isData()) { - return gcData < other.gcData; + WASM_UNREACHABLE("invalid literal type"); } - auto heapType = type.getHeapType(); - assert(heapType.isBasic()); - if (heapType.isMaybeShared(HeapType::i31)) { - return i32 < other.i32; - } - if (heapType.isMaybeShared(HeapType::ext)) { - if (hasExternPayload() != other.hasExternPayload()) { - return hasExternPayload() < other.hasExternPayload(); - } - if (hasExternPayload()) { - return getExternPayload() < other.getExternPayload(); - } - return internalize() < other.internalize(); + } + + assert(type.isRef()); + if (type.isNull()) { + // Pick nulls as the lowest. + return true; + } + if (type.isFunction()) { + return *funcData < *other.funcData; + } + if (type.isString()) { + return std::lexicographical_compare(gcData->values.begin(), + gcData->values.end(), + other.gcData->values.begin(), + other.gcData->values.end()); + } + if (type.isData()) { + return gcData < other.gcData; + } + auto heapType = type.getHeapType(); + assert(heapType.isBasic()); + if (heapType.isMaybeShared(HeapType::i31)) { + return i32 < other.i32; + } + if (heapType.isMaybeShared(HeapType::ext)) { + if (hasExternPayload() != other.hasExternPayload()) { + return hasExternPayload() < other.hasExternPayload(); } - if (heapType.isMaybeShared(HeapType::any)) { - return externalize() < other.externalize(); + if (hasExternPayload()) { + return getExternPayload() < other.getExternPayload(); } - WASM_UNREACHABLE("unexpected type"); + return internalize() < other.internalize(); } - WASM_UNREACHABLE("unexpected type"); + assert(heapType.isMaybeShared(HeapType::any)); + return externalize() < other.externalize(); } bool Literal::isNaN() { diff --git a/test/lit/passes/constraint-analysis.wast b/test/lit/passes/constraint-analysis.wast index dfc32e7ee0d..fb8fd74b856 100644 --- a/test/lit/passes/constraint-analysis.wast +++ b/test/lit/passes/constraint-analysis.wast @@ -3530,6 +3530,56 @@ ) ) + ;; CHECK: (func $iloop (type $8) (param $0 f32) + ;; CHECK-NEXT: (local $1 f32) + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (local.get $1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (loop + ;; CHECK-NEXT: (local.set $1 + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (loop $label + ;; CHECK-NEXT: (loop + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br_if $label + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; OPTIN: (func $iloop (type $8) (param $0 f32) + ;; OPTIN-NEXT: (local $1 f32) + ;; OPTIN-NEXT: (local.set $0 + ;; OPTIN-NEXT: (local.get $1) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: (if + ;; OPTIN-NEXT: (i32.const 0) + ;; OPTIN-NEXT: (then + ;; OPTIN-NEXT: (loop + ;; OPTIN-NEXT: (local.set $1 + ;; OPTIN-NEXT: (local.get $0) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: (else + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: (loop $label + ;; OPTIN-NEXT: (loop + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: (br_if $label + ;; OPTIN-NEXT: (i32.const 0) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: ) + ;; OPTIN-NEXT: ) (func $iloop (param $0 f32) ;; Regression test for an infinite loop: the specific cfg here + the ;; constraints lead to a situation where, if we were not careful, we would @@ -3537,6 +3587,8 @@ ;; end up updating a location to a combination of two constraints {A, B} and ;; then end up finding {B, A} in the next cycle, and then alternate those ;; two forever. This is fixed by sorting the constraints. + ;; + ;; (There is nothing to optimize here, we just should not hang or error.) (local $1 f32) (local.set $0 (local.get $1) From 7c27fe6972070a84bd9035ce3254b11d8a146faf Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 15:24:51 -0700 Subject: [PATCH 05/10] oops --- src/wasm/literal.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 6c590076598..9af900d5558 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -544,8 +544,8 @@ bool Literal::operator<(const Literal& other) const { assert(type.isRef()); if (type.isNull()) { - // Pick nulls as the lowest. - return true; + // All nulls are equal, and hence not < + return false; } if (type.isFunction()) { return *funcData < *other.funcData; From c92f2fafe9ea73d8411b42b99d9629b7195bb45e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Mon, 13 Jul 2026 15:53:17 -0700 Subject: [PATCH 06/10] simpl --- src/wasm/literal.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 9af900d5558..a5abca04f7c 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -550,13 +550,7 @@ bool Literal::operator<(const Literal& other) const { if (type.isFunction()) { return *funcData < *other.funcData; } - if (type.isString()) { - return std::lexicographical_compare(gcData->values.begin(), - gcData->values.end(), - other.gcData->values.begin(), - other.gcData->values.end()); - } - if (type.isData()) { + if (type.isData() || type.isString()) { return gcData < other.gcData; } auto heapType = type.getHeapType(); From 16a16e484e75d9429e56609893ddb8b36617dce5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 14 Jul 2026 09:03:52 -0700 Subject: [PATCH 07/10] upper_bound+insert --- src/ir/constraint.cpp | 6 ++---- src/ir/constraint.h | 10 ++++------ src/support/inplace_vector.h | 15 +++++++++++++++ test/gtest/inplace_vector.cpp | 27 +++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/ir/constraint.cpp b/src/ir/constraint.cpp index 432d5212bda..1c4a2817dc6 100644 --- a/src/ir/constraint.cpp +++ b/src/ir/constraint.cpp @@ -147,8 +147,8 @@ void AndedConstraintSet::approximateAnd(const Constraint& c) { } if (size() < MaxConstraints) { - push_back(c); - sort(); + // Insert into the right place, keeping us sorted. + insert(std::upper_bound(begin(), end(), c), c); return; } @@ -185,8 +185,6 @@ void AndedConstraintSet::approximateOr(const AndedConstraintSet& other) { clear(); } -void AndedConstraintSet::sort() { std::sort(begin(), end()); } - std::optional LocalConstraint::parse(Expression* curr) { auto parseEqZArgument = [&](Expression* value) -> std::optional { diff --git a/src/ir/constraint.h b/src/ir/constraint.h index 26d11633c59..e3be8305268 100644 --- a/src/ir/constraint.h +++ b/src/ir/constraint.h @@ -78,6 +78,10 @@ enum Result { True, False, Unknown }; // the comments below, `x` is used for the thing all the constraints are talking // about, which looks like a local, but it could be a global or a struct field // or anything else in general. +// +// While we are a vector, the order of constraints does not logically matter, +// and we keep ourselves sorted in a canonical form, so that simple ==, != etc. +// comparisons work. The canonical order also makes debug printing nicer. struct AndedConstraintSet : inplace_vector { // We could represent a contradiction using two constraints that contradict // each other (== 0 && != 0), but for simplicity we mark this explicitly. @@ -180,12 +184,6 @@ struct AndedConstraintSet : inplace_vector { setProvesNothing(); push_back(c); } - -private: - // While we are a vector, the order of constraints does not logically matter. - // We keep ourselves sorted in a canonical form, so that simple ==, != etc. - // comparisons work. The canonical order also makes debug printing nicer. - void sort(); }; // A local plus a constraint on it. diff --git a/src/support/inplace_vector.h b/src/support/inplace_vector.h index 09b8d3abf15..da365ce7fb3 100644 --- a/src/support/inplace_vector.h +++ b/src/support/inplace_vector.h @@ -151,6 +151,9 @@ template class inplace_vector { ConstIterator(const inplace_vector* parent, size_t index) : wasm::ParentIndexIterator*, ConstIterator>{ parent, index} {} + ConstIterator(const Iterator& other) + : wasm::ParentIndexIterator*, ConstIterator>{ + other.parent, other.index} {} ConstIterator(const ConstIterator& other) = default; const T& operator*() const { return (*this->parent)[this->index]; } @@ -162,6 +165,18 @@ template class inplace_vector { ConstIterator begin() const { return ConstIterator(this, 0); } ConstIterator end() const { return ConstIterator(this, size()); } + Iterator insert(ConstIterator pos, const T& x) { + assert(usedFixed < N); + assert(pos.index <= usedFixed); + size_t index = pos.index; + std::move_backward(fixed.begin() + index, + fixed.begin() + usedFixed, + fixed.begin() + usedFixed + 1); + fixed[index] = x; + usedFixed++; + return Iterator(this, index); + } + Iterator erase(ConstIterator first, ConstIterator last) { assert(first.index <= last.index); assert(last.index <= usedFixed); diff --git a/test/gtest/inplace_vector.cpp b/test/gtest/inplace_vector.cpp index 0480bc9f79f..23e535dedaf 100644 --- a/test/gtest/inplace_vector.cpp +++ b/test/gtest/inplace_vector.cpp @@ -84,3 +84,30 @@ TEST_F(InplaceVectorTest, EraseIf) { EXPECT_EQ(erased, 3u); EXPECT_TRUE(vec.empty()); } + +TEST_F(InplaceVectorTest, Insert) { + inplace_vector vec{10, 30, 40}; + + // Insert single element in middle (20 at index 1) + auto it = vec.insert(vec.begin() + 1, 20); + EXPECT_EQ(*it, 20); + EXPECT_EQ(vec.size(), 4u); + EXPECT_EQ(vec[0], 10); + EXPECT_EQ(vec[1], 20); + EXPECT_EQ(vec[2], 30); + EXPECT_EQ(vec[3], 40); + + // Insert at beginning (5 at index 0) + it = vec.insert(vec.begin(), 5); + EXPECT_EQ(*it, 5); + EXPECT_EQ(vec.size(), 5u); + EXPECT_EQ(vec[0], 5); + EXPECT_EQ(vec[1], 10); + + // Insert at end (50 at index 5) + it = vec.insert(vec.end(), 50); + EXPECT_EQ(*it, 50); + EXPECT_EQ(vec.size(), 6u); + EXPECT_EQ(vec[5], 50); +} + From bf1f5593ad690cde4e897b4fa9a7ecf4c7a1c200 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 14 Jul 2026 09:05:50 -0700 Subject: [PATCH 08/10] comment --- src/ir/constraint.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ir/constraint.cpp b/src/ir/constraint.cpp index 1c4a2817dc6..a55a77d6889 100644 --- a/src/ir/constraint.cpp +++ b/src/ir/constraint.cpp @@ -154,7 +154,9 @@ void AndedConstraintSet::approximateAnd(const Constraint& c) { // Otherwise, just do not add this one. // TODO: We could try to be clever and see if one of the existing ones makes - // more sense to drop. + // more sense to drop. In particular, we should prefer "better" ones + // like > over >= and so forth (sorting more precise ones earlier may be + // useful to implement that). } void AndedConstraintSet::approximateOr(const AndedConstraintSet& other) { From 6074460a23fa34f0af4876391779edcd977abb06 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 14 Jul 2026 09:07:20 -0700 Subject: [PATCH 09/10] comment --- src/wasm/literal.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index a5abca04f7c..a44a1c6e906 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -522,6 +522,8 @@ bool Literal::operator!=(const Literal& other) const { bool Literal::operator<(const Literal& other) const { if (type != other.type) { + // This is not deterministic between runs, and also FuncData, below. If this + // matters some day, we would need to find a stable way to compute it. return type.getID() < other.type.getID(); } From 8f510a23ab4054bf885d60290d49baacf07c48dd Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 14 Jul 2026 12:58:33 -0700 Subject: [PATCH 10/10] format --- test/gtest/inplace_vector.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/gtest/inplace_vector.cpp b/test/gtest/inplace_vector.cpp index 23e535dedaf..0f477f0f624 100644 --- a/test/gtest/inplace_vector.cpp +++ b/test/gtest/inplace_vector.cpp @@ -110,4 +110,3 @@ TEST_F(InplaceVectorTest, Insert) { EXPECT_EQ(vec.size(), 6u); EXPECT_EQ(vec[5], 50); } -