[PATCH] D77788: [PowerPC][Future] Enable Tail Calls for PC Relative Code

[Victor Huang via Phabricator via llvm-commits]

NeHuang added a comment.

Some nit comments added



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5509
+  // Check first if we have an indirect tail call when using PC Relative calls.
+  if (CFlags.IsTailCall && CFlags.IsIndirect &&
+      Subtarget.isUsingPCRelativeCalls()) {
----------------
nit: would it be better to adjust the if check in this way?

  if (CFlags.IsTailCall) {
    if (!(CFlags.IsIndirect && Subtarget.isUsingPCRelativeCalls())) {
      assert(((Callee.getOpcode() == ISD::Register &&
               cast<RegisterSDNode>(Callee)->getReg() == PPC::CTR) ||
              Callee.getOpcode() == ISD::TargetExternalSymbol ||
              Callee.getOpcode() == ISD::TargetGlobalAddress ||
              isa<ConstantSDNode>(Callee)) &&
             "Expecting a global address, external symbol, absolute value or "
             "register");
      assert(CallOpc == PPCISD::TC_RETURN &&
             "Unexpected call opcode for a tail call.");
    }
    DAG.getMachineFunction().getFrameInfo().setHasTailCall();
    return DAG.getNode(CallOpc, dl, MVT::Other, Ops);
  }


================
Comment at: llvm/lib/Target/PowerPC/PPCMCInstLower.cpp:90
 
-  if (MI->getOpcode() == PPC::BL8_NOTOC)
-    RefKind = MCSymbolRefExpr::VK_PPC_NOTOC;
+  unsigned MIOpcode = MI->getOpcode();
+  if (MIOpcode == PPC::BL8_NOTOC)
----------------
nit: Can we move `unsigned MIOpcode = MI->getOpcode();` down after line 97 and combined the check in line 91 with line 99?


================
Comment at: llvm/lib/Target/PowerPC/PPCMCInstLower.cpp:101
+      MIOpcode == PPC::TCRETURNdi || MIOpcode == PPC::TCRETURNdi8) {
+    if (Subtarget->isUsingPCRelativeCalls())
+      RefKind = MCSymbolRefExpr::VK_PPC_NOTOC;
----------------
nit: can we move the check before line 99 to save some check for non-pcrel mode? 


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-tail-calls.ll:5
+; RUN:   -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
+ at Func = external local_unnamed_addr global i32 (...)*, align 8
----------------
good to add a general description for the test


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77788/new/

https://reviews.llvm.org/D77788