Compiler: restore TCO of mutually recursive functions

[hhugo]

@vouillon. @micahcantor #1503 breaks tail call as mentioned by @vouillon

The global DCE does not preserve tail calls. We don't care normally since JavaScript does not support tail calls, but this makes a difference for the CPS transformation.

Except the Generate_closure pass does care about tail calls as it rewrite mutually recursive functions using a trampoline.

Do you see any issue with the change proposed here ?

[hhugo]

This issue was spotted by a single package (ezjsonm) during the opam release.
See ocaml/opam-repository#24888

[OlivierNicole]

I don’t exactly understand how global DCE broke tail calls and why this fixes it, so I won’t comment beyond the surface level.

[vouillon]

The call to Generate_closure.f should be after the CPS transformation.
I think the second commit is enough to fix the issue.

[hhugo]

The call to Generate_closure.f should be after the CPS transformation. I think the second commit is enough to fix the issue.

Can you explain the reasons ?
Moving it before seems to detect more deadcode (as show in 0fe4aab). It this invalid ?

[hhugo]

The call to Generate_closure.f should be after the CPS transformation. I think the second commit is enough to fix the issue.

Can you explain the reasons ? Moving it before seems to detect more deadcode (as show in 0fe4aab). It this invalid ?

Obviously, it removes the deadcode introduced by generate_closure.

[vouillon]

The call to Generate_closure.f should be after the CPS transformation. I think the second commit is enough to fix the issue.

Can you explain the reasons ? Moving it before seems to detect more deadcode (as show in 0fe4aab). It this invalid ?

Generate_closure.rewrite_mutable is only needed for functions within loops. The partial CPS transformation will turn some loops into mutual function calls, making it unnecessary.

[hhugo]

I've opened #1540 to get rid of rewrite_mutable. Is this the only reason ?

[vouillon]

I think so. rewrite_tc is disabled in this case.