diff options
author | wlei <wlei@fb.com> | 2021-02-10 10:04:39 -0800 |
---|---|---|
committer | Tom Stellard <tstellar@redhat.com> | 2021-02-19 21:21:13 -0800 |
commit | 610b51c04d3ca6b58555fa30ae52ad9762f9cf86 (patch) | |
tree | 1fc4a153746ad684c8f13fd1e9544a7e8b3b8b24 | |
parent | 66873fb695370f5bd333e327ec77e4710c7891c2 (diff) | |
download | llvm-610b51c04d3ca6b58555fa30ae52ad9762f9cf86.tar.gz |
[CSSPGO][llvm-profgen] Filter out the instructions without location info for symbolizer
It appears some instructions doesn't have the debug location info and the symbolizer will return an empty call stack for them which will cause some crash later in profile unwinding. Actually we do not record the sample info for them, so this change just filter out those instruction.
As those instruction would appears at the begin and end of the instruction list, without them we need to add the boundary check for IP `advance` and `backward`.
Also for pseudo probe based profile, we actually don't need the symbolized location info, so here just change to use an empty stack for it. This could save half of the binary loading time.
Differential Revision: https://reviews.llvm.org/D96434
-rw-r--r-- | llvm/test/tools/llvm-profgen/inline-cs-noprobe.test | 4 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profgen/noinline-cs-noprobe.test | 8 | ||||
-rw-r--r-- | llvm/test/tools/llvm-profgen/recursion-compression-noprobe.test | 30 | ||||
-rw-r--r-- | llvm/tools/llvm-profgen/PerfReader.cpp | 4 | ||||
-rw-r--r-- | llvm/tools/llvm-profgen/ProfileGenerator.cpp | 21 | ||||
-rw-r--r-- | llvm/tools/llvm-profgen/ProfiledBinary.cpp | 18 | ||||
-rw-r--r-- | llvm/tools/llvm-profgen/ProfiledBinary.h | 9 |
7 files changed, 58 insertions, 36 deletions
diff --git a/llvm/test/tools/llvm-profgen/inline-cs-noprobe.test b/llvm/test/tools/llvm-profgen/inline-cs-noprobe.test index d8cc1932f877..cb562e347a3e 100644 --- a/llvm/test/tools/llvm-profgen/inline-cs-noprobe.test +++ b/llvm/test/tools/llvm-profgen/inline-cs-noprobe.test @@ -1,12 +1,12 @@ ; RUN: llvm-profgen --perfscript=%S/Inputs/inline-cs-noprobe.perfscript --binary=%S/Inputs/inline-cs-noprobe.perfbin --output=%t --show-unwinder-output --csprof-cold-thres=0 | FileCheck %s --check-prefix=CHECK-UNWINDER ; RUN: FileCheck %s --input-file %t -; CHECK:[main:1 @ foo]:44:0 +; CHECK:[main:1 @ foo]:309:0 ; CHECK: 2.1: 14 ; CHECK: 3: 15 ; CHECK: 3.1: 14 bar:14 ; CHECK: 3.2: 1 -; CHECK:[main:1 @ foo:3.1 @ bar]:14:0 +; CHECK:[main:1 @ foo:3.1 @ bar]:84:0 ; CHECK: 1: 14 ; CHECK-UNWINDER: Binary(inline-cs-noprobe.perfbin)'s Range Counter: diff --git a/llvm/test/tools/llvm-profgen/noinline-cs-noprobe.test b/llvm/test/tools/llvm-profgen/noinline-cs-noprobe.test index 9d5c787e7f92..c5e6dc1111ca 100644 --- a/llvm/test/tools/llvm-profgen/noinline-cs-noprobe.test +++ b/llvm/test/tools/llvm-profgen/noinline-cs-noprobe.test @@ -1,15 +1,15 @@ ; RUN: llvm-profgen --perfscript=%S/Inputs/noinline-cs-noprobe.perfscript --binary=%S/Inputs/noinline-cs-noprobe.perfbin --output=%t --show-unwinder-output --csprof-cold-thres=0 | FileCheck %s --check-prefix=CHECK-UNWINDER ; RUN: FileCheck %s --input-file %t -; CHECK:[main:1 @ foo:3 @ bar]:12:3 +; CHECK:[main:1 @ foo]:54:0 +; CHECK: 2: 3 +; CHECK: 3: 3 bar:3 +; CHECK:[main:1 @ foo:3 @ bar]:50:3 ; CHECK: 0: 3 ; CHECK: 1: 3 ; CHECK: 2: 2 ; CHECK: 4: 1 ; CHECK: 5: 3 -; CHECK:[main:1 @ foo]:6:0 -; CHECK: 2: 3 -; CHECK: 3: 3 bar:3 ; CHECK-UNWINDER: Binary(noinline-cs-noprobe.perfbin)'s Range Counter: ; CHECK-UNWINDER: main:1 @ foo diff --git a/llvm/test/tools/llvm-profgen/recursion-compression-noprobe.test b/llvm/test/tools/llvm-profgen/recursion-compression-noprobe.test index 03bab8407435..15bdd870879e 100644 --- a/llvm/test/tools/llvm-profgen/recursion-compression-noprobe.test +++ b/llvm/test/tools/llvm-profgen/recursion-compression-noprobe.test @@ -4,38 +4,38 @@ ; RUN: llvm-profgen --perfscript=%S/Inputs/recursion-compression-noprobe.perfscript --binary=%S/Inputs/recursion-compression-noprobe.perfbin --output=%t --csprof-cold-thres=0 ; RUN: FileCheck %s --input-file %t -; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa]:14:0 -; CHECK-UNCOMPRESS: 1: 1 -; CHECK-UNCOMPRESS: 2: 13 fb:11 -; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb]:12:0 +; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb]:48:0 ; CHECK-UNCOMPRESS: 1: 11 ; CHECK-UNCOMPRESS: 2: 1 fa:1 -; CHECK-UNCOMPRESS:[main:1 @ foo]:3:0 +; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa]:24:0 +; CHECK-UNCOMPRESS: 1: 1 +; CHECK-UNCOMPRESS: 2: 13 fb:11 +; CHECK-UNCOMPRESS:[main:1 @ foo]:7:0 ; CHECK-UNCOMPRESS: 2: 1 ; CHECK-UNCOMPRESS: 3: 2 fa:1 -; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa]:3:0 +; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa]:7:0 ; CHECK-UNCOMPRESS: 1: 1 ; CHECK-UNCOMPRESS: 2: 2 fb:1 -; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb]:1:0 +; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb]:2:0 ; CHECK-UNCOMPRESS: 2: 1 fa:1 -; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb:2 @ fa]:1:0 +; CHECK-UNCOMPRESS:[main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb:2 @ fa]:2:0 ; CHECK-UNCOMPRESS: 4: 1 -; CHECK: [main:1 @ foo:3 @ fa]:14:0 -; CHECK: 1: 1 -; CHECK: 2: 13 fb:11 -; CHECK: [main:1 @ foo:3 @ fa:2 @ fb]:12:0 +; CHECK: [main:1 @ foo:3 @ fa:2 @ fb]:48:0 ; CHECK: 1: 11 ; CHECK: 2: 1 fa:1 -; CHECK: [main:1 @ foo:3 @ fa:2 @ fb:2 @ fa]:4:0 +; CHECK: [main:1 @ foo:3 @ fa]:24:0 +; CHECK: 1: 1 +; CHECK: 2: 13 fb:11 +; CHECK: [main:1 @ foo:3 @ fa:2 @ fb:2 @ fa]:9:0 ; CHECK: 1: 1 ; CHECK: 2: 2 fb:1 ; CHECK: 4: 1 -; CHECK: [main:1 @ foo]:3:0 +; CHECK: [main:1 @ foo]:7:0 ; CHECK: 2: 1 ; CHECK: 3: 2 fa:1 -; CHECK: [main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb]:0:0 +; CHECK: [main:1 @ foo:3 @ fa:2 @ fb:2 @ fa:2 @ fb]:1:0 ; original code: diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp index 2e0b71f38e6d..1f842008db42 100644 --- a/llvm/tools/llvm-profgen/PerfReader.cpp +++ b/llvm/tools/llvm-profgen/PerfReader.cpp @@ -93,6 +93,8 @@ std::shared_ptr<StringBasedCtxKey> FrameStack::getContextKey() { std::shared_ptr<StringBasedCtxKey> KeyStr = std::make_shared<StringBasedCtxKey>(); KeyStr->Context = Binary->getExpandedContextStr(Stack); + if (KeyStr->Context.empty()) + return nullptr; KeyStr->genHashCode(); return KeyStr; } @@ -116,6 +118,8 @@ void VirtualUnwinder::collectSamplesFromFrame(UnwindState::ProfiledFrame *Cur, return; std::shared_ptr<ContextKey> Key = Stack.getContextKey(); + if (Key == nullptr) + return; auto Ret = CtxCounterMap->emplace(Hashable<ContextKey>(Key), SampleCounter()); SampleCounter &SCounter = Ret.first->second; for (auto &Item : Cur->RangeSamples) { diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp index 553ea71ea1fc..4cfadffebb18 100644 --- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp +++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp @@ -212,7 +212,6 @@ void CSProfileGenerator::updateBodySamplesforFunctionProfile( FunctionProfile.addBodySamples(LeafLoc.second.LineOffset, LeafLoc.second.Discriminator, Count - PreviousCount); - FunctionProfile.addTotalSamples(Count - PreviousCount); } } @@ -242,9 +241,13 @@ void CSProfileGenerator::populateFunctionBodySamples( while (IP.Address <= RangeEnd) { uint64_t Offset = Binary->virtualAddrToOffset(IP.Address); - const FrameLocation &LeafLoc = Binary->getInlineLeafFrameLoc(Offset); - // Recording body sample for this specific context - updateBodySamplesforFunctionProfile(FunctionProfile, LeafLoc, Count); + auto LeafLoc = Binary->getInlineLeafFrameLoc(Offset); + if (LeafLoc.hasValue()) { + // Recording body sample for this specific context + updateBodySamplesforFunctionProfile(FunctionProfile, *LeafLoc, Count); + } + // Accumulate total sample count even it's a line with invalid debug info + FunctionProfile.addTotalSamples(Count); // Move to next IP within the range IP.advance(); } @@ -266,9 +269,11 @@ void CSProfileGenerator::populateFunctionBoundarySamples( continue; // Record called target sample and its count - const FrameLocation &LeafLoc = Binary->getInlineLeafFrameLoc(SourceOffset); - FunctionProfile.addCalledTargetSamples(LeafLoc.second.LineOffset, - LeafLoc.second.Discriminator, + auto LeafLoc = Binary->getInlineLeafFrameLoc(SourceOffset); + if (!LeafLoc.hasValue()) + continue; + FunctionProfile.addCalledTargetSamples(LeafLoc->second.LineOffset, + LeafLoc->second.Discriminator, CalleeName, Count); // Record head sample for called target(callee) @@ -277,7 +282,7 @@ void CSProfileGenerator::populateFunctionBoundarySamples( OCalleeCtxStr << ContextId.rsplit(" @ ").first.str(); OCalleeCtxStr << " @ "; } - OCalleeCtxStr << getCallSite(LeafLoc) << " @ " << CalleeName.str(); + OCalleeCtxStr << getCallSite(*LeafLoc) << " @ " << CalleeName.str(); FunctionSamples &CalleeProfile = getFunctionProfileForContext(OCalleeCtxStr.str()); diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp index d7588d680cca..df6ef2a7699b 100644 --- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp +++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp @@ -119,7 +119,8 @@ bool ProfiledBinary::inlineContextEqual(uint64_t Address1, const FrameLocationStack &Context2 = getFrameLocationStack(Offset2); if (Context1.size() != Context2.size()) return false; - + if (Context1.empty()) + return false; // The leaf frame contains location within the leaf, and it // needs to be remove that as it's not part of the calling context return std::equal(Context1.begin(), Context1.begin() + Context1.size() - 1, @@ -134,6 +135,10 @@ std::string ProfiledBinary::getExpandedContextStr( for (auto Address : Stack) { uint64_t Offset = virtualAddrToOffset(Address); const FrameLocationStack &ExpandedContext = getFrameLocationStack(Offset); + // An instruction without a valid debug line will be ignored by sample + // processing + if (ExpandedContext.empty()) + return std::string(); for (const auto &Loc : ExpandedContext) { ContextVec.push_back(getCallSite(Loc)); } @@ -242,9 +247,14 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes, const MCInstrDesc &MCDesc = MII->get(Inst.getOpcode()); // Populate a vector of the symbolized callsite at this location - InstructionPointer IP(this, Offset); - Offset2LocStackMap[Offset] = symbolize(IP, true); - + // We don't need symbolized info for probe-based profile, just use an empty + // stack as an entry to indicate a valid binary offset + FrameLocationStack SymbolizedCallStack; + if (!UsePseudoProbes) { + InstructionPointer IP(this, Offset); + SymbolizedCallStack = symbolize(IP, true); + } + Offset2LocStackMap[Offset] = SymbolizedCallStack; // Populate address maps. CodeAddrs.push_back(Offset); if (MCDesc.isCall()) diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.h b/llvm/tools/llvm-profgen/ProfiledBinary.h index f6c7460e186d..ccb1c9d32e46 100644 --- a/llvm/tools/llvm-profgen/ProfiledBinary.h +++ b/llvm/tools/llvm-profgen/ProfiledBinary.h @@ -11,6 +11,7 @@ #include "CallContext.h" #include "PseudoProbe.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" #include "llvm/DebugInfo/Symbolize/Symbolize.h" #include "llvm/MC/MCAsmInfo.h" @@ -225,9 +226,11 @@ public: return FuncStartAddrMap[Offset]; } - const FrameLocation &getInlineLeafFrameLoc(uint64_t Offset, - bool NameOnly = false) { - return getFrameLocationStack(Offset).back(); + Optional<const FrameLocation> getInlineLeafFrameLoc(uint64_t Offset) { + const auto &Stack = getFrameLocationStack(Offset); + if (Stack.empty()) + return {}; + return Stack.back(); } // Compare two addresses' inline context |