Create PHI node for the return value only when the return value has uses.

[twoh]

Summary

Currently, a phi node is created in the normal destination to unify the return values from promoted calls and the original indirect call. This patch makes this phi node to be created only when the return value has uses.

This patch is necessary to generate valid code, as compiler crashes with the attached test case without this patch. Without this patch, an illegal phi node that has no incoming value from entry/catch is created in cleanup block.

I think existing implementation is good as far as there is at least one use of the original indirect call. insertCallRetPHI creates a new phi node in the normal destination block only when the original indirect call dominates its use and the normal destination block. Otherwise, fixupPHINodeForNormalDest will handle the unification of return values naturally without creating a new phi node. However, if there's no use, insertCallRetPHI still creates a new phi node even when the original indirect call does not dominate the normal destination block, because getCallRetPHINode returns false.

[davidxl]

lib/Transforms/Instrumentation/IndirectCallPromotion.cpp

592 ↗

(On Diff #112781)

may be wrap this check inside the function just as other checks?

[twoh]

Fix a test.

[xur]

Thanks for the fix.

LGTM, but I agree with David that it's preferable to put the check inside insertCallRetPHI() (around isVoidTy()).

[twoh]

Addressing @davidxl and @xur's comment. Thanks!

[xur]

LGTM

llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp

 }
 
 // Create a PHI to unify the return values of calls.
 static void insertCallRetPHI(Instruction *Inst, Instruction *CallResult,
                              Function *DirectCallee) {
   if (Inst->getType()->isVoidTy())
     return;
 
+  if (Inst->use_empty())
+    return;
+
   BasicBlock *RetValBB = CallResult->getParent();
 
   BasicBlock *PHIBB;
   if (InvokeInst *II = dyn_cast<InvokeInst>(CallResult))
     RetValBB = II->getNormalDest();
 
   PHIBB = RetValBB->getSingleSuccessor();
   if (getCallRetPHINode(PHIBB, Inst))

llvm/trunk/test/Transforms/PGOProfile/icp_covariant_call_return.ll

 ; RUN: opt < %s -pgo-icall-prom -S | FileCheck %s --check-prefix=ICALL-PROM
 ; RUN: opt < %s -passes=pgo-icall-prom -S | FileCheck %s --check-prefix=ICALL-PROM
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 %struct.D = type { %struct.B }
 %struct.B = type { i32 (...)** }
 %struct.Base = type { i8 }
 %struct.Derived = type { i8 }
 
 declare noalias i8* @_Znwm(i64)
 declare void @_ZN1DC2Ev(%struct.D*);
 declare %struct.Derived* @_ZN1D4funcEv(%struct.D*);
 
-define i32 @bar() {
+define %struct.Base* @bar() {
 entry:
   %call = call noalias i8* @_Znwm(i64 8)
   %tmp = bitcast i8* %call to %struct.D*
   call void @_ZN1DC2Ev(%struct.D* %tmp)
   %tmp1 = bitcast %struct.D* %tmp to %struct.B*
   %tmp2 = bitcast %struct.B* %tmp1 to %struct.Base* (%struct.B*)***
   %vtable = load %struct.Base* (%struct.B*)**, %struct.Base* (%struct.B*)*** %tmp2, align 8
   %vfn = getelementptr inbounds %struct.Base* (%struct.B*)*, %struct.Base* (%struct.B*)** %vtable, i64 0
   %tmp3 = load %struct.Base* (%struct.B*)*, %struct.Base* (%struct.B*)** %vfn, align 8
 ; ICALL-PROM:  [[BITCAST:%[0-9]+]] = bitcast %struct.Base* (%struct.B*)* %tmp3 to i8*
 ; ICALL-PROM:  [[CMP:%[0-9]+]] = icmp eq i8* [[BITCAST]], bitcast (%struct.Derived* (%struct.D*)* @_ZN1D4funcEv to i8*)
 ; ICALL-PROM:  br i1 [[CMP]], label %if.true.direct_targ, label %if.false.orig_indirect, !prof [[BRANCH_WEIGHT:![0-9]+]]
 ; ICALL-PROM:if.true.direct_targ:
 ; ICALL-PROM:  [[ARG_BITCAST:%[0-9]+]] = bitcast %struct.B* %tmp1 to %struct.D*
 ; ICALL-PROM:  [[DIRCALL_RET:%[0-9]+]] = call %struct.Derived* @_ZN1D4funcEv(%struct.D* [[ARG_BITCAST]])
 ; ICALL-PROM:  [[DIRCALL_RET_CAST:%[0-9]+]] = bitcast %struct.Derived* [[DIRCALL_RET]] to %struct.Base*
 ; ICALL-PROM:  br label %if.end.icp 
 ; ICALL-PROM:if.false.orig_indirect:
 ; ICALL-PROM:  %call1 = call %struct.Base* %tmp3(%struct.B* %tmp1)
 ; ICALL-PROM:  br label %if.end.icp
 ; ICALL-PROM:if.end.icp:
 ; ICALL-PROM:  [[PHI_RET:%[0-9]+]] = phi %struct.Base* [ %call1, %if.false.orig_indirect ], [ [[DIRCALL_RET_CAST]], %if.true.direct_targ ]
   %call1 = call %struct.Base* %tmp3(%struct.B* %tmp1), !prof !1
-  ret i32 0
+  ret %struct.Base* %call1
 }
 
 !1 = !{!"VP", i32 0, i64 12345, i64 -3913987384944532146, i64 12345}
 ; ICALL-PROM-NOT: !1 = !{!"VP", i32 0, i64 12345, i64 -3913987384944532146, i64 12345}
 ; ICALL-PROM: [[BRANCH_WEIGHT]] = !{!"branch_weights", i32 12345, i32 0}
 ; ICALL-PROM-NOT: !1 = !{!"VP", i32 0, i64 12345, i64 -3913987384944532146, i64 12345}

llvm/trunk/test/Transforms/PGOProfile/icp_invoke_nouse.ll

+; RUN: opt < %s -icp-lto -pgo-icall-prom -S | FileCheck %s --check-prefix=ICP
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@_ZTISt9exception = external constant i8*
+@pfptr = global i32()* null, align 8
+
+define internal i32 @_ZL4bar1v() !PGOFuncName !0 {
+entry:
+  ret i32 100 
+}
+
+; Function Attrs: uwtable
+define i32 @_Z3fooi(i32 %x) local_unnamed_addr personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %tobool = icmp eq i32 %x, 0
+  br i1 %tobool, label %if.end, label %cleanup
+
+if.end:                                           ; preds = %entry
+  %fptr = load i32 ()*, i32 ()** @pfptr, align 8
+; ICP:  [[BITCAST_IC1:%[0-9]+]] = bitcast i32 ()* %fptr to i8*
+; ICP:  [[CMP_IC1:%[0-9]+]] = icmp eq i8* [[BITCAST_IC1]], bitcast (i32 ()* @_ZL4bar1v to i8*)
+; ICP:  br i1 [[CMP_IC1]], label %[[TRUE_LABEL_IC1:.*]], label %[[FALSE_LABEL_IC1:.*]], !prof [[BRANCH_WEIGHT:![0-9]+]]
+; ICP:[[TRUE_LABEL_IC1]]:
+; ICP:  invoke i32 @_ZL4bar1v()
+; ICP:          to label %[[DCALL_NORMAL_DEST_IC1:.*]] unwind label %lpad
+; ICP:[[FALSE_LABEL_IC1]]:
+  %call = invoke i32 %fptr()
+          to label %cleanup unwind label %lpad, !prof !1
+
+; ICP:[[DCALL_NORMAL_DEST_IC1]]:
+; ICP:  br label %cleanup
+
+lpad:                                             ; preds = %if.end
+  %0 = landingpad { i8*, i32 }
+          cleanup
+          catch i8* bitcast (i8** @_ZTISt9exception to i8*)
+  %1 = extractvalue { i8*, i32 } %0, 1
+  %2 = tail call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTISt9exception to i8*))
+  %matches = icmp eq i32 %1, %2
+  br i1 %matches, label %catch, label %ehcleanup
+
+catch:                                            ; preds = %lpad
+  %3 = extractvalue { i8*, i32 } %0, 0
+  %4 = tail call i8* @__cxa_begin_catch(i8* %3)
+  tail call void @__cxa_end_catch()
+  br label %cleanup
+
+cleanup:                                          ; preds = %catch, %if.end, %entry
+; ICP-NOT: %[0-9]+ = phi 
+  ret i32 0
+
+ehcleanup:                                        ; preds = %lpad
+  resume { i8*, i32 } %0
+}
+
+declare i32 @_Z3barv() local_unnamed_addr
+
+declare i32 @__gxx_personality_v0(...)
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.eh.typeid.for(i8*)
+
+declare i8* @__cxa_begin_catch(i8*) local_unnamed_addr
+
+declare void @__cxa_end_catch() local_unnamed_addr
+
+!0 = !{!"invoke.ll:_ZL4bar1v"}
+!1 = !{!"VP", i32 0, i64 10000, i64 -2732222848796217051, i64 10000}