[AutoFDO]Merge called target in body samples

[mingmingl]

Summary

Body samples could have count and call targets, merge them as well.

[wenlei]

cc @wlei

[wlei]

Good catch, thank you! For the test case, perhaps we can reuse the test https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-profdata/Inputs/sample-flatten-profile.proftext#L6 and add some dummy call target to the inlinees. like

   3: bar:10
    1: 10
+   2: 10 baz:10

[mingmingl]

add test case (and minor change to use 'try_emplace' that does nothing if key presents)

[mingmingl]

In D147297#4235771, @wlei wrote:

Good catch, thank you! For the test case, perhaps we can reuse the test https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-profdata/Inputs/sample-flatten-profile.proftext#L6 and add some dummy call target to the inlinees. like

   3: bar:10
    1: 10
+   2: 10 baz:10

thanks for confirming! Done.

[wlei]

llvm/include/llvm/ProfileData/SampleProf.h

1359–1360

Nit: Looks like there is an existing function https://github.com/llvm/llvm-project/blob/main/llvm/lib/ProfileData/SampleProf.cpp#L119 doing the similar thing, but I guess that requires some changes of it, I'm good for this version, note here just in case people like to refactor it in the future:)

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

4

Thanks for adding the test.
Did you check if test can pass without this patch? Here the baz is the top-level profile, it's possible that it's just copied from the input profile ( see the if (Ret.second) true branch). Or I'm thinking if we can add another line to the baz in line 28 so that it works for the either merging order.

[davidxl]

llvm/include/llvm/ProfileData/SampleProf.h

1359–1360

Perhaps add a TODO comment.

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

4

Either this or add another small test case on merging of VP profile.

[mingmingl]

added tests and did a small refactor.

llvm/include/llvm/ProfileData/SampleProf.h

1359–1360

Thanks for the pointer. Did a little refactor in the new code path (may slightly prefer a separate refactor if the effort is larger or may affect more users)

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

4

Did you check if test can pass without this patch?

I see your point. The test would fail without this patch. Now with test edited this is cleaerer.

Either this or add another small test case on merging of VP profile.

Good point. Edit this test to show profiles with the same line location are merged, and those with different line location are not.

[wlei]

LGTM, thanks for the fix!

llvm/include/llvm/ProfileData/SampleProf.h

1359–1360

Good idea to leverage the addSampleRecord, thanks!

[wlei]

llvm/include/llvm/ProfileData/SampleProf.h

781

Nit: for completeness, add the uint64_t Weight = 1 like other addXXX

[mingmingl]

Add 'Weight' for default parameter consistency.

llvm/include/llvm/ProfileData/SampleProf.h

public:
   sampleprof_error addCalledTargetSamples(uint32_t LineOffset,
                                           uint32_t Discriminator,
                                           StringRef FName, uint64_t Num,
                                           uint64_t Weight = 1) {
     return BodySamples[LineLocation(LineOffset, Discriminator)].addCalledTarget(
         FName, Num, Weight);
   }
 
