[llvm] 47c1f27 - Properly add out-of-module functions to the import list

[Hongtao Yu via llvm-commits]

Author: Hongtao Yu
Date: 2020-05-11T10:00:14-07:00
New Revision: 47c1f2741fe7d24e02c3f54bcb723994292c54c3

URL: https://github.com/llvm/llvm-project/commit/47c1f2741fe7d24e02c3f54bcb723994292c54c3
DIFF: https://github.com/llvm/llvm-project/commit/47c1f2741fe7d24e02c3f54bcb723994292c54c3.diff

LOG: Properly add out-of-module functions to the import list

This patch addresses two issues related to adding inline functions to the import list while recursively going through the profiling data.
1. For callsite samples, only add an inlined function to the import list if it's from outside of the module (i.e. only has a declaration inside the module).
2. For body samples, add each target function to the import list if it's from outside of the module (i.e. only has a declaration inside the module). Previously we were using getSubProgram() to check whether it has dbg info, which is inaccurate. This fix properly add imports and could improve the quality of the pass.

Added a few changes to the test to catch these cases.

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

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/SampleProf.h
    llvm/test/Transforms/SampleProfile/Inputs/function_metadata.compact.afdo
    llvm/test/Transforms/SampleProfile/Inputs/function_metadata.prof
    llvm/test/Transforms/SampleProfile/function_metadata.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index b2975bf6a73f..af636a95bbf8 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -527,14 +527,20 @@ class FunctionSamples {
                             uint64_t Threshold) const {
     if (TotalSamples <= Threshold)
       return;
-    S.insert(getGUID(Name));
+    auto isDeclaration = [](const Function *F) {
+      return !F || F->isDeclaration();
+    };
+    if (isDeclaration(M->getFunction(getFuncName()))) {
+      // Add to the import list only when it's defined out of module.
+      S.insert(getGUID(Name));
+    }
     // Import hot CallTargets, which may not be available in IR because full
     // profile annotation cannot be done until backend compilation in ThinLTO.
     for (const auto &BS : BodySamples)
       for (const auto &TS : BS.second.getCallTargets())
         if (TS.getValue() > Threshold) {
           const Function *Callee = M->getFunction(getFuncName(TS.getKey()));
-          if (!Callee || !Callee->getSubprogram())
+          if (isDeclaration(Callee))
             S.insert(getGUID(TS.getKey()));
         }
     for (const auto &CS : CallsiteSamples)

diff  --git a/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.compact.afdo b/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.compact.afdo
index 20bd89670777..c9de6e755c43 100644
Binary files a/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.compact.afdo and b/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.compact.afdo 
diff er

diff  --git a/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.prof b/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.prof
index 621bed722b7c..4ed3a681ddd6 100644
--- a/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.prof
+++ b/llvm/test/Transforms/SampleProfile/Inputs/function_metadata.prof
@@ -13,5 +13,6 @@ test:3200:0
   1: 1000 foo3:1000
 test_liveness:1000:0
  1: foo:1000
-  1: foo_available:1000
+  1: bar:1000
+   1: 2000 bar_dbg:1000 bar_available:1000
    2: 1000

diff  --git a/llvm/test/Transforms/SampleProfile/function_metadata.ll b/llvm/test/Transforms/SampleProfile/function_metadata.ll
index 0e772e86f72b..36d2d080fc0f 100644
--- a/llvm/test/Transforms/SampleProfile/function_metadata.ll
+++ b/llvm/test/Transforms/SampleProfile/function_metadata.ll
@@ -6,7 +6,11 @@
 
 declare void @foo()
 
-define void @foo_available() !dbg !11 {
+declare void @bar()
+
+declare !dbg !13 void @bar_dbg()
+
+define void @bar_available() !dbg !14 {
   ret void
 }
 
@@ -33,10 +37,10 @@ define void @test_liveness() !dbg !12 {
 ; metadata.
 ; CHECK: ![[ENTRY_TEST]] = !{!"function_entry_count", i64 1, i64 2494702099028631698, i64 6699318081062747564, i64 7682762345278052905,  i64 -7908226060800700466, i64 -2012135647395072713}
 
-; Check GUIDs for both foo and foo_available are included in the metadata to
+; Check GUIDs for foo, bar and bar_dbg are included in the metadata to
 ; make sure the liveness analysis can capture the dependency from test_liveness
-; to foo_available.
-; CHECK: ![[ENTRY_TEST_LIVENESS]] = !{!"function_entry_count", i64 1, i64 4005816710939881937, i64 6699318081062747564}
+; to bar. bar_available should not be included as it's within the same module.
+; CHECK: ![[ENTRY_TEST_LIVENESS]] = !{!"function_entry_count", i64 1, i64 6699318081062747564, i64 -2012135647395072713, i64 -1522495160813492905}
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9}
@@ -50,8 +54,9 @@ define void @test_liveness() !dbg !12 {
 !8 = !{i32 2, !"Dwarf Version", i32 4}
 !9 = !{i32 1, !"Debug Info Version", i32 3}
 !10 = !{!"clang version 3.5 "}
-!11 = distinct !DISubprogram(name: "foo_available", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !1, type: !6, retainedNodes: !2)
 !12 = distinct !DISubprogram(name: "test_liveness", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !1, type: !6, retainedNodes: !2)
+!13 = !DISubprogram(name: "bar_dbg", scope: !1, file: !1, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!14 = distinct !DISubprogram(name: "bar_available", line: 7, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 7, file: !1, scope: !1, type: !6, retainedNodes: !2)
 !15 = !DILexicalBlockFile(discriminator: 1, file: !1, scope: !7)
 !17 = distinct !DILexicalBlock(line: 10, column: 0, file: !1, scope: !7)
 !18 = !DILocation(line: 10, scope: !17)