[PowerPC][Future] Remove redundant r2 save and restore for indirect call

[NeHuang]

Summary

Currently an indirect call produces the following sequence on PCRelative mode:

extern void function( );
extern void (*ptrfunc) ( );

void g() {
  ptrfunc=function;
}

void f() {
  (*ptrfunc) ( );
}

Produce

	paddi 3, 0, .LC0@PCREL, 1
	ld 3, 0(3)
	std 2, 24(1)
	ld 12, 0(3)
	mtctr 12
	bctrl
	ld 2, 24(1)

Though the caller does not use or preserve r2, it is still saved and restored across a function call.
This patch is added to remove these redundant save and restores for indirect call.

[nemanjai]

There are 3 places that are concerned with whether or not we need to save/restore the TOC. There is certainly a possibility of these conditions diverging. Can you please write a small function that checks this and then use that in all 3 places?
Maybe something like

static inline bool isTOCSaveRequired(const PPCSubtarget &Subtarget) {
  return Subtarget.isAIXABI() ||
         (Subtarget.is64BitELFABI() && !Subtarget.isUsingPCRelativeCalls());
}

I suppose we don't really have to use it in LowerCall_64SVR4() since there we have already checked that we are on a 64-bit ELF, but I think it doesn't hurt to just use the same query everywhere.

Also, I think it would be useful to add a test case where the function pointer is passed as a parameter.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5426–5428

I don't think we should say "disable the code". Maybe:
// For ELFv2 ABI with PCRel, do not restore the TOC as it is not saved or used.

[NeHuang]

Addressed review comment.

[amyk]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5141

Is it worth mentioning AIX in the comment too, since the check also considers AIX?

6499

nit: End with period.

llvm/test/CodeGen/PowerPC/pcrel-indirect-call.ll

4

I don't believe the FileCheck prefix is needed if you wanted to prefix to say CHECK.
CHECK should be the default if no prefix is supplied.

[NeHuang]

Address review comments.

[nemanjai]

Minor change to the comment can be made on the commit. LGTM.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

5141

// AIX and 64-bit ELF ABIs w/o PCRel require a TOC save/restore around calls.

[NeHuang]

Address review comment.

[NeHuang]

Update "pcrel-call-linkage-with-calls.ll" after re-basing the branch.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

static bool isIndirectCall(const SDValue &Callee, SelectionDAG &DAG,
   // need to jump to the local entry point (see rL211174).
   if (!Subtarget.usesFunctionDescriptors() && !Subtarget.isELFv2ABI() &&
       isBLACompatibleAddress(Callee, DAG))
     return false;
 
   return true;
 }
 
