[PowerPC] update PPCTTIImpl::supportsTailCallFor() check conditions

[tingwang]

Summary

This patch reuse PPCTargetLowering::isEligibleForTCO() to check PPCTTIImpl::supportsTailCallFor().

Fixes #59315

[tingwang]

Refresh and Ping.

[shchenz]

hmm, another issue cause by inconsistent logic for the tail call check between the IR coro-split pass and the instruction selection pass. Should we make them be consistent to avoid more issues?
For example, can we refactor IsEligibleForTailCallOptimization_64SVR4 and IsEligibleForTailCallOptimization() and call them in the TTI function supportsTailCallFor? I see there is a SelectionDAG &DAG parameter for these two functions, but the DAG is used to get the Caller Function, I think it should be easy to change the SelectionDAG to a Function parameter?

And for mayBeEmittedAsTailCall(), maybe we should also add another patch to extend it for non 64BitELFABI targets?

llvm/test/Transforms/Coroutines/coro-split-musttail-ppc64le.ll

14

Why asserts is required? I don't see checks of debug logs?

[shchenz]

Seems for patch that addresses github issues, you can add a comment Fixes #59315 in the description.

[tingwang]

As suggested by Zheng, I'm looking for a solution that reuse existing functions in PPCISelLowering.

[tingwang]

(1) Use new API to do the check.
(2) Remove assert requirement.
(3) Fixed test case: CHECK-PCREL should not be tailcall.

[tingwang]

llvm/test/Transforms/Coroutines/coro-split-musttail-ppc64le.ll

14

Good catch. Thank you!

[tingwang]

In D140369#4038843, @shchenz wrote:

hmm, another issue cause by inconsistent logic for the tail call check between the IR coro-split pass and the instruction selection pass. Should we make them be consistent to avoid more issues?
For example, can we refactor IsEligibleForTailCallOptimization_64SVR4 and IsEligibleForTailCallOptimization() and call them in the TTI function supportsTailCallFor? I see there is a SelectionDAG &DAG parameter for these two functions, but the DAG is used to get the Caller Function, I think it should be easy to change the SelectionDAG to a Function parameter?

And for mayBeEmittedAsTailCall(), maybe we should also add another patch to extend it for non 64BitELFABI targets?

Thank you! I created D141673 to refactor the two functions as you suggested. Regarding mayBeEmittedAsTailCall(), it should be easy to extend, and will be addressed separately.

[shchenz]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5664

Should this be on PPC target only or it should be a target independent check? shouldBeMustTail checks some target independent conditions.

llvm/test/Transforms/Coroutines/coro-split-musttail-ppc64le.ll

13

nit: ppc32-- and powerpc-- should both be for PPC arch (32 bit, BE), I think just one line is needed.

[tingwang]

Update according to comments:
(1) Removed IsByValArg check which is redundant.
(2) Removed one test case scenario which is redundant.

[tingwang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5664

Yeah, these checks are already covered by shouldBeMustTail, and could be redundant. I removed the check and updated the patch. Thank you!

[shchenz]

LGTM with one nit.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5661

we may need a NFC patch to remove the IsByValArg for isEligibleForTCO as all callers pass false to it, we should be able to check the byval parameter inside isEligibleForTCO.

[tingwang]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5661

Sure, will do. Thank you!