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

llvm/lib/Target/PowerPC/PPCISelLowering.h

struct CallFlags {
                 bool IsPatchPoint, bool IsIndirect, bool HasNest, bool NoMerge)
           : CallConv(CC), IsTailCall(IsTailCall), IsVarArg(IsVarArg),
             IsPatchPoint(IsPatchPoint), IsIndirect(IsIndirect),
             HasNest(HasNest), NoMerge(NoMerge) {}
     };
 
     CCAssignFn *ccAssignFnForCall(CallingConv::ID CC, bool Return,
                                   bool IsVarArg) const;
+    bool supportsTailCallFor(const CallBase *CB) const;
 
   private:
     struct ReuseLoadInfo {
       SDValue Ptr;
       SDValue Chain;
       SDValue ResChain;
       MachinePointerInfo MPI;
       bool IsDereferenceable = false;

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

SDValue PPCTargetLowering::FinishCall(
 
   Chain = DAG.getCALLSEQ_END(Chain, NumBytes, BytesCalleePops, Glue, dl);
   Glue = Chain.getValue(1);
 
   return LowerCallResult(Chain, Glue, CFlags.CallConv, CFlags.IsVarArg, Ins, dl,
                          DAG, InVals);
 }
 
+bool PPCTargetLowering::supportsTailCallFor(const CallBase *CB) const {
+  CallingConv::ID CalleeCC = CB->getCallingConv();
+  const Function *CallerFunc = CB->getCaller();
+  CallingConv::ID CallerCC = CallerFunc->getCallingConv();
+  const Function *CalleeFunc = CB->getCalledFunction();
+  if (!CalleeFunc)
+    return false;
+  const GlobalValue *CalleeGV = dyn_cast<GlobalValue>(CalleeFunc);
+
+  SmallVector<ISD::OutputArg, 2> Outs;
+  SmallVector<ISD::InputArg, 2> Ins;
+  bool IsByValArg = false;
+
+  GetReturnInfo(CalleeCC, CalleeFunc->getReturnType(),
+                CalleeFunc->getAttributes(), Outs, *this,
+                CalleeFunc->getParent()->getDataLayout());
+
+  for (const Argument &Arg : CalleeFunc->args())
+    if (Arg.hasAttribute(Attribute::ByVal) ||
+        Arg.hasAttribute(Attribute::InAlloca) ||
+        Arg.hasAttribute(Attribute::Preallocated))
+      IsByValArg = true;
+
+  return isEligibleForTCO(CalleeGV, CalleeCC, CallerCC, CB,
+                          CalleeFunc->isVarArg(), Outs, Ins, IsByValArg,
+                          CallerFunc, false /*isCalleeExternalSymbol*/);
+}
+
 bool PPCTargetLowering::isEligibleForTCO(
     const GlobalValue *CalleeGV, CallingConv::ID CalleeCC,
     CallingConv::ID CallerCC, const CallBase *CB, bool isVarArg,
     const SmallVectorImpl<ISD::OutputArg> &Outs,
     const SmallVectorImpl<ISD::InputArg> &Ins, bool isByValArg,
     const Function *CallerFunc, bool isCalleeExternalSymbol) const {
   bool IsTailCall;
   if (Subtarget.useLongCalls() && !(CB && CB->isMustTailCall()))

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 {
-  // Subtargets using PC-Relative addressing supported.
-  if (ST->isUsingPCRelativeCalls())
-    return true;
-
-  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);
+  return TLI->supportsTailCallFor(CB);
 }

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
+; 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
 
 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)
   %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)

 ; CHECK: %[[addr1:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
 ; 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
-
 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
 declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
 declare i1 @llvm.coro.end(ptr, i1) #2
 declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
 declare ptr @malloc(i64)
 
 attributes #0 = { presplitcoroutine }
 attributes #1 = { argmemonly nounwind readonly }
 attributes #2 = { nounwind }
 attributes #3 = { nounwind readnone }