Collect stats in DAE2 by tlively · Pull Request #8367 · WebAssembly/binaryen

tlively · 2026-02-24T01:40:13Z

Add a DAE_STATS constant that can be set to collect and report the following statistics about DAE2 optimization:

The total number of removed parameters.
The number of removed parameters in unreferenced functions.
The number of removed parameters in inhabited function type trees.
The number of removed parameters (in either of the previous
categories) in forwarding cycles.
The size of the largest removed forwarding cycle.
The size of the longest removed forwarding chain.

Add a similar option to normal DAE and SignaturePruning to facilitate
comparisons.

Add a DAE_STATS constant that can be set to collect and report the following statistics about DAE2 optimization: - The total number of removed parameters. - The number of removed parameters in unreferenced functions. - The number of removed parameters in inhabited function type trees. - The number of removed parameters (in either of the previous categories) in forwarding cycles. - The size of the largest removed forwarding cycle. - The size of the longest removed forwarding chain. Add a similar option to normal DAE and SignaturePruning to facilitate comparisons.

kripken · 2026-02-24T18:01:45Z

src/passes/DeadArgumentElimination.cpp

+
+#ifndef DAE_STATS
+#define DAE_STATS 0
+#endif


Do we need this pattern? I think we normally just do ifdef in the places below.

This is kind of handy because it lets me enable the statistics gathering for all three passes by updating my build rather than editing all the files. It's not strictly necessary, but I think it pulls its weight.

I don't follow, why doesn't that work my way as well? Add -DDAE_STATS to the CMake command, and just rebuild?

Oh, you're saying we can just remove the #define DAE_STATS entirely? I do like having the #define at the top of the file to make it clear in the source that the option is available.

I feel the ifdef comments make that clear enough? Adding a general comment on top is also an option.

I'm just trying to keep things consistent with the rest of the codebase, to be clear, I don't feel strongly. A refactor is also an option.

If you have just the #ifdef without surrounding it by #ifndef, then you can't configure the option from the build settings IIUC. So we want all or nothing here. I'd be happy to add similar #ifndef wrappers around other similar defines in other files in a follow-up.

If you have just the #ifdef without surrounding it by #ifndef, then you can't configure the option from the build settings IIUC.

Wait, why?

E.g. possible-contents.cpp has many chunks of code with

#ifdef POSSIBLE_CONTENTS_DEBUG .. #endif

Can't I add -DPOSSIBLE_CONTENTS_DEBUG to the cflags for that to work..?

Yes, but not if you have #define POSSIBLE_CONTENTS_DEBUG at the top of the file, which I'm saying is useful to have for documentation purposes and for the case when you do want to turn on or off the option by quickly editing the file.

Fair enough, I prefer it the other way but not strongly.

However, if you do want to use this new pattern, will you refactor existing places to use it too?

Yep, will do.

kripken · 2026-02-24T18:02:36Z

src/passes/DeadArgumentElimination2.cpp

    TIME(std::cerr << "fixed point: " << timer.lastElapsed() << "\n");

+#if DAE_STATS
+


Suggested change

Also below.

kripken · 2026-02-24T18:03:31Z

src/passes/DeadArgumentElimination2.cpp

+    // Find the depth of this SCC,
+    // which one more than the max depth of its children.


Suggested change

// Find the depth of this SCC,

// which one more than the max depth of its children.

// Find the depth of this SCC, which is one more than the max depth of its

// children.

kripken · 2026-02-24T18:04:33Z

src/passes/DeadArgumentElimination2.cpp

+  }
+
+  // Find the strongly-connected components to find cycles. Consider only nodes
+  // that have been optimized out when computing the graph.


...is there not a simple place to do ++numRemovedFoo for each of the things you want?

No, there is no existing place where cycles are treated specially or even identified. Everything else operates at the level of individual nodes (params) and edges (forwarding relationships), and cycles are not special or visible at these local levels.

And similarly for chain depths. We could count the total numbers of removed parameters of different kinds at the point where we remove them, but that would require atomic counters and wouldn't materially simplify anything here.

I guess I was just thinking of such atomic counters, yeah. If you want cycles as well, sgtm.

I think it's very valuable to have the cycle and chain depth statistics because those are the unique advantages of this pass over existing passes.

Fair enough. I was just imagining a comparison of the things that overlap, like num removed params. Then we could assume more removals here was due to cycles, by inference.

tlively requested a review from kripken February 24, 2026 01:40

kripken reviewed Feb 24, 2026

View reviewed changes

kripken approved these changes Feb 24, 2026

View reviewed changes

fix formatting

e592837

tlively enabled auto-merge (squash) February 24, 2026 21:00

tlively merged commit 1210337 into main Feb 24, 2026
17 checks passed

tlively deleted the dae2-stats branch February 24, 2026 21:42

		TIME(std::cerr << "fixed point: " << timer.lastElapsed() << "\n");

		#if DAE_STATS

		// Find the depth of this SCC,
		// which one more than the max depth of its children.

Comments

Conversation

tlively commented Feb 24, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants