[AutoFDO] Make call targets order deterministic for sample profile

Summary:
StringMap is used for storing call target to frequency map for AutoFDO. However the iterating order of StringMap is non-deterministic, which leads to non-determinism in AutoFDO profile output. Now new API getSortedCallTargets and SortCallTargets are added for deterministic ordering and output.

Roundtrip test for text profile and binary profile is added.

Reviewers: wmi, davidxl, danielcdh

Subscribers: hiraditya, mgrang, llvm-commits, twoh

Tags: #llvm

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

llvm-svn: 369440

llvm/include/llvm/ProfileData/SampleProf.h

@@ -27,6 +27,7 @@
 #include <algorithm>
 #include <cstdint>
 #include <map>
+#include <set>
 #include <string>
 #include <system_error>
 #include <utility>
@@ -143,8 +144,18 @@ raw_ostream &operator<<(raw_ostream &OS, const LineLocation &Loc);
 /// will be a list of one or more functions.
 class SampleRecord {
 public:
-  using CallTargetMap = StringMap<uint64_t>;
+  using CallTarget = std::pair<StringRef, uint64_t>;
+  struct CallTargetComparator {
+    bool operator() (const CallTarget &LHS, const CallTarget &RHS) {
+      if (LHS.second != RHS.second)
+        return LHS.second > RHS.second;
 
+      return LHS.first < RHS.first;
+    }
+  };
+
+  using SortedCallTargetSet = std::set<CallTarget, CallTargetComparator>;
+  using CallTargetMap = StringMap<uint64_t>;
   SampleRecord() = default;
 
   /// Increment the number of samples for this record by \p S.
@@ -179,6 +190,18 @@ public:
 
   uint64_t getSamples() const { return NumSamples; }
   const CallTargetMap &getCallTargets() const { return CallTargets; }
+  const SortedCallTargetSet getSortedCallTargets() const {
+    return SortCallTargets(CallTargets);
+  }
+
+  /// Sort call targets in descending order of call frequency.
+  static const SortedCallTargetSet SortCallTargets(const CallTargetMap &Targets) {
+    SortedCallTargetSet SortedTargets;
+    for (const auto &I : Targets) {
+      SortedTargets.emplace(I.first(), I.second);
+    }
+    return SortedTargets;
+  }
 
   /// Merge the samples in \p Other into this record.
   /// Optionally scale sample counts by \p Weight.

llvm/lib/ProfileData/SampleProf.cpp

@@ -100,8 +100,8 @@ void SampleRecord::print(raw_ostream &OS, unsigned Indent) const {
   OS << NumSamples;
   if (hasCalls()) {
     OS << ", calls:";
-    for (const auto &I : getCallTargets())
-      OS << " " << I.first() << ":" << I.second;
+    for (const auto &I : getSortedCallTargets())
+      OS << " " << I.first << ":" << I.second;
   }
   OS << "\n";
 }

llvm/lib/ProfileData/SampleProfWriter.cpp

@@ -100,8 +100,8 @@ std::error_code SampleProfileWriterText::write(const FunctionSamples &S) {
 
     OS << Sample.getSamples();
 
-    for (const auto &J : Sample.getCallTargets())
-      OS << " " << J.first() << ":" << J.second;
+    for (const auto &J : Sample.getSortedCallTargets())
+      OS << " " << J.first << ":" << J.second;
     OS << "\n";
   }
 
@@ -294,8 +294,8 @@ std::error_code SampleProfileWriterBinary::writeBody(const FunctionSamples &S) {
     encodeULEB128(Loc.Discriminator, OS);
     encodeULEB128(Sample.getSamples(), OS);
     encodeULEB128(Sample.getCallTargets().size(), OS);
-    for (const auto &J : Sample.getCallTargets()) {
-      StringRef Callee = J.first();
+    for (const auto &J : Sample.getSortedCallTargets()) {
+      StringRef Callee = J.first;
       uint64_t CalleeSamples = J.second;
       if (std::error_code EC = writeNameIdx(Callee))
         return EC;

llvm/lib/Transforms/IPO/SampleProfile.cpp

@@ -192,7 +192,7 @@ class GUIDToFuncNameMapper {
 public:
   GUIDToFuncNameMapper(Module &M, SampleProfileReader &Reader,
                         DenseMap<uint64_t, StringRef> &GUIDToFuncNameMap)
-      : CurrentReader(Reader), CurrentModule(M), 
+      : CurrentReader(Reader), CurrentModule(M),
       CurrentGUIDToFuncNameMap(GUIDToFuncNameMap) {
     if (CurrentReader.getFormat() != SPF_Compact_Binary)
       return;
@@ -1292,17 +1292,12 @@ void SampleProfileLoader::buildEdges(Function &F) {
 }
 
 /// Returns the sorted CallTargetMap \p M by count in descending order.
-static SmallVector<InstrProfValueData, 2> SortCallTargets(
-    const SampleRecord::CallTargetMap &M) {
+static SmallVector<InstrProfValueData, 2> GetSortedValueDataFromCallTargets(
+    const SampleRecord::CallTargetMap & M) {
   SmallVector<InstrProfValueData, 2> R;
-  for (auto I = M.begin(); I != M.end(); ++I)
-    R.push_back({FunctionSamples::getGUID(I->getKey()), I->getValue()});
-  llvm::sort(R, [](const InstrProfValueData &L, const InstrProfValueData &R) {
-    if (L.Count == R.Count)
-      return L.Value > R.Value;
-    else
-      return L.Count > R.Count;
-  });
+  for (const auto &I : SampleRecord::SortCallTargets(M)) {
+    R.emplace_back(InstrProfValueData{FunctionSamples::getGUID(I.first), I.second});
+  }
   return R;
 }
 
@@ -1397,7 +1392,7 @@ void SampleProfileLoader::propagateWeights(Function &F) {
           if (!T || T.get().empty())
             continue;
           SmallVector<InstrProfValueData, 2> SortedCallTargets =
-              SortCallTargets(T.get());
+              GetSortedValueDataFromCallTargets(T.get());
           uint64_t Sum;
           findIndirectCallFunctionSamples(I, Sum);
           annotateValueSite(*I.getParent()->getParent()->getParent(), I,
@@ -1724,7 +1719,7 @@ bool SampleProfileLoaderLegacyPass::runOnModule(Module &M) {
 }
 
 bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM) {
-  
+
   DILocation2SampleMap.clear();
   // By default the entry count is initialized to -1, which will be treated
   // conservatively by getEntryCount as the same as unknown (None). This is

llvm/test/tools/llvm-profdata/Inputs/sample-profile.proftext

@@ -1,7 +1,3 @@
-_Z3bari:20301:1437
- 1: 1437
-_Z3fooi:7711:610
- 1: 610
 main:184019:0
  4: 534
  4.2: 534
@@ -14,3 +10,7 @@ main:184019:0
   1: 1000
  10: inline2:2000
   1: 2000
+_Z3bari:20301:1437
+ 1: 1437
+_Z3fooi:7711:610
+ 1: 610

llvm/test/tools/llvm-profdata/roundtrip.test

@@ -4,3 +4,6 @@ RUN: diff %t.0.proftext %S/Inputs/IR_profile.proftext
 RUN: llvm-profdata merge -o %t.1.profdata %t.0.proftext
 RUN: llvm-profdata show -o %t.1.proftext -all-functions -text %t.1.profdata 
 RUN: diff %t.1.proftext %S/Inputs/IR_profile.proftext
+RUN: llvm-profdata merge --sample --binary -output=%t.2.profdata %S/Inputs/sample-profile.proftext
+RUN: llvm-profdata merge --sample --text -output=%t.2.proftext %t.2.profdata
+RUN: diff %t.2.proftext %S/Inputs/sample-profile.proftext

llvm/test/tools/llvm-profdata/sample-profile-basic.test

@@ -3,7 +3,7 @@ Basic tests for sample profiles.
 1- Show all functions
 RUN: llvm-profdata show --sample %p/Inputs/sample-profile.proftext | FileCheck %s --check-prefix=SHOW1
 SHOW1-DAG: Function: main: 184019, 0, 7 sampled lines
-SHOW1-DAG: 9: 2064, calls: _Z3fooi:631 _Z3bari:1471
+SHOW1-DAG: 9: 2064, calls: _Z3bari:1471 _Z3fooi:631
 SHOW1-DAG: Function: _Z3fooi: 7711, 610, 1 sampled lines
 SHOW1-DAG: Function: _Z3bari: 20301, 1437, 1 sampled lines
 SHOW1-DAG: 1: 1437
@@ -26,7 +26,7 @@ RUN: diff %t-binary %t-text
 RUN: llvm-profdata merge --sample %p/Inputs/sample-profile.proftext -o %t-binprof
 RUN: llvm-profdata merge --sample --text %p/Inputs/sample-profile.proftext %t-binprof -o - | FileCheck %s --check-prefix=MERGE1
 MERGE1: main:368038:0
-MERGE1: 9: 4128 _Z3fooi:1262 _Z3bari:2942
+MERGE1: 9: 4128 _Z3bari:2942 _Z3fooi:1262
 MERGE1: _Z3bari:40602:2874
 MERGE1: _Z3fooi:15422:1220