[llvm-profgen] Ignore branch count against outline function

[wlei]

Summary

For some transformations like hot-cold split or coro split, it can outline its part of function ranges. Since sample loader is the early stage of backend and no split happens at that time, compiler can't recognize those function, so in llvm-profgen we should attribute the sample to the original function. This is already done for the body range samples since we use the symbols from dwarf which is created before the split.

But for branch samples, the call from master function to its outlined function is actually not a call to the original function, we shouldn't add head/callsie samples for it. So instead of dwarf symbol, we use the symbols from symbol table and ignore those functions with special suffixes(like .cold ,.resume) for accumulating the callsite/head samples.

[hoy]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

403

nit: isOutlinedFunction

We should still count samples for it except for the head samples.

407

Check against specific suffix here so that we don't overlook cases we are not aware of?

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

403

Yeah, this is only called by populateBoundarySamples, so the body samples can be collected for those outlined functions.

407

So I guess you mean we can't assume here it covers all the outlined function case, right? I was thinking all the non-outlined cases are all handled by getCanonicalFnName.

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

407

If we have an outlined function here, say it's name is foo.cold, it's function body code is still count against foo because we use the name from debug table. so all the body samples are collected into foo 's FunctionSamples. This is the right way.

But for the head samples, here we used the name from symbol table, so if we count this, we will have a sample line like

foo.cold:0:100

means the total_samples is zero and head samples is not.

So I'm wondering if we don't do anything for populateBoundarySamples and we have a post-process to trim all the FunctionSamples that headsamples is non-zero but total_samples is zero.

With this, I guess we won't overlook any cases. what do you think?

[hoy]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

407

Actually I meant the change here is a bit too general. We are assuming names with a suffix other than .__uniq, are all for backend-outlined functions. I'm not quite sure that's the case so suggesting we just check against .resume here. For example, the front end could outline functions and when it comes to sample loader, those outlined functions should have their own profiles.

[wlei]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

407

That makes sense, thanks!

Then how about we just leave it unchanged like before(we can leave the test in this patch).

Currently we already use the name from symbol table, so we won't have the issue as our internal tool which used from debug table.

The only issue is we might have some sample like foo.cold:0:100, but it won't have impact.

If it's a pre-SampleLoader outlined function, compiler can consume it otherwise it just ignore it.

[hoy]

llvm/tools/llvm-profgen/ProfileGenerator.cpp

407

That sounds good too. The extra profile there will just be ignored by the sample loader.

[wlei]

Remove the related code to check outlined function,.

If it's an outlined function with suffixes, currently we only generate a zero total count and non-zero head count FunctionSample like "foo.cold:0:100".

If the outlined function is generated before the SampleLoader, the compiler will consume it, otherwise it will ignore it.

[wenlei]

For llvm-profgen side, If it gets an outlined function with suffixes, currently we only generate a zero total count and non-zero head count FunctionSample like "foo.cold:0:100".

This seems like an inconsistent state, and it would also confuse profile similarity check depending on whether we have split/outline or not.

For compiler side, If the outlined function is generated before the SampleLoader, the compiler will consume it, otherwise it will ignore it.

Where do we attribute the body samples for the funclet? If that's with the main body (instead of function), we won't be able to consume the funclet profile for splitting before sample loader.

Though I think most split/outline happens after sample loader, and it's cleaner if we attribute all counts to the original function, then left to the split/outline to decide how to split the counts.

[wlei]

In D110864#3040437, @wenlei wrote:

For llvm-profgen side, If it gets an outlined function with suffixes, currently we only generate a zero total count and non-zero head count FunctionSample like "foo.cold:0:100".

This seems like an inconsistent state, and it would also confuse profile similarity check depending on whether we have split/outline or not.

For compiler side, If the outlined function is generated before the SampleLoader, the compiler will consume it, otherwise it will ignore it.

Where do we attribute the body samples for the funclet? If that's with the main body (instead of function), we won't be able to consume the funclet profile for splitting before sample loader.

Though I think most split/outline happens after sample loader, and it's cleaner if we attribute all counts to the original function, then left to the split/outline to decide how to split the counts.

I see, yeah, see the code below, if I understand correctly, the pre-sample-loader funclet won't have profile matched(always missing profile). We use findFunctionSamples which check DILocation2SampleMap for the Inst's profile. However, for every function, the DILocation2SampleMap will be reset to empty. So for a funclet, it can't find the loc from it's master function's DILocation2SampleMap, it will just return null.

bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM) {
  DILocation2SampleMap.clear();
  ...
}

template <typename BT>
const FunctionSamples *SampleProfileLoaderBaseImpl<BT>::findFunctionSamples(
    const InstructionT &Inst) const {
  const DILocation *DIL = Inst.getDebugLoc();
  auto it = DILocation2SampleMap.try_emplace(DIL, nullptr);
  ...
  return it.first->second;
}

[wlei]

revert back to check outlined function

Checked for compiler side, it can't match any of the pre-sample-loader outline function, so we can just ignore all the functions with .* suffix.

[wlei]

Filter out hot-cold split and coro split function only.

[hoy]

lgtm, thanks.

[wenlei]

lgtm, thanks.

[wenlei]

Would be good to clarify in commit message that the body samples will always be attributed to the main body based on dwarf symbol name which is pre-split.

llvm/test/tools/llvm-profgen/Inputs/coroutine.perfscript

+           401d4a 0x401d4a/0x402b60/P/-/-/14  0x401d04/0x401d43/M/-/-/1  0x401ceb/0x401cf0/P/-/-/1  0x401f74/0x401ce3/P/-/-/1  0x401f6f/0x401f74/P/-/-/1  0x401aff/0x401f60/P/-/-/10  0x402df6/0x401a50/P/-/-/2  0x402c63/0x402de0/P/-/-/1  0x402c51/0x402c5c/P/-/-/1  0x402dba/0x402c4f/P/-/-/3  0x402c4a/0x402da0/P/-/-/2  0x401999/0x402c30/P/-/-/1  0x4019d5/0x401995/P/-/-/1  0x4019c2/0x4019c7/P/-/-/1  0x402cbc/0x4019bf/P/-/-/2  0x402e3e/0x402cb5/P/-/-/2  0x402cb0/0x402e20/P/-/-/2  0x4019ba/0x402ca0/P/-/-/1  0x4019ab/0x4019b6/P/-/-/3  0x4019a1/0x4019a6/P/-/-/1  0x402c95/0x40199e/P/-/-/5  0x402c79/0x402c88/P/-/-/3  0x402e1d/0x402c74/P/-/-/14  0x402c6f/0x402e00/P/-/-/1  0x402dfd/0x402c68/P/-/-/1  0x401f5f/0x402df8/P/-/-/1  0x401f4f/0x401f54/P/-/-/1  0x401d17/0x401f4f/P/-/-/1  0x401d0a/0x401d0f/P/-/-/3  0x401cde/0x401cf0/P/-/-/1  0x402b2d/0x401cd6/P/-/-/2  0x401cd1/0x402b20/P/-/-/9
+           401c7b 0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/15  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/15  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/13  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/13  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/10  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/11  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/9  0x401c34/0x401c3f/P/-/-/1  0x401c7b/0x401c2d/P/-/-/13  0x401c34/0x401c3f/P/-/-/1

llvm/test/tools/llvm-profgen/Inputs/func-split.perfscript

+            4004f0
+  5541f689495641d7
+ 0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/1  0x400633/0x400516/P/-/-/1  0x4004da/0x400631/P/-/-/2  0x40062c/0x4004c0/P/-/-/1  0x40050d/0x40062a/P/-/-/3  0x40051f/0x4004f0/P/-/-/5  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/3  0x40051f/0x4004f0/P/-/-/4  0x40051f/0x4004f0/P/-/-/5

llvm/test/tools/llvm-profgen/coroutine.test

+; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/coroutine.perfscript --binary=%S/Inputs/coroutine.perfbin --output=%t
+; RUN: FileCheck %s --input-file %t --check-prefix=CHECK
+
+; Check that the head sample count for ticker is 0.
+; CHECK: _Z6tickeri:353:0
+; CHECK-NOT: _Z6tickeri.resume
+
+
+/*
+ * Inputs/coroutine.perfbin is generated by compiling the following source code:
+ * clang++ coroutine.cpp -std=c++2a -g2 -o coroutine
+ */
+
+#include <cstdint>
+#include <cstdlib>
+#include <ctime>
+#include <experimental/coroutine>
+#include <iostream>
+
+struct task {
+  struct promise_type {
+    task get_return_object() { return {}; }
+    std::experimental::suspend_never initial_suspend() { return {}; }
+    std::experimental::suspend_never final_suspend() noexcept { return {}; }
+    void return_void() {}
+    void unhandled_exception() {}
+  };
+};
+
+template <typename T>
+struct generator {
+  struct promise_type;
+  using handle = std::experimental::coroutine_handle<promise_type>;
+  struct promise_type {
+    int current_value;
+    static auto get_return_object_on_allocation_failure() { return generator{nullptr}; }
+    auto get_return_object() { return generator{handle::from_promise(*this)}; }
+    auto initial_suspend() { return std::experimental::suspend_always{}; }
+    auto final_suspend() { return std::experimental::suspend_always{}; }
+    void unhandled_exception() { std::terminate(); }
+    void return_void() {}
+    auto yield_value(int value) {
+      current_value = value;
+      return std::experimental::suspend_always{};
+    }
+  };
+  bool move_next() { return coro ? (coro.resume(), !coro.done()) : false; }
+  int current_value() { return coro.promise().current_value; }
+  generator(generator const &) = delete;
+  generator(generator &&rhs) : coro(rhs.coro) { rhs.coro = nullptr; }
+  ~generator() {
+    if (coro)
+      coro.destroy();
+  }
+
+private:
+  generator(handle h) : coro(h) {}
+  handle coro;
+};
+
+generator<int> ticker(int count) {
+  for (int i = 0; i < count; ++i) {
+    srand(time(NULL));
+    uint32_t a = rand() % 10 + 1;
+    uint32_t b = rand() % 10 + 1;
+    uint64_t c = 0;
+    for (int i = 0; i < 1500; ++i) {
+      c = ((uint64_t)a) + b;
+      a = b;
+      b = c % 2147483648ULL;
+    }
+    co_yield a;
+  }
+}
+
+int main() {
+  auto g = ticker(500000);
+  uint64_t ans = 0;
+  while (g.move_next()) {
+    ans += g.current_value();
+  }
+  std::cout << ans << "\n";
+}

llvm/test/tools/llvm-profgen/func-split.test

+; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/func-split.perfscript --binary=%S/Inputs/func-split.perfbin --output=%t
+; RUN: FileCheck %s --input-file %t --check-prefix=CHECK
+; RUN: llvm-profgen --format=text --perfscript=%S/Inputs/func-split.perfscript --binary=%S/Inputs/func-split.perfbin --output=%t --ignore-stack-samples
+; RUN: FileCheck %s --input-file %t --check-prefix=CHECK-STRIP-CTX
+
+;CHECK: [foo]:408:0
+;CHECK:  2.1: 27
+;CHECK:  3: 27
+;CHECK:  3.1: 2 bar:2
+;CHECK:  3.2: 26
+;CHECK: [foo:3.1 @ bar]:8:0
+;CHECK:  1: 1
+;CHECK:  5: 1
+;CHECK: [bar]:0:1
+
+;CHECK-NOT: foo.cold
+
+;CHECK-STRIP-CTX: foo:408:0
+;CHECK-STRIP-CTX:  0: 0
+;CHECK-STRIP-CTX:  2.1: 27
+;CHECK-STRIP-CTX:  3: 27
+;CHECK-STRIP-CTX:  3.1: 1 bar:1
+;CHECK-STRIP-CTX:  3.2: 26
+;CHECK-STRIP-CTX:  4: 0
+;CHECK-STRIP-CTX: bar:8:1
+;CHECK-STRIP-CTX:  1: 1
+;CHECK-STRIP-CTX:  5: 1
+
+;CHECK-STRIP-CTX-NOT: foo.cold
+
+
+; clang -g -O3 -fdebug-info-for-profiling func-split.c -mllvm -mfs-count-threshold=0
+; -fprofile-sample-use=profile.txt -fno-inline -mllvm --enable-split-machine-functions=1
+
+#include <stdio.h>
+
+int bar(int x, int y) {
+  if (x % 3) {
+    return x - y;
+  }
+  return x + y;
+}
+
+void foo() {
+  int s, i = 0;
+  while (i++ < 4000 * 4000)
+    if (i % 91 == 0) s = bar(i, s); else s += 30;
+  printf("sum is %d\n", s);
+}
+
+int main() {
+  foo();
+  return 0;
+}
+
+; profile.txt:
+
+foo:106269:0
+ 2.1: 2268
+ 2.2: 2217
+ 3: 2268
+ 3.1: 1 bar:1
+ 3.2: 2192
+bar:1032:1
+ 0: 24
+ 1: 24
+ 2: 16
+ 4: 8
+ 5: 24

llvm/tools/llvm-profgen/ProfileGenerator.h

protected:
   */
   void findDisjointRanges(RangeSample &DisjointRanges,
                           const RangeSample &Ranges);
   // Helper function for updating body sample for a leaf location in
   // FunctionProfile
   void updateBodySamplesforFunctionProfile(FunctionSamples &FunctionProfile,
                                            const SampleContextFrame &LeafLoc,
                                            uint64_t Count);
-
+  StringRef getCalleeNameForOffset(uint64_t TargetOffset);
   // Used by SampleProfileWriter
   SampleProfileMap ProfileMap;
 
   ProfiledBinary *Binary = nullptr;
 
   const ContextSampleCounterMap &SampleCounters;
 };
 

llvm/tools/llvm-profgen/ProfileGenerator.cpp

while (IP.Address <= RangeEnd) {
                                             DC);
       }
       // Move to next IP within the range.
       IP.advance();
     }
   }
 }
 
+static bool isOutlinedFunction(StringRef CalleeName) {
+  // Check whether it's from hot-cold func split or coro split.
+  return CalleeName.find(".resume") != StringRef::npos ||
+         CalleeName.find(".cold") != StringRef::npos;
+}
+
+StringRef ProfileGeneratorBase::getCalleeNameForOffset(uint64_t TargetOffset) {
+  // Get the callee name by branch target if it's a call branch.
+  StringRef CalleeName = FunctionSamples::getCanonicalFnName(
+      Binary->getFuncFromStartOffset(TargetOffset));
+
+  // We won't accumulate sample count againt outlined function.
+  if (CalleeName.size() == 0 || isOutlinedFunction(CalleeName))
+    return StringRef();
+
+  return CalleeName;
+}
+
 void ProfileGenerator::populateBoundarySamplesForAllFunctions(
     const BranchSample &BranchCounters) {
   for (auto Entry : BranchCounters) {
     uint64_t SourceOffset = Entry.first.first;
     uint64_t TargetOffset = Entry.first.second;
     uint64_t Count = Entry.second;
     assert(Count != 0 && "Unexpected zero weight branch");
 
-    // Get the callee name by branch target if it's a call branch.
-    StringRef CalleeName = FunctionSamples::getCanonicalFnName(
-        Binary->getFuncFromStartOffset(TargetOffset));
+    StringRef CalleeName = getCalleeNameForOffset(TargetOffset);
     if (CalleeName.size() == 0)
       continue;
     // Record called target sample and its count.
     const SampleContextFrameVector &FrameVec =
         Binary->getFrameLocationStack(SourceOffset);
     if (!FrameVec.empty()) {
       Count *= getDuplicationFactor(FrameVec.back().Location.Discriminator);
       FunctionSamples &FunctionProfile =
void CSProfileGenerator::populateBoundarySamplesForFunction(
     const BranchSample &BranchCounters) {
 
   for (auto Entry : BranchCounters) {
     uint64_t SourceOffset = Entry.first.first;
     uint64_t TargetOffset = Entry.first.second;
     uint64_t Count = Entry.second;
     assert(Count != 0 && "Unexpected zero weight branch");
 
-    // Get the callee name by branch target if it's a call branch
-    StringRef CalleeName = FunctionSamples::getCanonicalFnName(
-        Binary->getFuncFromStartOffset(TargetOffset));
+    StringRef CalleeName = getCalleeNameForOffset(TargetOffset);
     if (CalleeName.size() == 0)
       continue;
 
     // Record called target sample and its count
     auto LeafLoc = Binary->getInlineLeafFrameLoc(SourceOffset);
     if (!LeafLoc.hasValue())
       continue;
     Count *= getDuplicationFactor(LeafLoc->Location.Discriminator);
for (auto BI : BranchCounter) {
     const MCDecodedPseudoProbe *CallProbe =
         Binary->getCallProbeForAddr(SourceAddress);
     if (CallProbe == nullptr)
       continue;
     FunctionSamples &FunctionProfile =
         getFunctionProfileForLeafProbe(ContextStack, CallProbe);
     FunctionProfile.addBodySamples(CallProbe->getIndex(), 0, Count);
     FunctionProfile.addTotalSamples(Count);
-    StringRef CalleeName = FunctionSamples::getCanonicalFnName(
-        Binary->getFuncFromStartOffset(TargetOffset));
+    StringRef CalleeName = getCalleeNameForOffset(TargetOffset);
     if (CalleeName.size() == 0)
       continue;
     FunctionProfile.addCalledTargetSamples(CallProbe->getIndex(), 0, CalleeName,
                                            Count);
   }
 }
 
 FunctionSamples &CSProfileGenerator::getFunctionProfileForLeafProbe(