[CGP] Look through bitcasts when duplicating returns for tail calls

[thegameg]

Summary

The simple case of:

int *callee();
void *caller(void *a) {
  if (a == NULL)
    return callee();
  return a;
}

would generate a regular call instead of a tail call because we don't
look through the bitcast of the call to callee when duplicating the
return blocks.

[spatel]

llvm/lib/CodeGen/CodeGenPrepare.cpp

2035–2037 ↗

(On Diff #195631)

There is a peekThroughBitcast() util function in instcombine. Might be worth lifting to some common location, so we don't have to repeat it in other IR passes.

llvm/test/CodeGen/X86/tailcall-cgp-dup.ll

110–112 ↗

(On Diff #195631)

It's nice to have a codegen test, but the transform occurs in IR, so I'd prefer to see an 'opt -codegenprepare' test for this too. Preferably, that test (and this test) would be added to trunk before this patch, so we just see the improvement here. Finally, I prefer that FileCheck assertions are auto-generated using the scripts in the llvm/utils folder.

[thegameg]

• Use Value:: stripPointerCasts instead of checking for BitCastInst.
• Add opt -codegenprepare test and full codegen test separately (to commit before this).
• Use utils/update_test_checks.py and utils/update_llc_test_checks.py for the new tests.

[thegameg]

llvm/lib/CodeGen/CodeGenPrepare.cpp

2035–2037 ↗

(On Diff #195631)

Value::stripPointerCasts seems to be doing similar things, so I used that here.

llvm/test/CodeGen/X86/tailcall-cgp-dup.ll

110–112 ↗

(On Diff #195631)

Thanks, I agree, this is much nicer.

[spatel]

LGTM.

Note that there is a dedicated folder for CGP IR tests at 'llvm/test/Transforms/CodeGenPrepare' with target subfolders under there, but there's also precedence for doing it this way, so we see IR and asm in the same file.

[thegameg]

In D60837#1476259, @spatel wrote:

LGTM.

Thanks!

Note that there is a dedicated folder for CGP IR tests at 'llvm/test/Transforms/CodeGenPrepare' with target subfolders under there, but there's also precedence for doing it this way, so we see IR and asm in the same file.

Oh, good to know, thanks!

llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB, bool &ModifiedDT) {
   }
 
   /// Only dup the ReturnInst if the CallInst is likely to be emitted as a tail
   /// call.
   const Function *F = BB->getParent();
   SmallVector<CallInst*, 4> TailCalls;
   if (PN) {
     for (unsigned I = 0, E = PN->getNumIncomingValues(); I != E; ++I) {
-      CallInst *CI = dyn_cast<CallInst>(PN->getIncomingValue(I));
+      // Look through bitcasts.
+      Value *IncomingVal = PN->getIncomingValue(I)->stripPointerCasts();
+      CallInst *CI = dyn_cast<CallInst>(IncomingVal);
       // Make sure the phi value is indeed produced by the tail call.
       if (CI && CI->hasOneUse() && CI->getParent() == PN->getIncomingBlock(I) &&
           TLI->mayBeEmittedAsTailCall(CI) &&
           attributesPermitTailCall(F, CI, RetI, *TLI))
         TailCalls.push_back(CI);
     }
   } else {
     SmallPtrSet<BasicBlock*, 4> VisitedBBs;

llvm/trunk/test/CodeGen/X86/tailcall-cgp-dup.ll

 define i8* @f_ret8(i8* %obj) nounwind {
 ; OPT-LABEL: @f_ret8(
 ; OPT-NEXT:  entry:
 ; OPT-NEXT:    [[CMP:%.*]] = icmp eq i8* [[OBJ:%.*]], null
 ; OPT-NEXT:    br i1 [[CMP]], label [[RETURN:%.*]], label [[IF_THEN:%.*]]
 ; OPT:       if.then:
 ; OPT-NEXT:    [[PTR:%.*]] = tail call i32* @g_ret32()
 ; OPT-NEXT:    [[CASTED:%.*]] = bitcast i32* [[PTR]] to i8*
-; OPT-NEXT:    br label [[RETURN]]
+; OPT-NEXT:    ret i8* [[CASTED]]
 ; OPT:       return:
-; OPT-NEXT:    [[RETVAL:%.*]] = phi i8* [ [[CASTED]], [[IF_THEN]] ], [ [[OBJ]], [[ENTRY:%.*]] ]
-; OPT-NEXT:    ret i8* [[RETVAL]]
+; OPT-NEXT:    ret i8* [[OBJ]]
 ;
 ; CHECK-LABEL: f_ret8:
 ; CHECK:       ## %bb.0: ## %entry
-; CHECK-NEXT:    movq %rdi, %rax
 ; CHECK-NEXT:    testq %rdi, %rdi
-; CHECK-NEXT:    je LBB3_2
-; CHECK-NEXT:  ## %bb.1: ## %if.then
-; CHECK-NEXT:    pushq %rax
-; CHECK-NEXT:    callq _g_ret32
-; CHECK-NEXT:    addq $8, %rsp
-; CHECK-NEXT:  LBB3_2: ## %return
+; CHECK-NEXT:    je LBB3_1
+; CHECK-NEXT:  ## %bb.2: ## %if.then
+; CHECK-NEXT:    jmp _g_ret32 ## TAILCALL
+; CHECK-NEXT:  LBB3_1: ## %return
+; CHECK-NEXT:    movq %rdi, %rax
 ; CHECK-NEXT:    retq
 entry:
   %cmp = icmp eq i8* %obj, null
   br i1 %cmp, label %return, label %if.then
 
 if.then:
   %ptr = tail call i32* @g_ret32()
   %casted = bitcast i32* %ptr to i8*
   br label %return
 
 return:
   %retval = phi i8* [ %casted, %if.then ], [ %obj, %entry ]
   ret i8* %retval
 }