Collapse switches

[ncruces]

This pass adds several checks that #37 skipped, including #37 (comment).

Please @pgaskin, check that this doesn't break anything.

Please @msuozzo, check that this produces the effect you're after (avoid deep nesting of 100s of blocks).

It's a known issue that this introduces warnings for unreachable fallthrough statements. This can be improved (at the cost of more complexity) by introducing a more precise termination check here:

wasm2go/internal/passes/switch.go

Lines 114 to 118 in b615d76

	// If it is the final one, we need to add a fallthrough.
	if hasDefault && idx >= 0 {
	cc.Body = append(cc.Body, &ast.BranchStmt{Tok: token.FALLTHROUGH})
	return idx
	}

However, this only causes warnings, and whatever we do there should err only on the side of caution: if in doubt, add the fallthrough.

I don't think this can be made simpler, and still be correct. I'm also not entirely sure it is correct yet.

[msuozzo]

Confirmed this works for my case (and performs better than my initial attempt). I'll run my proptests against this to see if it turns up anything.

Thanks!

[pgaskin]

This passes all of my test suites, but I haven't put it through my WIP fuzzer or reviewed the code yet (will do that later).

There is a slight improvement for C++ binaries, but Rust binaries improve significantly.

[ncruces]

Interesting, I'm guessing due to enums and match.

What I didn't test is line numbers, and… I'm not sure I want to. Those certainly break the assumption that code is "this or that shape" and in some ways, that it's "valid to begin with."

Mostly likely, they just render the optimization ineffective, as cases won't have a single goto label; which can be improved (with some additional complexity), but not fixed generally.

I'd love to find a way to track line numbers in a way that doesn't change the shape of the AST (at least until after optimization).

[ncruces]

Supporting cases with more than just goto label is possible in some narrow cases, but I don't think the complexity pays off.

[pgaskin]

What I didn't test is line numbers

So I originally didn't even plan to try and make them work with optimizations, but they actually do pretty well since they are just statements in the AST. They're a best-effort debugging feature anyways, so it's not critical.

[ncruces]

This … wait for it … was still flawed.

I do a decent job of validating the switch is in good shape, that we can move the case clause. But then I'm happy to move invalid code into it. Just changing the order of passes generates wrong code.

AST manipulation is … a kludge. I think I can make this one work, but it's a kludge.

[ncruces]

So, as you can see compared to #37, not simple at all to make something like this robust.

It's now OK (seems to be) for most passes to reordered: I've tried a few random orders, and tests pass.

The ones that can't be reordered, are easier to reason about.

[ncruces]

I'm inclined to merge this, but it's the most ambitious optimization so far by a large margin.
So, I'll only merge after some more testing.

One of the reasons I was sceptical was that it took me so long to figure out #42 (comment).
I'd noticed a consequence of it while debugging #37, and it took me the entire week to figure out why.

Even for SQLite, which is not so terrible, this reduces the maximum indentation level from 222 to 108, so a nice improvement. I'll investigate if a robust version of #38 can be done too (the effect compounds).

[ncruces]

It's now OK (seems to be) for most passes to reordered: I've tried a few random orders, and tests pass.

The ones that can't be reordered, are easier to reason about.

All passes can be reordered now. Tested with:

psss := []func(fn *ast.FuncDecl){
	passes.RemoveSelfAssigns,
	passes.RemoveBlankAssigns,
	passes.RemoveUnusedLocals,
	passes.InlineSwitchGotos,
	passes.UnnestBlocks,
	passes.UnnestCases,
	passes.RemoveEmptyStmts,
	passes.InlineGotoEnd,
	passes.InlineGotoReturn,
}
psss = slices.Repeat(psss, 5)
rand.Shuffle(len(psss), func(i, j int) {
	psss[i], psss[j] = psss[j], psss[i]
})
for _, p := range psss[:len(psss)/3] {
	p(fn.decl)
}

passes.RemoveUnusedLocals(fn.decl)

This gives me some confidence that I'm not pigeonholing on the current generated code, that the passes are somewhat general and robust.