[CodeGen] Enable tail call position check for speculatable functions

[NeHuang]

Summary

In the function "Analysis.cpp:isInTailCallPosition", it only checks whether a call is in a tail call position if the call has side effects, access memory or it is not safe to speculative execute. Therefore, a speculatable function will not go through tail call position check and improperly tail called when it is not in a tail-call position. An example as below:

IR reproducer

define dso_local void @caller(double* nocapture %res, double %a) local_unnamed_addr #0 {
entry:
  %call = tail call double @callee(double %a) #2
  store double %call, double* %res, align 8
  ret void
}
define double @callee(double) local_unnamed_addr #1 {
  ret double 4.5
}
attributes #0 = { nounwind }
attributes #1 = { readnone speculatable }
attributes #2 = { nounwind noinline }

produces a tail call in the caller without storing the result, which can be seen in the initial SDAG below

llc < reproducer.ll -debug-only=isel -mtriple=x86_64
=== caller
Initial selection DAG: %bb.0 'caller:entry'
SelectionDAG has 12 nodes:
  t0: ch = EntryToken
  t2: i64,ch = CopyFromReg t0, Register:i64 %0
  t5: i64 = GlobalAddress<double (double)* @callee> 0
    t4: f64,ch = CopyFromReg t0, Register:f64 %1
  t7: ch,glue = CopyToReg t0, Register:f64 $xmm0, t4
  t11: ch,glue = X86ISD::TC_RETURN t7, TargetGlobalAddress:i64<double (double)* @callee> 0, Constant:i32<0>, Register:f64 $xmm0, RegisterMask:Untyped, t7:1

This patch enables tail call position check for speculatable functions.

[jdoerfert]

Test for speculatable with a single return use but intermediate instructions missing.

[NeHuang]

In D80661#2058528, @jdoerfert wrote:

Test for speculatable with a single return use but intermediate instructions missing.

Thanks for your input. "@speculatable_callee_return_use_only" was created for single return use case in "tailcall-speculatable-callee.ll".

[NeHuang]

In D80661#2058528, @jdoerfert wrote:

Test for speculatable with a single return use but intermediate instructions missing.

Sorry, I misunderstood your point in previous comment. I am adding and verifying a case where intermediate instructions (not use the result of callee) added between the speculatable callee and the return instruction.

[jdoerfert]

In D80661#2064090, @NeHuang wrote:

In D80661#2058528, @jdoerfert wrote:

Test for speculatable with a single return use but intermediate instructions missing.

Sorry, I misunderstood your point in previous comment. I am adding and verifying a case where intermediate instructions (not use the result of callee) added between the speculatable callee and the return instruction.

That's what I meant, thx :)

[NeHuang]

Previously we have an assumption that speculatable callee will be moved down if there are intermediate instructions (not using the result of callee) between it and the return instruction. However, the assumption does not hold in our experiment that no passes in opt or llc will move the speculatable callee down. Therefore we need to handle speculatable calls same as the other calls in bool llvm::isInTailCallPosition that checking if it is in a tail call position.

Added a new test case that intermediate instructions (not use result of callee) between speculatable callee and the return instruction.

[jdoerfert]

LGTM, one nit below.

llvm/test/CodeGen/PowerPC/tailcall-speculatable-callee.ll

61

Please add a FIXME explaining this call could be moved to make it a tail call.

llvm/lib/CodeGen/Analysis.cpp

