Flatten WeakTopologicalOrdering into a single entry vector

[tlively]

Replace the recursive std::variant/Cycle representation of
WeakTopologicalOrdering with a single contiguous std::vector
where cycle ends store the header block and jump target index. This
avoids per-cycle vector allocations and recursive evaluation in
WTOWorklist::run.

Benchmark results across 16 WebAssembly modules (3 iterations,
interleaved):

• --constraint-analysis:
  • Geomean: 1.606s -> 1.592s (-0.9%)
  • Total time: 64.05s -> 63.67s (-0.6%; faster on 11/16 modules)
• --rse:
  • Geomean: 0.871s -> 0.852s (-2.2%)
  • Total time: 26.03s -> 25.39s (-2.5%; faster on 12/16 modules)

[kripken]

Before I start to read this, can I ask about the process here? This was part of the initial sequence of PRs iianm. Why was this not applied to the first in the sequence? I mean, why is this a followup and not the initial version?

[tlively]

The initial PR was the simplest possible code for the new algorithm, and all the follow-on PRs have been much smaller optimizations on top of that, often increasing complexity in exchange for better performance. This one in particular increases complexity significantly by making the nesting structure of the WTO implicit instead of explicit.

Separating out the follow-on optimizations helps us consider whether they independently carry their weight. For example, there was one optimization I had locally that turned out not to be helpful, so I dropped it before uploading the PRs in this stack.

[kripken]

What marks the cycle's start? Maybe I'm not understanding this comment. Is cycleTarget meaningful in all cases?

[tlively]

Nothing marks the cycle's start besides the fact that later cycleTargets point back to it. cycleTarget determines whether this is a "normal" entry (when it is NoTarget) or a cycle end marker entry.

[kripken]

I'm still not sure how to read this. So a particular block B will appear twice, once at the start and once at the end?

Some things that might be confusing me: the word "visits" on line 97, and the word "either".

Perhaps this can be explained as follows?

1. Each block appears in one Entry.}
2. cycleTarget is usually NoTarget, except for the end of a cycle, where it points to the cycle's start.

For example:

[ {B1, NoTarget}, {B2, B1} ]

This is a simple loop `B1 -> B2 -> B1`.

[tlively]

Not quite. When cycleTarget != NoTarget, block is the header of the loop, which already appeared previously as a normal entry (see lines 247 and 249). Storing the loop header directly with the cycle end marker means line 332 has one less indirection.

[kripken]

Ok, then how about this comment:

1. A normal Entry refers to a block, and has NoTarget for cycleTarget.
2. A loop end is marked by an Entry where the block (which appeared before
   already) is the header, and cycleTarget is...?

For example:
[..]

Where I still do not follow is the end of part 2. It seems like the block is already saying where the backedge goes to, so we just need a boolean "this is a cycle end"? What information is conveyed in cycleTarget?

(an example in the comment might help)

[kripken]

I see, thanks. This doesn't look obviously more complex, though now that I am reading it, I see I don't follow... 😄

[tlively]

Well this is obviously redundant!

[kripken]

Suggested change

	BasicBlock* block = nullptr;
	BasicBlock* block;

This made me think it was optional, and I don't see a benefit to this default?