Add a --constant-arrays-to-data pass by gkdn · Pull Request #9274 · WebAssembly/binaryen

gkdn · 2026-10-17T04:35:21Z

Follow-up to #9248: a late pass that converts array.new_fixed of constant
numeric values in function bodies to array.new_data when that is smaller
(array.new_fixed needs a full *.const per element, a data segment stores raw
bytes).

Since array.new_fixed is a constant expression that other optimizations
understand, the default pipeline runs this only in the last set of opts
(lastOpts, #9238).

Global initializers are left alone, as array.new_data is not a constant
expression there.
Converted payloads go into one new passive segment. Identical payloads are
stored once, and a payload identical to an existing, never-dropped passive
segment reuses that segment.
A group of identical payloads is converted only if the estimated binary size
is smaller, and the new segment is only added if the savings that depend on it
pay for its header.
The segment is never dropped and all reads are in bounds, so the result
cannot trap.

Note that this does not make it safe to raise the 64-element cap in
OptimizeInstructions: the conversion is not always reversible. A larger
array.new_fixed can be hoisted into a global initializer, where it cannot be
turned back into array.new_data. On a large J2Wasm binary, removing the cap
moved the remaining big tables into globals and grew the binary (+0.9 KB raw,
+1.4 KB gzipped), which this pass cannot recover.

Convert array.new_fixed of constant numeric values in function bodies to array.new_data when that is smaller in the binary. array.new_fixed needs a full *.const per element, while a data segment stores raw bytes. As array.new_fixed is a constant expression that other optimizations understand, this is only done in the last set of default optimizations (lastOpts). All converted payloads go into a single new passive segment. Identical payloads are stored once, and payloads that already appear in the new segment or in an existing passive segment that is never dropped reuse those bytes.

* Copy debug locations to the new array.new_data. * Use FindAllPointers and FindAll in the parallel analysis (which now also collects the dropped segments), Builder::makeDataSegment, and the LEB writers to measure sizes. * Only add the new segment if the conversions that use it pay for its header, and still reuse existing segments otherwise. * Reuse existing segments only if they are identical to the payload, to avoid a quadratic substring search (left as a TODO, along with array.new_elem). * Check all module items in the lit tests (--all-items), including the new segment's contents and debug locations.

* Move the size estimates into helpers: getArrayNewFixedSize, getArrayNewDataSize and getNewSegmentOverhead. * Split run() into findCandidates, groupByPayload, decide and replace. * Build payloads as std::vector<char> (like DataSegment::data) with getBits and memcpy, as the interpreter does, instead of casting. No functional change.

* Order the pass top-down: run, then each step in the order it is called, with the helpers of planConversions after it. * Rename Group to PayloadGroup, Decision to Conversion, decide to planConversions, replace to applyConversions, and addSegment to addNewSegment. Store the array in Candidate instead of a get() method. * Plan without modifying the module: the new segment's contents are gathered in newSegmentData, and run() adds the segment. * Comment the non-obvious parts: why unreachable, empty and non-numeric arrays are skipped, why dropped segments cannot be reused, why the result cannot trap, pointer stability of the candidates, and why the new segment's cost is checked after all groups. No functional change.

gemini-code-assist

Code Review

This pull request introduces the --constant-arrays-to-data optimization pass, which converts array.new_fixed instructions of constant numeric values into array.new_data from passive data segments when it reduces binary size. The pass is integrated into the end of the default optimization pipeline. The review feedback suggests avoiding registering the pass if the bulk-memory feature is disabled, and recommends replacing functional-style casts with explicit static_cast calls to avoid potential narrowing warnings on 64-bit platforms and conform to modern C++ practices.

gemini-code-assist · 2026-10-17T04:37:54Z

+  if (options.lastOpts && wasm->features.hasGC() &&
+      (options.optimizeLevel >= 2 || options.shrinkLevel >= 1)) {


Since the constant-arrays-to-data pass requires the bulk-memory feature (as checked in ConstantArraysToData::run), we can avoid adding the pass to the runner entirely if bulk-memory is not enabled. This saves a small amount of overhead.

Suggested change

if (options.lastOpts && wasm->features.hasGC() &&

(options.optimizeLevel >= 2 || options.shrinkLevel >= 1)) {

if (options.lastOpts && wasm->features.hasGC() && wasm->features.hasBulkMemory() &&

(options.optimizeLevel >= 2 || options.shrinkLevel >= 1)) {

gemini-code-assist · 2026-10-17T04:37:55Z

+  return getWrittenSize(U32LEB(segment)) + 1 +
+         getWrittenSize(S32LEB(int32_t(offset))) + 1 +
+         getWrittenSize(S32LEB(int32_t(size)));


Using C-style or functional-style casts like int32_t(offset) is discouraged in modern C++. It is safer and more idiomatic to use static_cast<int32_t> per the Google C++ Style Guide.

Suggested change

return getWrittenSize(U32LEB(segment)) + 1 +

getWrittenSize(S32LEB(int32_t(offset))) + 1 +

getWrittenSize(S32LEB(int32_t(size)));

return getWrittenSize(U32LEB(segment)) + 1 +

getWrittenSize(S32LEB(static_cast<int32_t>(offset))) + 1 +

getWrittenSize(S32LEB(static_cast<int32_t>(size)));

gemini-code-assist · 2026-10-17T04:37:55Z

+      auto savings = getCodeSavings(group, newSegmentIndex, offset) -
+                     int64_t(group.payload.size());


Using C-style or functional-style casts like int64_t(group.payload.size()) is discouraged in modern C++. It is safer and more idiomatic to use static_cast<int64_t> per the Google C++ Style Guide.

Suggested change

auto savings = getCodeSavings(group, newSegmentIndex, offset) -

int64_t(group.payload.size());

auto savings = getCodeSavings(group, newSegmentIndex, offset) -

static_cast<int64_t>(group.payload.size());

gemini-code-assist · 2026-10-17T04:37:55Z

+        newSegmentSavings <=
+          int64_t(getNewSegmentOverhead(*module, newSegmentData.size()))) {


Using C-style or functional-style casts like int64_t(...) is discouraged in modern C++. It is safer and more idiomatic to use static_cast<int64_t> per the Google C++ Style Guide.

Suggested change

newSegmentSavings <=

int64_t(getNewSegmentOverhead(*module, newSegmentData.size()))) {

newSegmentSavings <=

static_cast<int64_t>(getNewSegmentOverhead(*module, newSegmentData.size()))) {

gemini-code-assist · 2026-10-17T04:37:55Z

+      savings +=
+        int64_t(getArrayNewFixedSize(array)) -
+        int64_t(getArrayNewDataSize(segment, offset, array->values.size()));


Using C-style or functional-style casts like int64_t(...) is discouraged in modern C++. It is safer and more idiomatic to use static_cast<int64_t> per the Google C++ Style Guide.

Suggested change

savings +=

int64_t(getArrayNewFixedSize(array)) -

int64_t(getArrayNewDataSize(segment, offset, array->values.size()));

savings +=

static_cast<int64_t>(getArrayNewFixedSize(array)) -

static_cast<int64_t>(getArrayNewDataSize(segment, offset, array->values.size()));

gemini-code-assist · 2026-10-17T04:37:55Z

+          builder.makeConst(int32_t(conversion.offset)),
+          builder.makeConst(int32_t(array->values.size())));


Using C-style or functional-style casts like int32_t(...) is discouraged in modern C++. It is safer and more idiomatic to use static_cast<int32_t> per the Google C++ Style Guide.

Suggested change

builder.makeConst(int32_t(conversion.offset)),

builder.makeConst(int32_t(array->values.size())));

builder.makeConst(static_cast<int32_t>(conversion.offset)),

builder.makeConst(static_cast<int32_t>(array->values.size())));

tlively · 2026-10-19T19:34:51Z

+// TODO: Also reuse payloads that appear inside other data. That needs an
+//       efficient substring search, to avoid quadratic behavior.


A local AI review noticed that without this TODO, this pass will generate duplicate data segments when run with --converge because lastOpts is true at the end of every convergence iteration and previous OptimizeInstructions runs will always give this pass more work to do. @kripken, maybe we should consider setting lastOpts to be true only after convergence has been achieved?

Since we already calculate reusableSegments, we could also avoid this problem by emitting a separate new segment for each group, rather than a single concatenated segment.

Yes, we need to decide on what lastOpts means if converge is run, I guess. Possibly we should just disable it? It keeps running until a condition, and it checks the condition after running the passes, so it never knows which opts are last until later.

Never running it would be unfortunate because then passes like this one would never be run. I would lean toward either doing one extra iteration after convergence with lastOpts is true, or keeping the status quo and making sure round trips don't keep adding more cruft.

Yeah, a single last iteration might be better, good point.

tlively · 2026-10-19T21:09:12Z

+  // the given segment and offset. The arrays may have different types, so we
+  // measure each one.


How do the different types matter here?

* Give each new payload its own segment instead of concatenating them. With --converge, lastOpts is set in every iteration, so arrays that OptimizeInstructions lowers back to array.new_fixed were converted again, duplicating their bytes in a new segment. Now they reuse their old segment if it is still alive (it is identical to the payload), or replace it if it was removed. Add a test for that. * Find candidates and data.drops in one walk, and use an InsertOrderedMap to gather the results in function order. * Keep only the location in Candidate, and rename module to wasm. * Explain why arrays with identical payloads are measured separately. * In the test, move the global up, use hex for packed values and describe their bytes, and remove the unneeded global getter.

gkdn · 2026-10-20T03:09:35Z

@tlively should I follow the above feedback from gemini?

tlively · 2026-10-20T04:14:23Z

No, you can ignore that if you'd like. Either way is fine by me.

kripken · 2026-10-20T16:15:10Z

+  return getWrittenSize(U32LEB(segment)) + 2 + 1 +
+         getWrittenSize(S32LEB(int32_t(size)));


Suggested change

return getWrittenSize(U32LEB(segment)) + 2 + 1 +

getWrittenSize(S32LEB(int32_t(size)));

return getWrittenSize(U32LEB(segment)) + 2 + getConstSize(Literal(int32_t(size)));

(seems better to reuse the Literal logic for size computation)

kripken · 2026-10-20T16:26:09Z

+
+// Candidates with identical payloads, which can share one copy of the bytes.
+struct PayloadGroup {
+  std::string_view payload;


Why not have this as a std::string as well, like in Candidate?

If it makes sense as a view, I think it is worth documenting what it is a view of, and why that thing outlives this view.

(another option is to just do candidates[0].payload, perhaps as a helper function)

kripken · 2026-10-20T16:34:11Z

+        }
+      });
+
+    // The map is in function order, so this order is deterministic.