bool llvm::isInTailCallPosition(const CallBase &Call, const TargetMachine &TM) {
   // been fully understood.
   if (!Ret &&
       ((!TM.Options.GuaranteedTailCallOpt &&
         Call.getCallingConv() != CallingConv::Tail) || !isa<UnreachableInst>(Term)))
     return false;
 
   // If I will have a chain, make sure no other instruction that will have a
   // chain interposes between I and the return.
-  if (Call.mayHaveSideEffects() || Call.mayReadFromMemory() ||
-      !isSafeToSpeculativelyExecute(&Call))
-    for (BasicBlock::const_iterator BBI = std::prev(ExitBB->end(), 2);; --BBI) {
-      if (&*BBI == &Call)
-        break;
-      // Debug info intrinsics do not get in the way of tail call optimization.
-      if (isa<DbgInfoIntrinsic>(BBI))
-        continue;
-      // A lifetime end or assume intrinsic should not stop tail call
-      // optimization.
-      if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(BBI))
-        if (II->getIntrinsicID() == Intrinsic::lifetime_end ||
-            II->getIntrinsicID() == Intrinsic::assume)
-          continue;
-      if (BBI->mayHaveSideEffects() || BBI->mayReadFromMemory() ||
-          !isSafeToSpeculativelyExecute(&*BBI))
-        return false;
-    }
+  // Check for all calls including speculatable functions.
 
   const Function *F = ExitBB->getParent();
   return returnTypeIsEligibleForTailCall(
       F, &Call, Ret, *TM.getSubtargetImpl(*F)->getTargetLowering());
 }
 
 bool llvm::attributesPermitTailCall(const Function *F, const Instruction *I,
                                     const ReturnInst *Ret,

llvm/test/CodeGen/PowerPC/tailcall-speculatable-callee.ll

+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | FileCheck %s
+
+; The tests check the behavior of the tail call decision when the callee is speculatable.
+
+; Callee should be tail called in this function since it is at a tail call position.
+define dso_local double @speculatable_callee_return_use_only (double* nocapture %res, double %a) #0 {
+; CHECK-LABEL: speculatable_callee_return_use_only:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: b callee
+entry:
+  %value = tail call double @callee(double %a) #2
+  ret double %value
+}
+
+; Callee should not be tail called since it is not at a tail call position.
+define dso_local void @speculatable_callee_non_return_use_only (double* nocapture %res, double %a) #0 {
+; CHECK-LABEL: speculatable_callee_non_return_use_only:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: mflr r0
+; CHECK-NEXT: std r30, -16(r1)  # 8-byte Folded Spill
+; CHECK-NEXT: std r0, 16(r1)
+; CHECK-NEXT: stdu r1, -48(r1)
+; CHECK-NEXT: mr r30, r3
+; CHECK-NEXT: bl callee
+; CHECK-NEXT: stfdx f1, 0, r30
+; CHECK-NEXT: addi r1, r1, 48
+; CHECK-NEXT: ld r0, 16(r1)
+; CHECK-NEXT: ld r30, -16(r1) # 8-byte Folded Reload
+; CHECK-NEXT: mtlr r0
+; CHECK-NEXT: blr
+entry:
+  %call = tail call double @callee(double %a) #2
+  store double %call, double* %res, align 8
+  ret void
+}
+
+; Callee should not be tail called since it is not at a tail call position.
+define dso_local double @speculatable_callee_multi_use (double* nocapture %res, double %a) #0 {
+  ; CHECK-LABEL: speculatable_callee_multi_use:
+  ; CHECK: # %bb.0: # %entry
+  ; CHECK-NEXT: mflr r0
+  ; CHECK-NEXT: std r30, -16(r1)  # 8-byte Folded Spill
+  ; CHECK-NEXT: std r0, 16(r1)
+  ; CHECK-NEXT: stdu r1, -48(r1)
+  ; CHECK-NEXT: mr r30, r3
+  ; CHECK-NEXT: bl callee
+  ; CHECK-NEXT: stfdx f1, 0, r30
+  ; CHECK-NEXT: addi r1, r1, 48
+  ; CHECK-NEXT: ld r0, 16(r1)
+  ; CHECK-NEXT: ld r30, -16(r1) # 8-byte Folded Reload
+  ; CHECK-NEXT: mtlr r0
+  ; CHECK-NEXT: blr
+  entry:
+  %call = tail call double @callee(double %a) #2
+  store double %call, double* %res, align 8
+  ret double %call
+}
+
+; Callee should not be tail called since it is not at a tail call position.
+; FIXME: A speculatable callee can be tail called if it is moved into a valid tail call position.
+define dso_local double @speculatable_callee_intermediate_instructions (double* nocapture %res, double %a) #0 {
+  ; CHECK-LABEL: speculatable_callee_intermediate_instructions:
+  ; CHECK: # %bb.0: # %entry
+  ; CHECK-NEXT: mflr r0
+  ; CHECK-NEXT: std r30, -16(r1)  # 8-byte Folded Spill
+  ; CHECK-NEXT: std r0, 16(r1)
+  ; CHECK-NEXT: stdu r1, -48(r1)
+  ; CHECK-NEXT: mr r30, r3
+  ; CHECK-NEXT: bl callee
+  ; CHECK-NEXT: lis r3, 16404
+  ; CHECK-NEXT: ori r3, r3, 52428
+  ; CHECK-NEXT: sldi r3, r3, 32
+  ; CHECK-NEXT: oris r3, r3, 52428
+  ; CHECK-NEXT: ori r3, r3, 52429
+  ; CHECK-NEXT: std r3, 0(r30)
+  ; CHECK-NEXT: addi r1, r1, 48
+  ; CHECK-NEXT: ld r0, 16(r1)
+  ; CHECK-NEXT: ld r30, -16(r1)  # 8-byte Folded Reload
+  ; CHECK-NEXT: mtlr r0
+  ; CHECK-NEXT: blr
+
+  entry:
+  %call = tail call double @callee(double %a) #2
+  store double 5.2, double* %res, align 8
+  ret double %call
+}
+
+
+define double @callee(double) #1 {
+  ret double 4.5
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { readnone speculatable }
+attributes #2 = { nounwind noinline }