[PATCH] D29020: [ARM] Change TCReturn to tBL if tailcall optimization fails.

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added a comment.

In https://reviews.llvm.org/D29020#654783, @rengolin wrote:

> Thanks!
>
> Now, I'm trying to understand what the problem is.
>
> It seems that a previous process to deal with tail calls missed a spot, and you're adding the fix on the last possible stage to just change it to a branch&link.
>
> The idea seems fine, but I'm worried that the implementation could leave untested areas uncovered.
>
> Not to mention that, if there is a process that deals with tail calls, the code should not leave that unchecked. Ie. there should be no `TCRETURN*` after it at all.
>
> I don't remember well that part of the code, so I may be missing something, but it looks to me that there is a more encompassing solution that we're not seeing here.
>
> Also, from the test alone, it's not clear what cases fail to be processed and what don't. Can you elaborate on the description of the review what was the problem you found, what was the approach and what cases you hope to have covered?
>
> cheers,
> --renato


The tail call optimisation is performed before register allocation, so at that point we don't know if LR is being spilt or not.  If LR was spilt to the stack, then we cannot do a tail call optimisation. That would involve popping back into LR which is not possible in Thumb1 code.

To me, this seems like the logical place to catch this case.

If you're happy with that explanation, I can move it into the commit message.  My apologies that is was missing; it took some digging to find the rationale for this bit of code.


https://reviews.llvm.org/D29020