New pass: MultiHeap2Local by kripken · Pull Request #9330 · WebAssembly/binaryen

kripken · 2026-10-30T23:03:06Z

This is a form of escape analysis, like Heap2Local, but it can handle multiple
allocations which Heap2Local can't, but otoh those must fit a simple, easy-to
identify pattern.

The pattern here is that while structs are allocated and used, all the uses
are simple struct.gets/sets. So we can just use the data in the fields
directly:

   x = new Foo(42);
   if (condition) x = new Foo(1337);
   else x.value = -1;
   bar(x.value, x.value);

=>

   x_value = (new Foo(42)).value;
   if (condition) x_value = (new Foo(1337)).value;
   else x_value = -1;
   bar(x_value, x_value);

We add a new x_value local here which stores the field, and write to
it from struct.new/struct.set, and read from it in struct.get. If those
are the only operations - no escape of the reference, and no ref.eq
etc. - then this is clearly valid to do. And after doing it, the allocations are
"untangled" - each is used just to write its fields, so their live ranges do
not overlap, and Heap2Local will remove them all. So this new pass,
when it optimizes, always leads to beneficial results, without tradeoffs.

This is a reboot of #9271, with the changes that it handles struct.sets,
and does not handle unknown values (the latter introduced unclear
tradeoffs). As a result the code in the pass is a lot simpler now.

This helps #8641 and almost fixes it. Specifically, once we merge
the two locals there, then this new pass will do all the rest. And we
should be getting those locals merged either through @stevenfontanella's
nullability pass, or my TypeGeneralizing pass.

Aside from helping that original testcase in #8641 (after I manually
merged the locals), this also helps 3/4 large wasm testcases I tried on,
and has no downside on 1/4.

Co-authored-by: Thomas Lively <tlively123@gmail.com>

tlively

It would be good to see if we can get coverage for the remaining uncovered branches as well.

tlively · 2026-11-03T05:35:52Z

