[CGProfile] don't emit cgprofile entry if called function is dllimport

[rnk]

I think this is reasonable. There is no need to record call graph profile edges from functions to imported functions in the metadata. However, I would prefer to move the check into CodeGen, so that if we receive some IR that has edges like this, we don't emit an object file that cannot be linked. I'd be OK doing the check in both places.

[zequanwu]

In D88127#2291167, @rnk wrote:

However, I would prefer to move the check into CodeGen, so that if we receive some IR that has edges like this, we don't emit an object file that cannot be linked.

I think we just want to do it here, simply don't generate the "CG Profile" metadata entry for dllimport function, since the IR could complied to assembly or object file. Otherwise, we need two checks in both MCAsmStreamer and MCCOFFStreamer.

[rnk]

I'm saying the fix belongs in TargetObjectFileLoweringImpl.cpp, which is the point where we translate from IR to assembly. We can't fix this after producing assembly, because we don't know what symbols are marked dllimport at that point.

[zequanwu]

In D88127#2291320, @rnk wrote:

I'm saying the fix belongs in TargetObjectFileLoweringImpl.cpp, which is the point where we translate from IR to assembly. We can't fix this after producing assembly, because we don't know what symbols are marked dllimport at that point.

That's reverted. (https://reviews.llvm.org/rG91aed9bf975f1e4346cc8f4bdefc98436386ced2). I don't know if I should put two patches together.

[rnk]

In D88127#2291342, @zequanwu wrote:

That's reverted. (https://reviews.llvm.org/rG91aed9bf975f1e4346cc8f4bdefc98436386ced2). I don't know if I should put two patches together.

Well, normally code that has been reverted cannot be relanded without changes, so it's common to revert the revert locally, edit it, and reupload it for code review. I believe you can even "reopen" the closed code review issue that you used for the first review, and it will show the changes you made clearly.

[rnk]

In any case, if you want to do that separately, I think this change stands on its own, so I will approve it as is.

llvm/lib/Transforms/Instrumentation/CGProfile.cpp

 
 static bool runCGProfilePass(
     Module &M, function_ref<BlockFrequencyInfo &(Function &)> GetBFI,
     function_ref<TargetTransformInfo &(Function &)> GetTTI, bool LazyBFI) {
   MapVector<std::pair<Function *, Function *>, uint64_t> Counts;
   InstrProfSymtab Symtab;
   auto UpdateCounts = [&](TargetTransformInfo &TTI, Function *F,
                           Function *CalledF, uint64_t NewCount) {
-    if (!CalledF || !TTI.isLoweredToCall(CalledF))
+    if (!CalledF || !TTI.isLoweredToCall(CalledF) ||
+        CalledF->hasDLLImportStorageClass())
       return;
     uint64_t &Count = Counts[std::make_pair(F, CalledF)];
     Count = SaturatingAdd(Count, NewCount);
   };
   // Ignore error here.  Indirect calls are ignored if this fails.
   (void)(bool) Symtab.create(M);
   for (auto &F : M) {
     // Avoid extra cost of running passes for BFI when the function doesn't have

llvm/test/Instrumentation/cgprofile.ll

 ; RUN: opt < %s -passes cg-profile -S | FileCheck %s
 ; RUN: opt < %s -cg-profile -S | FileCheck %s
 
 declare void @b()
 
 define void @a() !prof !1 {
   call void @b()
   ret void
 }
 
 @foo = common global i32 ()* null, align 8
 declare i32 @func1()
 declare i32 @func2()
 declare i32 @func3()
 declare i32 @func4()
+declare dllimport i32 @func5()
 
 define void @freq(i1 %cond) !prof !1 {
   %tmp = load i32 ()*, i32 ()** @foo, align 8
   call i32 %tmp(), !prof !3
   br i1 %cond, label %A, label %B, !prof !2
 A:
   call void @a();
   ret void
 B:
   call void @b();
   ret void
 }
 
 !1 = !{!"function_entry_count", i64 32}
 !2 = !{!"branch_weights", i32 5, i32 10}
-!3 = !{!"VP", i32 0, i64 1600, i64 7651369219802541373, i64 1030, i64 -4377547752858689819, i64 410, i64 -6929281286627296573, i64 150, i64 -2545542355363006406, i64 10}
+!3 = !{!"VP", i32 0, i64 1600, i64 7651369219802541373, i64 1030, i64 -4377547752858689819, i64 410, i64 -6929281286627296573, i64 150, i64 -2545542355363006406, i64 10, i64 3667884930908592509, i64 0}
 
 ; CHECK: !llvm.module.flags = !{![[cgprof:[0-9]+]]}
 ; CHECK: ![[cgprof]] = !{i32 5, !"CG Profile", ![[prof:[0-9]+]]}
 ; CHECK: ![[prof]] = !{![[e0:[0-9]+]], ![[e1:[0-9]+]], ![[e2:[0-9]+]], ![[e3:[0-9]+]], ![[e4:[0-9]+]], ![[e5:[0-9]+]], ![[e6:[0-9]+]]}
 ; CHECK: ![[e0]] = !{void ()* @a, void ()* @b, i64 32}
 ; CHECK: ![[e1]] = !{void (i1)* @freq, i32 ()* @func4, i64 1030}
 ; CHECK: ![[e2]] = !{void (i1)* @freq, i32 ()* @func2, i64 410}
 ; CHECK: ![[e3]] = !{void (i1)* @freq, i32 ()* @func3, i64 150}
 ; CHECK: ![[e4]] = !{void (i1)* @freq, i32 ()* @func1, i64 10}
 ; CHECK: ![[e5]] = !{void (i1)* @freq, void ()* @a, i64 11}
 ; CHECK: ![[e6]] = !{void (i1)* @freq, void ()* @b, i64 21}
+; CHECK-NOT: !{void (i1)* @freq, void ()* @func5, i64 0}
 No newline at end of file