+// AIX and 64-bit ELF ABIs w/o PCRel require a TOC save/restore around calls.
+static inline bool isTOCSaveRestoreRequired(const PPCSubtarget &Subtarget) {
+  return Subtarget.isAIXABI() ||
+         (Subtarget.is64BitELFABI() && !Subtarget.isUsingPCRelativeCalls());
+}
+
 static unsigned getCallOpcode(PPCTargetLowering::CallFlags CFlags,
                               const Function &Caller,
                               const SDValue &Callee,
                               const PPCSubtarget &Subtarget,
                               const TargetMachine &TM) {
   if (CFlags.IsTailCall)
     return PPCISD::TC_RETURN;
 
   // This is a call through a function pointer.
   if (CFlags.IsIndirect) {
     // AIX and the 64-bit ELF ABIs need to maintain the TOC pointer accross
     // indirect calls. The save of the caller's TOC pointer to the stack will be
     // inserted into the DAG as part of call lowering. The restore of the TOC
     // pointer is modeled by using a pseudo instruction for the call opcode that
     // represents the 2 instruction sequence of an indirect branch and link,
     // immediately followed by a load of the TOC pointer from the the stack save
-    // slot into gpr2.
-    if (Subtarget.isAIXABI() || Subtarget.is64BitELFABI())
-      return PPCISD::BCTRL_LOAD_TOC;
-
-    // An indirect call that does not need a TOC restore.
-    return PPCISD::BCTRL;
+    // slot into gpr2. For 64-bit ELFv2 ABI with PCRel, do not restore the TOC
+    // as it is not saved or used.
+    return isTOCSaveRestoreRequired(Subtarget) ? PPCISD::BCTRL_LOAD_TOC
+                                               : PPCISD::BCTRL;
   }
 
-  // FIXME: At this moment indirect calls are treated ahead of the
-  // PC Relative condition because binaries can still contain a possible
-  // mix of functions that use a TOC and functions that do not use a TOC.
-  // Once the PC Relative feature is complete this condition should be moved
-  // up ahead of the indirect calls and should return a PPCISD::BCTRL for
-  // that case.
   if (Subtarget.isUsingPCRelativeCalls()) {
     assert(Subtarget.is64BitELFABI() && "PC Relative is only on ELF ABI.");
     return PPCISD::CALL_NOTOC;
   }
 
   // The ABIs that maintain a TOC pointer accross calls need to have a nop
   // immediately following the call instruction if the caller and callee may
   // have different TOC bases. At link time if the linker determines the calls
else {
 
     // For the TOC based ABIs, we have saved the TOC pointer to the linkage area
     // on the stack (this would have been done in `LowerCall_64SVR4` or
     // `LowerCall_AIX`). The call instruction is a pseudo instruction that
     // represents both the indirect branch and a load that restores the TOC
     // pointer from the linkage area. The operand for the TOC restore is an add
     // of the TOC save offset to the stack pointer. This must be the second
     // operand: after the chain input but before any other variadic arguments.
-    if (Subtarget.is64BitELFABI() || Subtarget.isAIXABI()) {
+    // For 64-bit ELFv2 ABI with PCRel, do not restore the TOC as it is not
+    // saved or used.
+    if (isTOCSaveRestoreRequired(Subtarget)) {
       const MCRegister StackPtrReg = Subtarget.getStackPointerRegister();
 
       SDValue StackPtr = DAG.getRegister(StackPtrReg, RegVT);
       unsigned TOCSaveOffset = Subtarget.getFrameLowering()->getTOCSaveOffset();
       SDValue TOCOff = DAG.getIntPtrConstant(TOCSaveOffset, dl);
       SDValue AddTOC = DAG.getNode(ISD::ADD, dl, RegVT, StackPtr, TOCOff);
       Ops.push_back(AddTOC);
     }
SDValue PPCTargetLowering::LowerCall_64SVR4(
 
   if (!MemOpChains.empty())
     Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, MemOpChains);
 
   // Check if this is an indirect call (MTCTR/BCTRL).
   // See prepareDescriptorIndirectCall and buildCallOperands for more
   // information about calls through function pointers in the 64-bit SVR4 ABI.
   if (CFlags.IsIndirect) {
-    assert(!CFlags.IsTailCall &&  "Indirect tails calls not supported");
+    // For 64-bit ELFv2 ABI with PCRel, do not save the TOC of the
+    // caller in the TOC save area.
+    if (isTOCSaveRestoreRequired(Subtarget)) {
+      assert(!CFlags.IsTailCall && "Indirect tails calls not supported");
       // Load r2 into a virtual register and store it to the TOC save area.
       setUsesTOCBasePtr(DAG);
       SDValue Val = DAG.getCopyFromReg(Chain, dl, PPC::X2, MVT::i64);
       // TOC save area offset.
       unsigned TOCSaveOffset = Subtarget.getFrameLowering()->getTOCSaveOffset();
       SDValue PtrOff = DAG.getIntPtrConstant(TOCSaveOffset, dl);
       SDValue AddPtr = DAG.getNode(ISD::ADD, dl, PtrVT, StackPtr, PtrOff);
-    Chain = DAG.getStore(
-        Val.getValue(1), dl, Val, AddPtr,
-        MachinePointerInfo::getStack(DAG.getMachineFunction(), TOCSaveOffset));
+      Chain = DAG.getStore(Val.getValue(1), dl, Val, AddPtr,
+                           MachinePointerInfo::getStack(
+                               DAG.getMachineFunction(), TOCSaveOffset));
+    }
     // In the ELFv2 ABI, R12 must contain the address of an indirect callee.
     // This does not mean the MTCTR instruction must use R12; it's easier
     // to model this as an extra parameter, so do that.
     if (isELFv2ABI && !CFlags.IsPatchPoint)
       RegsToPass.push_back(std::make_pair((unsigned)PPC::X12, Callee));
   }
 
   // Build a sequence of copy-to-reg nodes chained together with token chain

llvm/test/CodeGen/PowerPC/pcrel-call-linkage-with-calls.ll

 ; CHECK-S-NEXT:    blr
 entry:
   %call = tail call signext i32 @externCall(i32 signext %a)
   ret i32 %call
 }
 
 define dso_local signext i32 @IndirectCall1(i32 signext %a, i32 signext %b) local_unnamed_addr {
 ; CHECK-ALL-LABEL: IndirectCall1:
-; CHECK-S:         addis r2, r12, .TOC.-.Lfunc_gep13@ha
-; CHECK-S-NEXT:    addi r2, r2, .TOC.-.Lfunc_gep13@l
-; CHECK-S:         .localentry     IndirectCall1, .Lfunc_lep13-.Lfunc_gep13
 ; CHECK-S:       # %bb.0: # %entry
 ; CHECK-S-NEXT:    mflr r0
 ; CHECK-S-NEXT:    std r0, 16(r1)
 ; CHECK-S-NEXT:    stdu r1, -32(r1)
-; CHECK-S-NEXT:    std r2, 24(r1)
 ; CHECK-S-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-S-NEXT:    .cfi_offset lr, 16
 ; CHECK-S-NEXT:    pld r12, indirectCall@PCREL(0), 1
 ; CHECK-S-NEXT:    add r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    mtctr r12
 ; CHECK-S-NEXT:    bctrl
-; CHECK-S-NEXT:    ld 2, 24(r1)
 ; CHECK-S-NEXT:    plwz r4, globalVar@PCREL(0), 1
 ; CHECK-S-NEXT:    mullw r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    addi r1, r1, 32
 ; CHECK-S-NEXT:    ld r0, 16(r1)
 ; CHECK-S-NEXT:    mtlr r0
 ; CHECK-S-NEXT:    blr
 entry:
   %add = add nsw i32 %b, %a
   %0 = load i32 (i32)*, i32 (i32)** @indirectCall, align 8
   %call = tail call signext i32 %0(i32 signext %add)
   %1 = load i32, i32* @globalVar, align 4
   %mul = mul nsw i32 %1, %call
   ret i32 %mul
 }
 
 define dso_local signext i32 @IndirectCall2(i32 signext %a, i32 signext %b) local_unnamed_addr {
 ; CHECK-ALL-LABEL: IndirectCall2:
-; CHECK-S:         addis r2, r12, .TOC.-.Lfunc_gep14@ha
-; CHECK-S-NEXT:    addi r2, r2, .TOC.-.Lfunc_gep14@l
-; CHECK-S:         .localentry     IndirectCall2, .Lfunc_lep14-.Lfunc_gep14
 ; CHECK-S:       # %bb.0: # %entry
 ; CHECK-S-NEXT:    mflr r0
 ; CHECK-S-NEXT:    std r0, 16(r1)
 ; CHECK-S-NEXT:    stdu r1, -32(r1)
-; CHECK-S-NEXT:    std r2, 24(r1)
 ; CHECK-S-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-S-NEXT:    .cfi_offset lr, 16
 ; CHECK-S-NEXT:    pld r12, indirectCall@PCREL(0), 1
 ; CHECK-S-NEXT:    add r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    mtctr r12
 ; CHECK-S-NEXT:    bctrl
-; CHECK-S-NEXT:    ld 2, 24(r1)
 ; CHECK-S-NEXT:    pld r4, externGlobalVar@got@pcrel(0), 1
 ; CHECK-S-NEXT:    lwz r4, 0(r4)
 ; CHECK-S-NEXT:    mullw r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    addi r1, r1, 32
 ; CHECK-S-NEXT:    ld r0, 16(r1)
 ; CHECK-S-NEXT:    mtlr r0
 ; CHECK-S-NEXT:    blr
 entry:
   %add = add nsw i32 %b, %a
   %0 = load i32 (i32)*, i32 (i32)** @indirectCall, align 8
   %call = tail call signext i32 %0(i32 signext %add)
   %1 = load i32, i32* @externGlobalVar, align 4
   %mul = mul nsw i32 %1, %call
   ret i32 %mul
 }
 
 define dso_local signext i32 @IndirectCall3(i32 signext %a, i32 signext %b, i32 (i32)* nocapture %call_param) local_unnamed_addr {
 ; CHECK-ALL-LABEL: IndirectCall3:
-; CHECK-S:         addis r2, r12, .TOC.-.Lfunc_gep15@ha
-; CHECK-S-NEXT:    addi r2, r2, .TOC.-.Lfunc_gep15@l
-; CHECK-S:         .localentry     IndirectCall3, .Lfunc_lep15-.Lfunc_gep15
 ; CHECK-S:       # %bb.0: # %entry
 ; CHECK-S-NEXT:    mflr r0
 ; CHECK-S-NEXT:    std r0, 16(r1)
 ; CHECK-S-NEXT:    stdu r1, -32(r1)
-; CHECK-S-NEXT:    std r2, 24(r1)
 ; CHECK-S-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-S-NEXT:    .cfi_offset lr, 16
 ; CHECK-S-NEXT:    add r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    mtctr r5
 ; CHECK-S-NEXT:    mr r12, r5
 ; CHECK-S-NEXT:    bctrl
-; CHECK-S-NEXT:    ld 2, 24(r1)
 ; CHECK-S-NEXT:    plwz r4, globalVar@PCREL(0), 1
 ; CHECK-S-NEXT:    mullw r3, r4, r3
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    addi r1, r1, 32
 ; CHECK-S-NEXT:    ld r0, 16(r1)
 ; CHECK-S-NEXT:    mtlr r0
 ; CHECK-S-NEXT:    blr
 entry:
   %add = add nsw i32 %b, %a
   %call = tail call signext i32 %call_param(i32 signext %add)
   %0 = load i32, i32* @globalVar, align 4
   %mul = mul nsw i32 %0, %call
   ret i32 %mul
 }
 
 define dso_local signext i32 @IndirectCallNoGlobal(i32 signext %a, i32 signext %b, i32 (i32)* nocapture %call_param) local_unnamed_addr {
 ; CHECK-ALL-LABEL: IndirectCallNoGlobal:
-; CHECK-S:         addis r2, r12, .TOC.-.Lfunc_gep16@ha
-; CHECK-S-NEXT:    addi r2, r2, .TOC.-.Lfunc_gep16@l
-; CHECK-S:         .localentry     IndirectCallNoGlobal, .Lfunc_lep16-.Lfunc_gep16
 ; CHECK-S:       # %bb.0: # %entry
 ; CHECK-S-NEXT:    mflr r0
 ; CHECK-S-NEXT:    .cfi_def_cfa_offset 48
 ; CHECK-S-NEXT:    .cfi_offset lr, 16
 ; CHECK-S-NEXT:    .cfi_offset r30, -16
 ; CHECK-S-NEXT:    std r30, -16(r1) # 8-byte Folded Spill
 ; CHECK-S-NEXT:    std r0, 16(r1)
 ; CHECK-S-NEXT:    stdu r1, -48(r1)
 ; CHECK-S-NEXT:    mtctr r5
 ; CHECK-S-NEXT:    mr r12, r5
-; CHECK-S-NEXT:    std r2, 24(r1)
 ; CHECK-S-NEXT:    mr r30, r4
 ; CHECK-S-NEXT:    bctrl
-; CHECK-S-NEXT:    ld 2, 24(r1)
 ; CHECK-S-NEXT:    add r3, r3, r30
 ; CHECK-S-NEXT:    extsw r3, r3
 ; CHECK-S-NEXT:    addi r1, r1, 48
 ; CHECK-S-NEXT:    ld r0, 16(r1)
 ; CHECK-S-NEXT:    ld r30, -16(r1) # 8-byte Folded Reload
 ; CHECK-S-NEXT:    mtlr r0
 ; CHECK-S-NEXT:    blr
 entry:
   %call = tail call signext i32 %call_param(i32 signext %a)
   %add = add nsw i32 %call, %b
   ret i32 %add
 }
 
 define dso_local signext i32 @IndirectCallOnly(i32 signext %a, i32 (i32)* nocapture %call_param) local_unnamed_addr {
 ; CHECK-ALL-LABEL: IndirectCallOnly:
-; CHECK-S:         addis r2, r12, .TOC.-.Lfunc_gep17@ha
-; CHECK-S-NEXT:    addi r2, r2, .TOC.-.Lfunc_gep17@l
-; CHECK-S:         .localentry     IndirectCallOnly, .Lfunc_lep17-.Lfunc_gep17
 ; CHECK-S:       # %bb.0: # %entry
 ; CHECK-S-NEXT:    mflr r0
 ; CHECK-S-NEXT:    std r0, 16(r1)
 ; CHECK-S-NEXT:    stdu r1, -32(r1)
-; CHECK-S-NEXT:    std r2, 24(r1)
 ; CHECK-S-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-S-NEXT:    .cfi_offset lr, 16
 ; CHECK-S-NEXT:    mtctr r4
 ; CHECK-S-NEXT:    mr r12, r4
 ; CHECK-S-NEXT:    bctrl
-; CHECK-S-NEXT:    ld 2, 24(r1)
 ; CHECK-S-NEXT:    addi r1, r1, 32
 ; CHECK-S-NEXT:    ld r0, 16(r1)
 ; CHECK-S-NEXT:    mtlr r0
 ; CHECK-S-NEXT:    blr
 entry:
   %call = tail call signext i32 %call_param(i32 signext %a)
   ret i32 %call
 }
 
 attributes #0 = { noinline }
 

llvm/test/CodeGen/PowerPC/pcrel-got-indirect.ll

 }
 
 define dso_local void @ReadFuncPtr() local_unnamed_addr  {
 ; CHECK-LABEL: ReadFuncPtr:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    mflr r0
 ; CHECK-NEXT:    std r0, 16(r1)
 ; CHECK-NEXT:    stdu r1, -32(r1)
-; CHECK-NEXT:    std r2, 24(r1)
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset lr, 16
 ; CHECK-NEXT:    pld r3, ptrfunc@got@pcrel(0), 1
 ; CHECK-NEXT:    ld r12, 0(r3)
 ; CHECK-NEXT:    mtctr r12
 ; CHECK-NEXT:    bctrl
-; CHECK-NEXT:    ld 2, 24(r1)
 ; CHECK-NEXT:    addi r1, r1, 32
 ; CHECK-NEXT:    ld r0, 16(r1)
 ; CHECK-NEXT:    mtlr r0
 ; CHECK-NEXT:    blr
 entry:
   %0 = load void ()*, void ()** bitcast (void (...)** @ptrfunc to void ()**), align 8
   tail call void %0()
   ret void

llvm/test/CodeGen/PowerPC/pcrel-indirect-call.ll

+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=future -ppc-asm-full-reg-names < %s | FileCheck %s
+
+; The test checks the behavior of PC Relative indirect calls. When using
+; PC Relative, TOC save and restore are no longer required. Function pointer
+; is passed as a parameter in this test.
+
+; Function Attrs: noinline
+define dso_local void @IndirectCallExternFuncPtr(void ()* nocapture %ptrfunc) {
+; CHECK-LABEL: IndirectCallExternFuncPtr:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    mflr r0
+; CHECK-NEXT:    std r0, 16(r1)
+; CHECK-NEXT:    stdu r1, -32(r1)
+
+; CHECK-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-NEXT:    .cfi_offset lr, 16
+; CHECK-NEXT:    mtctr r3
+; CHECK-NEXT:    mr r12, r3
+; CHECK-NEXT:    bctrl
+
+; CHECK-NEXT:    addi r1, r1, 32
+; CHECK-NEXT:    ld r0, 16(r1)
+; CHECK-NEXT:    mtlr r0
+; CHECK-NEXT:    blr
+entry:
+  tail call void %ptrfunc()
+  ret void
+}
+
+define dso_local void @FuncPtrPassAsParam() {
+entry:
+  tail call void @IndirectCallExternFuncPtr(void ()* nonnull @Function)
+  ret void
+}
+
+declare void @Function()