Add TailCall optimization pass (--tail-call)

[tlively]

Add a top-down walker pass that converts call, call_indirect, and call_ref instructions in tail position into return calls. As it traverses the expression tree, it keeps a set of expressions known to be in tail position because they are children of returns, or the end of blocks or ifs in tail position, etc. When a call is found in tail position, it is turned into a return call. Keep track of the exception handling depth to avoid incorrectly turning calls inside exception handlers into return calls.

[kripken]

Rather than a custom ::scan(), can we use ExpressionStackWalker? When we see a call we can walk up the stack of parents and see that they are just fallthroughs.

[tlively]

We would still need to duplicate a lot of the logic here. For example, the part where we look for side effects in br_table and br_if conditions or the part where we check whether the next instruction in a block is a return or a br to another tail-position block. And we wouldn't be able to use any generic helpers relating to fallthrough expressions because many fallthrough expressions like casts are not relevant here.

[kripken]

Maybe I'm not understanding what the br_if logic does. What is it actually handling?

[tlively]

If you have (br_if $out (call $tail) (cond-with-side-effect)), then you can't optimize even if the br_if itself is in tail position because the side effect is executed between the call and the end of the function.

[kripken]

Oh, yes, but that is done inside getImmediateFallthrough?

In my mind we just need to get the expression stack, then go from the call up to parents, checking falling-through works in each step?

[tlively]

Another complication is that for e.g. br_table, we would need to essentially do a DAG traversal of potentially many parent labels to make sure they all end up being in tail position. In contrast, the current approach can easily check this because it maintains a set of tail-position labels.

[kripken]

Ah yes, for br_table you do need a more complex analysis. But br_table is rare, br_table with a value even rarer, with a call rarer still, and to be in tail call position... well it's not impossible 😄 But if that is the only case, I think the simplification of this pass would be the better option, it should be much shorter.

[tlively]

I've pushed your suggested approach in the last commit (although I would still need to go through and update test comments). It is possible to make the pass simpler, as you say, but IMO it is not a whole lot simpler and it's not worth the loss in optimization power since the original approach was also simple enough.

[KKiiim]

Thanks for the alternative approach.

[KKiiim]

Should this case converted to return_call ?
That is the only diff compared to our internal implementation.

[tlively]

Yes, this should be optimized. That it's not is another casualty of the ExpressionStackWalker + fallthrough expression approach.

[kripken]

I think ExpressionStackWalker can handle this, by handling not just a fallthrough but also simply skipping up when we have a br? That is, the expression stack makes it easy to see that this is, in fact, in tail position.

[tlively]

No, the problem here is the single-arm If, which is not a fallthrough expression. We would have to add more special handling for it.

[kripken]

Correct, it isn't a fallthrough, but the special handling seems pretty simple? Just looking at the expression stack, the situation is easy to infer.

[kripken]

Looks like this isn't optimized, I assume because of the ExpressionStackWalker limitation?

But: this case should be handled by RemovedUnusedBrs. I see that it actually takes more than one cycle atm, but --remove-unused-brs --vacuum --remove-unused-brs leads to

 (func $void-br-table-all-tail (type $0) (param $idx i32)
  (block $out1
   (block $out2
    (call $void-callee)
   )
  )
 )

Which the ExpressionStackWalker version can then handle.

Is there a br_table case where we can't depend on RemoveUnusedBrs to handle things for us?

[tlively]

I've pushed a third approach that uses a proper preorder traversal with custom infrastructure that lets us pass isTail as a parameter rather than storing it in a side table. This also lets us split the logic out into different visitors that propagate isTail as appropriate rather than doing everything in scan. This is the approach I think we should go with.

[KKiiim]

LGTM

[KKiiim]

The PreWalker can be extracted from the current pass file and used as a shared helper class.