From 4a9dd70213f5359b5dc5b3772f81a0b1c40ee55b Mon Sep 17 00:00:00 2001
From: Dehao Chen <dehao@google.com>
Date: Mon, 6 Feb 2017 23:33:15 +0000
Subject: [PATCH] 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

llvm-svn: 294260
---
 .../llvm/Analysis/IndirectCallSiteVisitor.h       | 12 ++----------
 llvm/include/llvm/IR/CallSite.h                   | 14 ++++++++++++++
 llvm/lib/Transforms/IPO/SampleProfile.cpp         |  3 ++-
 .../SampleProfile/Inputs/indirect-call.prof       |  3 +++
 .../Transforms/SampleProfile/indirect-call.ll     | 15 +++++++++++++++
 5 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/Analysis/IndirectCallSiteVisitor.h b/llvm/include/llvm/Analysis/IndirectCallSiteVisitor.h
index 71a8cb886321..3c40cc0235cc 100644
--- a/llvm/include/llvm/Analysis/IndirectCallSiteVisitor.h
+++ b/llvm/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());
   }
 };
 
diff --git a/llvm/include/llvm/IR/CallSite.h b/llvm/include/llvm/IR/CallSite.h
index b02c89474146..18601706f511 100644
--- a/llvm/include/llvm/IR/CallSite.h
+++ b/llvm/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) {
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 87015272b96a..d66782793551 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/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);
diff --git a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof b/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
index 9ebfd77147d0..ac32967bd546 100644
--- a/llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof
+++ b/llvm/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
diff --git a/llvm/test/Transforms/SampleProfile/indirect-call.ll b/llvm/test/Transforms/SampleProfile/indirect-call.ll
index c868d0b83a27..e6e294fd6bfd 100644
--- a/llvm/test/Transforms/SampleProfile/indirect-call.ll
+++ b/llvm/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}