+  (func $optimize-struct-get-different-fields (param $x i32) (result i32)
+    ;; Optimize when gets of $ref read different fields (field 0 and field 1).
+    (local $ref (ref null $struct-2fields))
+    (if


Do all the tests need this if-then-else structure? It's definitely important to show we can handle it, but then for some of these other tests, I think a sequence of (drop (struct.get (local.get $x))) and (struct.set (local.get $x)...) would be sufficient.

I guess this is showing that Heap2Local benefits? Again, I think we can cover that in just a couple tests.

A straightline sequence would have dead code, which I think makes it harder to read. That is, the if-else sequence makes it more realistic. Also more robust to possible future code changes.

I'm not sure what you mean about dead code. For example, couldn't this test be simplified to this?

(func $optimize-struct-get-different-fields (param $x i32) (result i32) ;; Optimize when gets of $ref read different fields (field 0 and field 1). (local $ref (ref null $struct-2fields)) (local.set $ref (struct.new_default $struct-2fields) ) (drop (struct.get $struct-2fields 0 (local.get $ref) ) ) (drop (struct.get $struct-2fields 1 (local.get $ref) ) ) )

You need two struct.news, though. Two subsequent ones are dead code.

(We could drop the two struct.gets, I suppose, but it is shorter to do an i32.add as the test does, and more realistic?)

Oh, because otherwise the optimization doesn't trigger? I don't think it's a problem to have dead code in a test case. It's much more concise than having all the if-else boilerplate, including the $x param, etc.

Yes, we only optimize 2 struct.news (as Heap2Local handles the trivial case of 1, as discussed before).

Do you feel strongly here? I guess I don't, but otoh it does seem strictly better to me to have more realistic testcases, and also a variety of things in them, not all just being the absolute minimum for the current state of the code.

I don't feel strongly enough to make you rewrite so many tests :)

tlively · 2026-11-03T05:44:28Z

+      )
+    )
+    (drop
+      (local.get $no)


How is this different from the previous test with a dropped local.get? Is it just that there's also a local.tee in this one?

This test has locals $yes and $no, and optimizes one and not the other. I'll add to the comment This tests that one local does not interfere with another.

kripken · 2026-11-03T16:11:01Z

It would be good to see if we can get coverage for the remaining uncovered branches as well.

Where are you seeing that data? Is the new bot running somewhere I can read its output?

tlively · 2026-11-03T18:22:48Z

Right here in the diff, for example on https://github.com/WebAssembly/binaryen/pull/9330/changes#diff-a0afad61098fa88f25940c27395e9488baa969a1ff63f3d874322ff45164b60eR409.

kripken · 2026-11-03T18:45:36Z

Right here in the diff, for example on https://github.com/WebAssembly/binaryen/pull/9330/changes#diff-a0afad61098fa88f25940c27395e9488baa969a1ff63f3d874322ff45164b60eR409.

Interesting, I see no coverage report there. Do I need to enable something?

kripken · 2026-11-03T18:46:41Z

Oh.. I have to scroll down and hunt for the coverage messages. Kind of inconvenient...

kripken · 2026-11-03T18:57:34Z

+  Index getFieldIndex(Expression* curr) {
+    if (auto* get = curr->dynCast<StructGet>()) {
+      return get->index;
+    } else if (auto* set = curr->dynCast<StructSet>()) {


This is a false positive, as this code path is very much hit! Removing it reaches an abort.

kripken · 2026-11-03T18:58:02Z

+      return set->index;
+    } else {
+      WASM_UNREACHABLE("bad op");
+    }


Also a false positive - that line is unreachable!

kripken · 2026-11-03T19:00:55Z

Added testcases for coverage. tbh I don't think they are really worth it... I guess coverage is good, but they are also trivially true in the code. Also there are 2 false positives from this coverage analysis.

tlively · 2026-11-03T19:01:47Z

There's also a side panel for the coverage annotations you can enable. It's not available at time of writing because the CI has not finished computing the coverage for the latest commits, but the button will be right next to the comment side panel button when it is available.

tlively · 2026-11-03T19:03:33Z

Added testcases for coverage. tbh I don't think they are really worth it... I guess coverage is good, but they are also trivially true in the code. Also there are 2 false positives from this coverage analysis.

I think we should feel free to not add tests when they don't make sense, rather than bending over backward to get to 100% coverage. I hope the annotations are still useful for finding missed edge cases. We should try to eliminate false positives, though. Do you remember what they were?

kripken · 2026-11-03T19:14:48Z

I posted a comment next to each of the 2 false positives.

kripken · 2026-11-03T19:16:15Z

(Looks like the 2 false positives vanished after a new code push, but the comments show which lines and what was going on.)

tlively

LGTM!

kripken · 2026-11-03T22:24:18Z

Oops, I forgot to update the title before merging, so the commit has the old name... but the code that landed has the updated name.

kripken added 30 commits October 15, 2026 14:00

start

66f0b3a

form

e69921a

commentses

324b32e

commentses

8c19fde

work

d39f84e

work

36fed8f

work

549a1f0

work

5414281

work

273f8e2

work

3d9e22f

work

18790e7

work

bbed88f

work

88d62e1

work

63b9bea

work

03bd309

work

110938c

work

b5a8718

work

6da0c54

work

15be564

work

8b0309e

work

ff69080

work

0c2a6d2

fuzz

84bbb61

help

86c90f3

work

934598d

work

931dd17

work

7748a47

work

39bb1c9

work

7c504b3

work

53a7f35

kripken and others added 8 commits November 2, 2026 12:38

Update src/passes/HeapUntangling.cpp

3850442

Co-authored-by: Thomas Lively <tlively123@gmail.com>

Update src/passes/HeapUntangling.cpp

6bf9112

Co-authored-by: Thomas Lively <tlively123@gmail.com>

Update src/passes/HeapUntangling.cpp

ff2b7e8

Co-authored-by: Thomas Lively <tlively123@gmail.com>

format

88a0da9

feedback: add tee to tee example

bd59e5d

Update src/passes/HeapUntangling.cpp

a456ad2

Co-authored-by: Thomas Lively <tlively123@gmail.com>

format

f17c9b1

feedback: save a little work for non-nullable locals

ad37f5e

tlively reviewed Nov 3, 2026

View reviewed changes

add a comment to a test

2a91fb2

kripken added 3 commits November 3, 2026 10:51

add test for nested local.set with another local index

5cbf1ab

add test with a struct.set of an irrelevant ref

cb0684a

test a struct.set of a local.get of another local

963b448

kripken commented Nov 3, 2026

View reviewed changes

rename to MultiHeap2Local

33f8aba

tlively approved these changes Nov 3, 2026

View reviewed changes

kripken merged commit e437de4 into WebAssembly:main Nov 3, 2026
18 checks passed

kripken deleted the psgs branch November 3, 2026 22:23

kripken changed the title ~~New pass: HeapUntangling~~ New pass: MultiHeap2Local Nov 3, 2026

Conversation

kripken commented Oct 30, 2026

Uh oh!

tlively left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

tlively commented Nov 3, 2026

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlively commented Nov 3, 2026

Uh oh!

tlively commented Nov 3, 2026

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

tlively left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kripken commented Nov 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kripken commented Nov 3, 2026 •

edited

Loading