[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 ↗

(On Diff #488970)

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 ↗

(On Diff #488970)

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

5672 ↗

(On Diff #500719)

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

5672 ↗

(On Diff #500719)

Sure, will do. Thank you!

llvm/lib/Target/PowerPC/PPCISelLowering.h

struct CallFlags {
           : CallConv(CC), IsTailCall(IsTailCall), IsVarArg(IsVarArg),
             IsPatchPoint(IsPatchPoint), IsIndirect(IsIndirect),
             HasNest(HasNest), NoMerge(NoMerge) {}
     };
 
     CCAssignFn *ccAssignFnForCall(CallingConv::ID CC, bool Return,
                                   bool IsVarArg) const;
 
+    // Return whether the call instruction can potentially be optimized to a
+    // tail call. This will cause the optimizers to attempt to move, or
+    // duplicate return instructions to help enable tail call optimizations.
+    bool mayBeEmittedAsTailCall(const CallInst *CI) const override;
+
   private:
     struct ReuseLoadInfo {
       SDValue Ptr;
       SDValue Chain;
       SDValue ResChain;
       MachinePointerInfo MPI;
       bool IsDereferenceable = false;
       bool IsInvariant = false;
private:
     /// handled by the VINSERTB instruction introduced in ISA 3.0. This is
     /// essentially v16i8 vector version of VINSERTH.
     SDValue lowerToVINSERTB(ShuffleVectorSDNode *N, SelectionDAG &DAG) const;
 
     /// lowerToXXSPLTI32DX - Return the SDValue if this VECTOR_SHUFFLE can be
     /// handled by the XXSPLTI32DX instruction introduced in ISA 3.1.
     SDValue lowerToXXSPLTI32DX(ShuffleVectorSDNode *N, SelectionDAG &DAG) const;
 
-    // Return whether the call instruction can potentially be optimized to a
-    // tail call. This will cause the optimizers to attempt to move, or
-    // duplicate return instructions to help enable tail call optimizations.
-    bool mayBeEmittedAsTailCall(const CallInst *CI) const override;
     bool hasBitPreservingFPLogic(EVT VT) const override;
     bool isMaskAndCmp0FoldingBeneficial(const Instruction &AndI) const override;
 
     /// getAddrModeForFlags - Based on the set of address flags, select the most
     /// optimal instruction format to match by.
     PPC::AddrMode getAddrModeForFlags(unsigned Flags) const;
 
     /// computeMOFlags - Given a node N and it's Parent (a MemSDNode), compute

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

InstructionCost PPCTTIImpl::getVPMemoryOpCost(unsigned Opcode, Type *Src,
 
   // Usually we should not get to this point, but the following is an attempt to
   // model the cost of legalization. Currently we can only lower intrinsics with
   // evl but no mask, on Power 9/10. Otherwise, we must scalarize.
   return getMaskedMemoryOpCost(Opcode, Src, Alignment, AddressSpace, CostKind);
 }
 
 bool PPCTTIImpl::supportsTailCallFor(const CallBase *CB) const {
+  // Can't tail call when always use indirect calls.
+  if (ST->useLongCalls())
+    return false;
+
   // Subtargets using PC-Relative addressing supported.
   if (ST->isUsingPCRelativeCalls())
     return true;
 
+  auto *C = dyn_cast<CallInst>(CB);
+  if (!C)
+    return false;
+
+  if (ST->is64BitELFABI())
+    return TLI->mayBeEmittedAsTailCall(C);
+
+  // Below checks mimic IsEligibleForTailCallOptimization
+  if (!ST->getTargetMachine().Options.GuaranteedTailCallOpt)
+    return false;
+
   const Function *Callee = CB->getCalledFunction();
   // Indirect calls and variadic argument functions not supported.
   if (!Callee || Callee->isVarArg())
     return false;
 
-  const Function *Caller = CB->getCaller();
-  // Support if we can share TOC base.
-  return ST->getTargetMachine().shouldAssumeDSOLocal(*Caller->getParent(),
-                                                     Callee);
+  // shouldBeMustTail already confirmed: CallerCC == CalleeCC, and that CB does
+  // not have any ABI-impacting function attributes including ByVal. Then
+  // non-PIC/GOT tail calls are supported.
+  if (CB->getCallingConv() == CallingConv::Fast &&
+      ST->getTargetMachine().getRelocationModel() != Reloc::PIC_)
+    return true;
+
+  return false;
 }

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

 ; Tests that some target (e.g. ppc) can support tail call under condition.
 ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S \
 ; RUN:     -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 | FileCheck %s
 ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S \
 ; RUN:     -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr10 --code-model=medium \
-; RUN:     | FileCheck %s --check-prefix=CHECK-PCREL
+; RUN:     | FileCheck %s --check-prefix=CHECK-MUSTTAIL
+; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S \
+; RUN:     -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr10 --code-model=medium -mattr=+longcall \
+; RUN:     | FileCheck %s
+; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S \
+; RUN:     -mtriple=powerpc-- | FileCheck %s
+; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S \
+; RUN:     -mtriple=ppc32-- | FileCheck %s
+; REQUIRES: asserts
 
 define void @f() #0 {
 entry:
   %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
   %alloc = call ptr @malloc(i64 16) #3
   %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
 
   %save = call token @llvm.coro.save(ptr null)

 ; CHECK-NOT: musttail call fastcc void %[[addr1]](ptr null)
 
 ; Verify that ppc target not using PC-Relative addressing in the resume part resume call is not marked with musttail.
 ; CHECK-LABEL: @f.resume(
 ; CHECK: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
 ; CHECK-NEXT: call fastcc void %[[addr2]](ptr null)
 
 ; Verify that ppc target using PC-Relative addressing in the resume part resume call is marked with musttail.
-; CHECK-PCREL-LABEL: @f.resume(
-; CHECK-PCREL: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; CHECK-PCREL-NEXT: musttail call fastcc void %[[addr2]](ptr null)
-; CHECK-PCREL-NEXT: ret void
+; CHECK-MUSTTAIL-LABEL: @f.resume(
+; CHECK-MUSTTAIL: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
+; CHECK-MUSTTAIL-NEXT: musttail call fastcc void %[[addr2]](ptr null)
+; CHECK-MUSTTAIL-NEXT: ret void
 
 declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
 declare i1 @llvm.coro.alloc(token) #2
 declare i64 @llvm.coro.size.i64() #3
 declare ptr @llvm.coro.begin(token, ptr writeonly) #2
 declare token @llvm.coro.save(ptr) #2
 declare ptr @llvm.coro.frame() #3
 declare i8 @llvm.coro.suspend(token, i1) #2