[RISCV] Permit tail call to an externally-defined function with weak linkage

[liaolucy]

Summary

As described in D45395 This has been modeled after ARM's tail call opt.
ARM's abi seems to limit weak symbol.

I did not find the limitation for RISCV. (Please correct me if I am wrong)

gcc seems to use the tail-call opt: https://godbolt.org/z/bjWE68n5o

[reames]

I don't believe this is, in general, legal to do for RISCV psABI. The problem is that a weak symbol can resolve to the address 0, and that 0 may be outside the addressable range of a pcrelative call/branch sequence. There is disagreement between the linkers as to whether such sequences should be rewritten to a different instruction sequence. For non-tail calls, we end up emitting an explicit load through the GOT and call sequence.

Note that this concern does not apply in all code models. (I believe medlow is not effected, but double check!) You can look at the code which emits the GOT lookup for general calls, and there's probably cases where we can emit the direct tail call. We just need to restrict it appropriately.

p.s. Your example with gcc is a static (not pc relative) call.

[MaskRay]

In D143137#4099933, @reames wrote:

I don't believe this is, in general, legal to do for RISCV psABI. The problem is that a weak symbol can resolve to the address 0, and that 0 may be outside the addressable range of a pcrelative call/branch sequence. There is disagreement between the linkers as to whether such sequences should be rewritten to a different instruction sequence. For non-tail calls, we end up emitting an explicit load through the GOT and call sequence.

Note that this concern does not apply in all code models. (I believe medlow is not effected, but double check!) You can look at the code which emits the GOT lookup for general calls, and there's probably cases where we can emit the direct tail call. We just need to restrict it appropriately.

p.s. Your example with gcc is a static (not pc relative) call.

@reames I think this change is fine. call and tail use the same relocation type. The "zero target address, possibly not reachable by the relocated location" issue only applies to possibly address significant relocation types.
Branch relocations are not address significant and linkers can resolve the relocation to the current location (infinite loop if executed) to avoid relocation out-of-range errors (see D103001).

[reames]

Removing myself as reviewer to remove the "Needs Changes" bit. I'm explicitly deferring to @MaskRay as he's much more knowledgeable here than I am. If he says this is fine; it's fine.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

 
 /// isEligibleForTailCallOptimization - Check whether the call is eligible
 /// for tail call optimization.
 /// Note: This is modelled after ARM's IsEligibleForTailCallOptimization.
 bool RISCVTargetLowering::isEligibleForTailCallOptimization(
     CCState &CCInfo, CallLoweringInfo &CLI, MachineFunction &MF,
     const SmallVector<CCValAssign, 16> &ArgLocs) const {
 
-  auto &Callee = CLI.Callee;
   auto CalleeCC = CLI.CallConv;
   auto &Outs = CLI.Outs;
   auto &Caller = MF.getFunction();
   auto CallerCC = Caller.getCallingConv();
 
   // Exception-handling functions need a special set of instructions to
   // indicate a return to the hardware. Tail-calling another function would
   // probably break this.
bool RISCVTargetLowering::isEligibleForTailCallOptimization(
 
   // Do not tail call opt if either caller or callee uses struct return
   // semantics.
   auto IsCallerStructRet = Caller.hasStructRetAttr();
   auto IsCalleeStructRet = Outs.empty() ? false : Outs[0].Flags.isSRet();
   if (IsCallerStructRet || IsCalleeStructRet)
     return false;
 
-  // Externally-defined functions with weak linkage should not be
-  // tail-called. The behaviour of branch instructions in this situation (as
-  // used for tail calls) is implementation-defined, so we cannot rely on the
-  // linker replacing the tail call with a return.
-  if (GlobalAddressSDNode *G = dyn_cast<GlobalAddressSDNode>(Callee)) {
-    const GlobalValue *GV = G->getGlobal();
-    if (GV->hasExternalWeakLinkage())
-      return false;
-  }
-
   // The callee has to preserve all registers the caller needs to preserve.
   const RISCVRegisterInfo *TRI = Subtarget.getRegisterInfo();
   const uint32_t *CallerPreserved = TRI->getCallPreservedMask(MF, CallerCC);
   if (CalleeCC != CallerCC) {
     const uint32_t *CalleePreserved = TRI->getCallPreservedMask(MF, CalleeCC);
     if (!TRI->regmaskSubsetEqual(CallerPreserved, CalleePreserved))
       return false;
   }

llvm/test/CodeGen/RISCV/tail-calls.ll

 ; CHECK-LABEL: caller_indirect_args
 ; CHECK-NOT: tail callee_indirect_args
 ; CHECK: call callee_indirect_args
 entry:
   %call = tail call i32 @callee_indirect_args(fp128 0xL00000000000000003FFF000000000000)
   ret void
 }
 
-; Externally-defined functions with weak linkage should not be tail-called.
-; The behaviour of branch instructions in this situation (as used for tail
-; calls) is implementation-defined, so we cannot rely on the linker replacing
-; the tail call with a return.
+; Perform tail call optimization for external weak symbol.
 declare extern_weak void @callee_weak()
 define void @caller_weak() nounwind {
 ; CHECK-LABEL: caller_weak
-; CHECK-NOT: tail callee_weak
-; CHECK: call callee_weak
+; CHECK: tail callee_weak
 entry:
   tail call void @callee_weak()
   ret void
 }
 
 ; Exception-handling functions need a special set of instructions to indicate a
 ; return to the hardware. Tail-calling another function would probably break
 ; this.