It seems simpler to me to iterate on wasm->functions to get the deterministic ordering there. This does add a lookup, but then InsertOrderedMap adds a lookup structure, so I doubt it is any faster.

Also, this depends on the inserts to the InsertOrderedMap map to be in a deterministic order, and while that is the case in ModuleUtils::ParallelFunctionAnalysis atm, it might change in theory (that is an internal detail).

Hmm, doing this is the only reason we allow configuring the function-parallel analysis with an InsertOrderedMap in the first place, though (introduced in #3793). If we don't think that's a good idea, we should remove that functionality.

Ah, I forgot about these other uses. Another option might be to document that this is part of the contract of the ParallelFunctionAnalysis class (i.e. that the simple loop on wasm->functions in the constructor is not an internal implementation detail). That's probably good enough.

kripken · 2026-10-20T17:04:28Z

+      Index segment = wasm->dataSegments.size();
+      auto savings =
+        getCodeSavings(group, segment) - int64_t(group.payload.size()) -
+        int64_t(getNewSegmentOverhead(*wasm, group.payload.size()));


Please explain what the last two terms are here. The first of them is unclear to me.

gkdn added 4 commits October 16, 2026 17:16

gkdn requested a review from a team as a code owner October 17, 2026 04:35

gkdn requested review from tlively and removed request for a team October 17, 2026 04:35

gemini-code-assist Bot reviewed Oct 17, 2026

View reviewed changes

tlively reviewed Oct 19, 2026

View reviewed changes

Comment thread src/passes/ConstantArraysToData.cpp Outdated

kripken reviewed Oct 20, 2026

View reviewed changes

		if (options.lastOpts && wasm->features.hasGC() &&
		(options.optimizeLevel >= 2 \|\| options.shrinkLevel >= 1)) {

		auto savings = getCodeSavings(group, newSegmentIndex, offset) -
		int64_t(group.payload.size());

		newSegmentSavings <=
		int64_t(getNewSegmentOverhead(*module, newSegmentData.size()))) {

		builder.makeConst(int32_t(conversion.offset)),
		builder.makeConst(int32_t(array->values.size())));

		// TODO: Also reuse payloads that appear inside other data. That needs an
		// efficient substring search, to avoid quadratic behavior.

		// the given segment and offset. The arrays may have different types, so we
		// measure each one.

		return getWrittenSize(U32LEB(segment)) + 2 + 1 +
		getWrittenSize(S32LEB(int32_t(size)));

Conversation

gkdn commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 17, 2026

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gkdn commented Oct 20, 2026

Uh oh!

tlively commented Oct 20, 2026

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 Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gkdn commented Oct 17, 2026 •

edited

Loading

kripken Oct 20, 2026 •

edited

Loading