Fix the samplepgo indirect call promotion bug: we should not promote a direct call.

Summary: Checking CS.getCalledFunction() == nullptr does not necessary indicate indirect call. We also need to check if CS.getCalledValue() is not a constant.

Reviewers: davidxl

Reviewed By: davidxl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D29570

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294260 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Analysis/IndirectCallSiteVisitor.h

@@ -21,16 +21,8 @@ struct PGOIndirectCallSiteVisitor
   PGOIndirectCallSiteVisitor() {}
 
   void visitCallSite(CallSite CS) {
-    if (CS.getCalledFunction() || !CS.getCalledValue())
-      return;
-    Instruction *I = CS.getInstruction();
-    if (CallInst *CI = dyn_cast<CallInst>(I)) {
-      if (CI->isInlineAsm())
-        return;
-    }
-    if (isa<Constant>(CS.getCalledValue()))
-      return;
-    IndirectCallInsts.push_back(I);
+    if (CS.isIndirectCall())
+      IndirectCallInsts.push_back(CS.getInstruction());
   }
 };
 

include/llvm/IR/CallSite.h

@@ -111,6 +111,20 @@ public:
     return dyn_cast<FunTy>(getCalledValue());
   }
 
+  /// Returns true if the callsite is an indirect call
+  bool isIndirectCall() const {
+    Value *V = getCalledValue();
+    if (!V)
+      return false;
+    if (isa<FunTy>(V) || isa<Constant>(V))
+      return false;
+    if (CallInst *CI = dyn_cast<CallInst>(getInstruction())) {
+      if (CI->isInlineAsm())
+        return false;
+    }
+    return true;
+  }
+
   /// setCalledFunction - Set the callee to the specified value.
   ///
   void setCalledFunction(Value *V) {

lib/Transforms/IPO/SampleProfile.cpp

@@ -634,7 +634,8 @@ bool SampleProfileLoader::inlineHotFunctions(Function &F) {
       InlineFunctionInfo IFI(nullptr, ACT ? &GetAssumptionCache : nullptr);
       Function *CalledFunction = CallSite(I).getCalledFunction();
       Instruction *DI = I;
-      if (!CalledFunction && !PromotedInsns.count(I)) {
+      if (!CalledFunction && !PromotedInsns.count(I) &&
+          CallSite(I).isIndirectCall()) {
         auto CalleeFunctionName = findCalleeFunctionSamples(*I)->getName();
         const char *Reason = "Callee function not available";
         CalledFunction = F.getParent()->getFunction(CalleeFunctionName);

test/Transforms/SampleProfile/Inputs/indirect-call.prof

@@ -6,3 +6,6 @@ test_inline:3000:0
 test_noinline:3000:0
  5: foo_noinline:3000
   1: 3000
+test_direct:3000:0
+ 5: foo_direct:3000
+  1: 3000

test/Transforms/SampleProfile/indirect-call.ll

@@ -47,6 +47,21 @@ define i32 @foo_noinline(i32 %x) !dbg !3 {
   ret i32 %x
 }
 
+define void @foo_direct() !dbg !3 {
+  ret void
+}
+
+; CHECK-LABEL: @test_direct
+; We should not promote a direct call.
+define void @test_direct() !dbg !3 {
+; CHECK-NOT: icmp
+; CHECK: call
+  call void @foo_alias(), !dbg !5
+  ret void
+}
+
+@foo_alias = alias void (), void ()* @foo_direct
+
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!2}