[PATCH] D84994: [SampleFDO] Fix a crash when the sample profile uses md5 and -sample-profile-merge-inlinee is enabled

[Wei Mi via Phabricator via llvm-commits]

wmi created this revision.
wmi added reviewers: wenlei, davidxl.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
wmi requested review of this revision.

When -sample-profile-merge-inlinee is enabled, new FunctionSamples may be created during profile merge without GUIDToFuncNameMap being initialized. That will occasionally cause compiler crash. The patch fixes it.


Repository:
  rL LLVM

https://reviews.llvm.org/D84994

Files:
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/test/Transforms/SampleProfile/Inputs/inline-mergeprof.md5.prof
  llvm/test/Transforms/SampleProfile/inline-mergeprof.ll


Index: llvm/test/Transforms/SampleProfile/inline-mergeprof.ll
===================================================================
--- llvm/test/Transforms/SampleProfile/inline-mergeprof.ll
+++ llvm/test/Transforms/SampleProfile/inline-mergeprof.ll
@@ -3,9 +3,13 @@
 ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline-mergeprof.prof -sample-profile-merge-inlinee=true -S | FileCheck -check-prefix=SCALE %s
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-mergeprof.prof -sample-profile-merge-inlinee=false -S | FileCheck -check-prefix=SCALE %s
 
-; Test we properly merge not inlined profile properly with '-sample-profile-merge-inlinee'
+; Test we properly merge not inlined profile with '-sample-profile-merge-inlinee'
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-mergeprof.prof -sample-profile-merge-inlinee=true -S | FileCheck -check-prefix=MERGE  %s
 
+; Test we properly merge not inlined profile with '-sample-profile-merge-inlinee'
+; when the profile uses md5.
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline-mergeprof.md5.prof -sample-profile-merge-inlinee=true -S | FileCheck -check-prefix=MERGE  %s
+
 @.str = private unnamed_addr constant [11 x i8] c"sum is %d\0A\00", align 1
 
 define i32 @main() #0 !dbg !6 {
Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===================================================================
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -233,6 +233,24 @@
   SampleProfileLoader &SPLoader;
 };
 
+/// Set GUIDToFuncNameMap for FunctionSamples in FSToUpdate and all the
+/// callsite FunctionSamples recursively.
+void SetGUIDToFuncNameMapImpl(std::queue<FunctionSamples *> &FSToUpdate,
+                              DenseMap<uint64_t, StringRef> *Map) {
+  while (!FSToUpdate.empty()) {
+    FunctionSamples *FS = FSToUpdate.front();
+    FSToUpdate.pop();
+    FS->GUIDToFuncNameMap = Map;
+    for (const auto &ICS : FS->getCallsiteSamples()) {
+      const FunctionSamplesMap &FSMap = ICS.second;
+      for (auto &IFS : FSMap) {
+        FunctionSamples &FS = const_cast<FunctionSamples &>(IFS.second);
+        FSToUpdate.push(&FS);
+      }
+    }
+  }
+}
+
 class GUIDToFuncNameMapper {
 public:
   GUIDToFuncNameMapper(Module &M, SampleProfileReader &Reader,
@@ -281,19 +299,7 @@
     for (auto &IFS : CurrentReader.getProfiles()) {
       FSToUpdate.push(&IFS.second);
     }
-
-    while (!FSToUpdate.empty()) {
-      FunctionSamples *FS = FSToUpdate.front();
-      FSToUpdate.pop();
-      FS->GUIDToFuncNameMap = Map;
-      for (const auto &ICS : FS->getCallsiteSamples()) {
-        const FunctionSamplesMap &FSMap = ICS.second;
-        for (auto &IFS : FSMap) {
-          FunctionSamples &FS = const_cast<FunctionSamples &>(IFS.second);
-          FSToUpdate.push(&FS);
-        }
-      }
-    }
+    SetGUIDToFuncNameMapImpl(FSToUpdate, Map);
   }
 
   SampleProfileReader &CurrentReader;
@@ -995,6 +1001,8 @@
         const FunctionSamples *FS = nullptr;
         if (auto *CB = dyn_cast<CallBase>(&I)) {
           if (!isa<IntrinsicInst>(I) && (FS = findCalleeFunctionSamples(*CB))) {
+            assert((!FunctionSamples::UseMD5 || FS->GUIDToFuncNameMap) &&
+                   "GUIDToFuncNameMap has to be populated");
             AllCandidates.push_back(CB);
             if (FS->getEntrySamples() > 0)
               localNotInlinedCallSites.try_emplace(CB, FS);
@@ -1114,6 +1122,11 @@
       // top-down processing of functions' annotation.
       FunctionSamples *OutlineFS = Reader->getOrCreateSamplesFor(*Callee);
       OutlineFS->merge(*FS);
+
+      // Recursively setup GUIDToFuncNameMap in OutlineFS.
+      std::queue<FunctionSamples *> FSToUpdate;
+      FSToUpdate.push(OutlineFS);
+      SetGUIDToFuncNameMapImpl(FSToUpdate, &GUIDToFuncNameMap);
     } else {
       auto pair =
           notInlinedCallInfo.try_emplace(Callee, NotInlinedProfileInfo{0});


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84994.282089.patch
Type: text/x-patch
Size: 4009 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200731/e7544a74/attachment.bin>