+  sampleprof_error addSampleRecord(LineLocation Location,
+                                   const SampleRecord &SampleRecord, uint64_t Weight = 1) {
+    return BodySamples[Location].merge(SampleRecord, Weight);
+  }
+
   // Remove a call target and decrease the body sample correspondingly. Return
   // the number of body samples actually decreased.
   uint64_t removeCalledTargetAndBodySample(uint32_t LineOffset,
                                            uint32_t Discriminator,
                                            StringRef FName) {
     uint64_t Count = 0;
     auto I = BodySamples.find(LineLocation(LineOffset, Discriminator));
     if (I != BodySamples.end()) {
public:
   }
 
 private:
   static void flattenNestedProfile(SampleProfileMap &OutputProfiles,
                                    const FunctionSamples &FS) {
     // To retain the context, checksum, attributes of the original profile, make
     // a copy of it if no profile is found.
     SampleContext &Context = FS.getContext();
-    auto Ret = OutputProfiles.emplace(Context, FS);
+    auto Ret = OutputProfiles.try_emplace(Context, FS);
     FunctionSamples &Profile = Ret.first->second;
     if (Ret.second) {
       // When it's the copy of the old profile, just clear all the inlinees'
       // samples.
       Profile.getCallsiteSamples().clear();
       // We recompute TotalSamples later, so here set to zero.
       Profile.setTotalSamples(0);
     } else {
-      for (const auto &Line : FS.getBodySamples()) {
-        Profile.addBodySamples(Line.first.LineOffset, Line.first.Discriminator,
-                               Line.second.getSamples());
+      for (const auto &[LineLocation, SampleRecord] : FS.getBodySamples()) {
+        Profile.addSampleRecord(LineLocation, SampleRecord);
       }
     }
 
     assert(Profile.getCallsiteSamples().empty() &&
            "There should be no inlinees' profiles after flattening.");
 
     // TotalSamples might not be equal to the sum of all samples from
     // BodySamples and CallsiteSamples. So here we use "TotalSamples =

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

 baz:160:10
  1: 10
  3: 20
+ 4: 21 qux:5 quux:6 corge:10
+ 4.1: 12 quux:3 grault:4 thud:5
  5: foo:30
   1: 20
   3: bar:10
    1: 10
    !CFGChecksum: 4
    !Attributes: 4
   !CFGChecksum: 3
   !Attributes: 3
  !CFGChecksum: 1
  !Attributes: 1
 main:110:1
  4: 1
  4.2: 1
  7: 1
  9: 3 bar:2 foo:1
  10: foo:2
   4: 1
   3: bar:1
    1: 1
    !CFGChecksum: 4
    !Attributes: 4
   !CFGChecksum: 3
   !Attributes: 3
  10: baz:20
+  4: 15 qux:3 quux:7 corge:5
   10: 1
   6: bar:3
    1: 2
    7: 1
    !CFGChecksum: 4
    !Attributes: 4
   !CFGChecksum: 1
   !Attributes: 1

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

 ; RUN: llvm-profdata merge --sample --convert-sample-profile-layout=flat --text %S/Inputs/sample-flatten-profile.proftext -o - | FileCheck %s --match-full-lines --strict-whitespace
 ; RUN: llvm-profdata merge --sample --extbinary %S/Inputs/sample-flatten-profile.proftext -o %t2 && llvm-profdata merge --sample --convert-sample-profile-layout=flat --text %t2 -o - | FileCheck %s --match-full-lines --strict-whitespace
 
 ; RUN: llvm-profdata merge --sample --convert-sample-profile-layout=flat --text %S/Inputs/sample-flatten-profile-cs.proftext -o - | FileCheck %s --match-full-lines --strict-whitespace --check-prefix=CHECK-CS
 ; RUN: llvm-profdata merge --sample --extbinary %S/Inputs/sample-flatten-profile-cs.proftext -o %t2 && llvm-profdata merge --sample --convert-sample-profile-layout=flat --text %t2 -o - | FileCheck %s --match-full-lines --strict-whitespace --check-prefix=CHECK-CS
 
 ; CHECK:baz:169:10
 ; CHECK-NEXT: 1: 10
 ; CHECK-NEXT: 3: 20
+; CHECK-NEXT: 4: 36 corge:15 quux:13 qux:8
+; CHECK-NEXT: 4.1: 12 thud:5 grault:4 quux:3
 ; CHECK-NEXT: 5: 20 foo:20
 ; CHECK-NEXT: 6: 2 bar:2
 ; CHECK-NEXT: 10: 1
 ; CHECK-NEXT: !CFGChecksum: 1
 ; CHECK-NEXT: !Attributes: 1
 ; CHECK-NEXT:foo:134:21
 ; CHECK-NEXT: 1: 21
 ; CHECK-NEXT: 3: 12 bar:11
 ; CHECK-NEXT: 4: 1
 ; CHECK-NEXT: !CFGChecksum: 3
 ; CHECK-NEXT: !Attributes: 3
-; CHECK-NEXT:main:91:1
+; CHECK-NEXT:main:104:1
 ; CHECK-NEXT: 4: 1
 ; CHECK-NEXT: 4.2: 1
 ; CHECK-NEXT: 7: 1
 ; CHECK-NEXT: 9: 3 bar:2 foo:1
-; CHECK-NEXT: 10: 3 baz:2 foo:1
+; CHECK-NEXT: 10: 16 baz:15 foo:1
 ; CHECK-NEXT: !CFGChecksum: 2
 ; CHECK-NEXT: !Attributes: 2
 ; CHECK-NEXT:bar:15:14
 ; CHECK-NEXT: 1: 14
 ; CHECK-NEXT: 7: 1
 ; CHECK-NEXT: !CFGChecksum: 4
 ; CHECK-NEXT: !Attributes: 4