[PATCH] llvm-cov: Added -b option for branch probabilities.
Justin Bogner
mail at justinbogner.com
Wed Dec 11 16:36:22 PST 2013
Yuchen Wu <yuchenericwu at hotmail.com> writes:
> From 5fc36ff869820aa0b527d7da1469541a413bd068 Mon Sep 17 00:00:00 2001
> From: Yuchen Wu <yuchen_wu at apple.com>
> Date: Tue, 3 Dec 2013 14:27:17 -0800
> Subject: [PATCH 2/7] llvm-cov: Added -b option for branch probabilities.
>
> This option tells llvm-cov to print out branch probabilities when
> a basic block contains multiple branches. It also prints out some
> function summary info including the number of times the function enters,
> the percent of time it returns, and how many blocks were executed.
>
> Also updated tests.
> ---
> include/llvm/Support/GCOV.h | 61 +++++++++--
> lib/IR/GCOV.cpp | 125 ++++++++++++++++++++---
> test/tools/llvm-cov/Inputs/test_-a_-b.cpp.gcov | 134 +++++++++++++++++++++++++
> test/tools/llvm-cov/Inputs/test_-a_-b.h.gcov | 12 +++
> test/tools/llvm-cov/llvm-cov.test | 4 +
> tools/llvm-cov/llvm-cov.cpp | 5 +-
> 6 files changed, 317 insertions(+), 24 deletions(-)
> create mode 100644 test/tools/llvm-cov/Inputs/test_-a_-b.cpp.gcov
> create mode 100644 test/tools/llvm-cov/Inputs/test_-a_-b.h.gcov
>
> diff --git a/include/llvm/Support/GCOV.h b/include/llvm/Support/GCOV.h
> index 852c0be..6708802 100644
> --- a/include/llvm/Support/GCOV.h
> +++ b/include/llvm/Support/GCOV.h
> @@ -36,9 +36,11 @@ namespace GCOV {
>
> /// GCOVOptions - A struct for passing gcov options between functions.
> struct GCOVOptions {
> - GCOVOptions(bool A): AllBlocks(A) {}
> + GCOVOptions(bool A, bool B): AllBlocks(A), BranchProb(B) {}
> + operator bool() const { return AllBlocks || BranchProb; }
>
> bool AllBlocks;
> + bool BranchProb;
> };
>
> /// GCOVBuffer - A wrapper around MemoryBuffer to provide GCOV specific
> @@ -236,6 +238,7 @@ private:
> uint32_t ProgramCount;
> };
>
> +/// GCOVEdge - Collects edge information.
> struct GCOVEdge {
> GCOVEdge(GCOVBlock *S, GCOVBlock *D): Src(S), Dst(D), Count(0) {}
>
> @@ -247,12 +250,21 @@ struct GCOVEdge {
> /// GCOVFunction - Collects function information.
> class GCOVFunction {
> public:
> + typedef SmallVectorImpl<GCOVBlock *>::const_iterator BlockIterator;
> +
> GCOVFunction(GCOVFile &P) : Parent(P), Ident(0), LineNumber(0) {}
> ~GCOVFunction();
> bool readGCNO(GCOVBuffer &Buffer, GCOV::GCOVVersion Version);
> bool readGCDA(GCOVBuffer &Buffer, GCOV::GCOVVersion Version);
> + StringRef getName() const { return Name; }
> StringRef getFilename() const { return Filename; }
> size_t getNumBlocks() const { return Blocks.size(); }
> + uint64_t getEntryCount() const;
> + uint64_t getExitCount() const;
> +
> + BlockIterator block_begin() const { return Blocks.begin(); }
> + BlockIterator block_end() const { return Blocks.end(); }
> +
> void dump() const;
> void collectLineCounts(FileInfo &FI);
> private:
> @@ -268,26 +280,43 @@ private:
>
> /// GCOVBlock - Collects block information.
> class GCOVBlock {
> + struct EdgeWeight {
> + EdgeWeight(GCOVBlock *D): Dst(D), Count(0) {}
> +
> + GCOVBlock *Dst;
> + uint64_t Count;
> + };
> +
> + struct SortDstEdgesFtor {
Please spell out Functor here. Ftor is not obvious.
> + bool operator()(const GCOVEdge *E1, const GCOVEdge *E2) {
> + return E1->Dst->Number < E2->Dst->Number;
> + }
> + };
> public:
> typedef SmallVectorImpl<GCOVEdge *>::const_iterator EdgeIterator;
>
> - GCOVBlock(GCOVFunction &P, uint32_t N) :
> - Parent(P), Number(N), Counter(0), SrcEdges(), DstEdges(), Lines() {}
> + GCOVBlock(GCOVFunction &P, uint32_t N) : Parent(P), Number(N), Counter(0),
> + SortedDstEdges(true), SrcEdges(), DstEdges(), Lines() {}
> ~GCOVBlock();
> + void addLine(uint32_t N) { Lines.push_back(N); }
> + uint32_t getLastLine() const { return Lines.back(); }
> + void addCount(size_t DstEdgeNo, uint64_t N);
> + uint64_t getCount() const { return Counter; }
> +
> void addSrcEdge(GCOVEdge *Edge) {
> assert(Edge->Dst == this); // up to caller to ensure edge is valid
> SrcEdges.push_back(Edge);
> }
> void addDstEdge(GCOVEdge *Edge) {
> assert(Edge->Src == this); // up to caller to ensure edge is valid
> + // If block numbers in edge vector are not in order, set false
> + if (DstEdges.size() && DstEdges.back()->Dst->Number > Edge->Dst->Number)
> + SortedDstEdges = false;
The comment here is just describing the following code, instead of
saying what it's doing. Something more like "Check if adding this edge
causes us to become unsorted" would be more helpful to the reader.
> DstEdges.push_back(Edge);
> }
> - void addLine(uint32_t N) { Lines.push_back(N); }
> - uint32_t getLastLine() const { return Lines.back(); }
> - void addCount(size_t DstEdgeNo, uint64_t N);
> - uint64_t getCount() const { return Counter; }
> size_t getNumSrcEdges() const { return SrcEdges.size(); }
> size_t getNumDstEdges() const { return DstEdges.size(); }
> + bool orderDstEdges();
Why order? Isn't the verb you're looking for here "sort"?
>
> EdgeIterator src_begin() const { return SrcEdges.begin(); }
> EdgeIterator src_end() const { return SrcEdges.end(); }
> @@ -300,17 +329,31 @@ private:
> GCOVFunction &Parent;
> uint32_t Number;
> uint64_t Counter;
> + bool SortedDstEdges;
I don't like this name. IsSorted or DstEdgesAreSorted read better.
> SmallVector<GCOVEdge *, 16> SrcEdges;
> SmallVector<GCOVEdge *, 16> DstEdges;
> SmallVector<uint32_t, 16> Lines;
> };
>
> +// It is unlikely--but possible--for multiple functions to be on the same line.
> +// But optimize for the common case.
What is this comment talking about?
> +typedef SmallVector<const GCOVFunction *, 1> FunctionVector;
> +typedef DenseMap<uint32_t, FunctionVector> FunctionLines;
> typedef SmallVector<const GCOVBlock *, 4> BlockVector;
> -typedef DenseMap<uint32_t, BlockVector> LineData;
> +typedef DenseMap<uint32_t, BlockVector> BlockLines;
> +
> class FileInfo {
> + struct LineData {
> + BlockLines Blocks;
> + FunctionLines Functions;
> + };
> public:
> void addBlockLine(StringRef Filename, uint32_t Line, const GCOVBlock *Block) {
> - LineInfo[Filename][Line-1].push_back(Block);
> + LineInfo[Filename].Blocks[Line-1].push_back(Block);
> + }
> + void addFunctionLine(StringRef Filename, uint32_t Line,
> + const GCOVFunction *Function) {
> + LineInfo[Filename].Functions[Line-1].push_back(Function);
> }
> void setRunCount(uint32_t Runs) { RunCount = Runs; }
> void setProgramCount(uint32_t Programs) { ProgramCount = Programs; }
> diff --git a/lib/IR/GCOV.cpp b/lib/IR/GCOV.cpp
> index fb73700..2df36d0 100644
> --- a/lib/IR/GCOV.cpp
> +++ b/lib/IR/GCOV.cpp
> @@ -19,6 +19,7 @@
> #include "llvm/Support/Format.h"
> #include "llvm/Support/MemoryObject.h"
> #include "llvm/Support/system_error.h"
> +#include <algorithm>
> using namespace llvm;
>
> //===----------------------------------------------------------------------===//
> @@ -278,10 +279,25 @@ bool GCOVFunction::readGCDA(GCOVBuffer &Buff, GCOV::GCOVVersion Version) {
> Block.addCount(EdgeNo, ArcCount);
> --Count;
> }
> + // Sort destination edges by block number, nop if already sorted.
> + // This is required for printing branch info in the correct order.
> + Block.orderDstEdges();
Why are you explaining what this function does here, instead of having a
doc comment on the function itself?
> }
> return true;
> }
>
> +/// getEntryCount - Get the number of times the function was called by
> +/// retrieving the entry block's count.
> +uint64_t GCOVFunction::getEntryCount() const {
> + return Blocks.front()->getCount();
> +}
> +
> +/// getExitCount - Get the number of times the function returned by retrieving
> +/// the exit block's count.
> +uint64_t GCOVFunction::getExitCount() const {
> + return Blocks.back()->getCount();
> +}
> +
> /// dump - Dump GCOVFunction content to dbgs() for debugging purposes.
> void GCOVFunction::dump() const {
> dbgs() << "===== " << Name << " @ " << Filename << ":" << LineNumber << "\n";
> @@ -296,6 +312,7 @@ void GCOVFunction::collectLineCounts(FileInfo &FI) {
> for (SmallVectorImpl<GCOVBlock *>::iterator I = Blocks.begin(),
> E = Blocks.end(); I != E; ++I)
> (*I)->collectLineCounts(FI);
> + FI.addFunctionLine(Filename, LineNumber, this);
> }
>
> //===----------------------------------------------------------------------===//
> @@ -318,6 +335,15 @@ void GCOVBlock::addCount(size_t DstEdgeNo, uint64_t N) {
> DstEdges[DstEdgeNo]->Dst->Counter += N;
> }
>
> +bool GCOVBlock::orderDstEdges() {
> + if (SortedDstEdges)
> + return false;
> +
> + SortDstEdgesFtor SortEdges;
> + std::stable_sort(DstEdges.begin(), DstEdges.end(), SortEdges);
> + return true;
> +}
I'm not convinced the return value is all that useful - you're basically
just exposing an implementation detail.
> /// collectLineCounts - Collect line counts. This must be used after
> /// reading .gcno and .gcda files.
> void GCOVBlock::collectLineCounts(FileInfo &FI) {
> @@ -357,6 +383,25 @@ void GCOVBlock::dump() const {
> //===----------------------------------------------------------------------===//
> // FileInfo implementation.
>
> +// Safe integer division, returns 0 if divisor is 0.
> +static uint32_t safeDiv(uint64_t Numerator, uint64_t Divisor) {
> + if (Divisor == 0)
> + return 0;
> + return Numerator/Divisor;
> +}
Why do you need this?
> +
> +// This custom division function mimics gcov's branch ouputs:
> +// - Round to closest whole number
> +// - Only output 0% or 100% if it's exactly that value
> +static uint32_t branchDiv(uint64_t Numerator, uint64_t Divisor) {
> + uint8_t Res = safeDiv(Numerator*100+Divisor/2, Divisor);
> + if (Res == 0 && Numerator)
> + return 1;
This seems like pretty strange behaviour if Divisor == 0.
> + if (Res == 100 && Numerator != Divisor)
> + return 99;
> + return Res;
> +}
> +
> /// print - Print source files with collected line count information.
> void FileInfo::print(StringRef GCNOFile, StringRef GCDAFile,
> const GCOVOptions &Options) const {
> @@ -384,11 +429,36 @@ void FileInfo::print(StringRef GCNOFile, StringRef GCDAFile,
>
> const LineData &Line = I->second;
> for (uint32_t i = 0; !AllLines.empty(); ++i) {
> - LineData::const_iterator BlocksIt = Line.find(i);
> + if (Options.BranchProb) {
> + // Print function and block summary.
> + FunctionLines::const_iterator FuncsIt = Line.Functions.find(i);
> + if (FuncsIt != Line.Functions.end()) {
> + const SmallVectorImpl<const GCOVFunction *> &Funcs = FuncsIt->second;
> + for (SmallVectorImpl<const GCOVFunction*>::const_iterator I =
> + Funcs.begin(), E = Funcs.end(); I != E; ++I) {
> + const GCOVFunction *Func = *I;
> + uint64_t EntryCount = Func->getEntryCount();
> + uint32_t BlocksExecuted = 0;
> + for (GCOVFunction::BlockIterator I = Func->block_begin(),
> + E = Func->block_end(); I != E; ++I) {
> + const GCOVBlock *Block = *I;
> + if (Block->getNumDstEdges() && Block->getCount())
> + ++BlocksExecuted;
> + }
> +
> + OS << "function " << Func->getName() << " called " << EntryCount
> + << " returned " << safeDiv(Func->getExitCount()*100, EntryCount)
> + << "% blocks executed "
> + << safeDiv(BlocksExecuted*100, Func->getNumBlocks()-1) << "%\n";
> + }
> + }
> + }
Would it simplify things to break this out into a function? This is
pretty nested and a bit hard to read.
>
> - if (BlocksIt != Line.end()) {
> - // Add up the block counts to form line counts.
> + BlockLines::const_iterator BlocksIt = Line.Blocks.find(i);
> + if (BlocksIt != Line.Blocks.end()) {
> const BlockVector &Blocks = BlocksIt->second;
> +
> + // Add up the block counts to form line counts.
> uint64_t LineCount = 0;
> for (BlockVector::const_iterator I = Blocks.begin(), E = Blocks.end();
> I != E; ++I) {
> @@ -413,21 +483,48 @@ void FileInfo::print(StringRef GCNOFile, StringRef GCDAFile,
> OS << format("%5u:", i+1) << P.first << "\n";
> AllLines = P.second;
>
> - if (Options.AllBlocks && BlocksIt != Line.end()) {
> - // Output the counts for each block at the last line of the block.
> - uint32_t BlockNo = 0;
> - const BlockVector &Blocks = BlocksIt->second;
> - for (BlockVector::const_iterator I = Blocks.begin(), E = Blocks.end();
> - I != E; ++I) {
> - const GCOVBlock *Block = *I;
> - if (Block->getLastLine() != i+1)
> - continue;
> -
> + if (!Options || BlocksIt == Line.Blocks.end())
> + continue;
> +
> + // Everything below implements additional options supported by gcov
This is kind of an ugly way to break things up. More functions?
> + uint32_t BlockNo = 0;
> + uint32_t EdgeNo = 0;
> + const BlockVector &Blocks = BlocksIt->second;
> + for (BlockVector::const_iterator I = Blocks.begin(), E = Blocks.end();
> + I != E; ++I) {
> + const GCOVBlock *Block = *I;
> + if (Block->getLastLine() != i+1)
> + continue;
> +
> + if (Options.AllBlocks) {
> + // Output the counts for each block at the last line of the block.
> if (Block->getCount() == 0)
> OS << " $$$$$:";
> else
> OS << format("%9lu:", Block->getCount());
> - OS << format("%5u-block %u\n", i+1, BlockNo++);
> + OS << format("%5u-block %2u\n", i+1, BlockNo++);
> + }
> +
> + if (Options.BranchProb && Block->getNumDstEdges() > 1) {
> + // Print branch probabilities for blocks that have conditional
> + // branches.
> + SmallVector<uint64_t, 16> BranchCounts;
> + uint64_t TotalCounts = 0;
> + for (GCOVBlock::EdgeIterator I = Block->dst_begin(),
> + E = Block->dst_end(); I != E; ++I) {
> + const GCOVEdge *Edge = *I;
> + BranchCounts.push_back(Edge->Count);
> + TotalCounts += Edge->Count;
> + }
> +
> + for (SmallVectorImpl<uint64_t>::const_iterator I =
> + BranchCounts.begin(), E = BranchCounts.end(); I != E; ++I) {
> + if (TotalCounts)
> + OS << format("branch %2u taken %u%%\n", EdgeNo++,
> + branchDiv(*I, TotalCounts));
> + else
> + OS << format("branch %2u never executed\n", EdgeNo++);
> + }
> }
> }
> }
> diff --git a/test/tools/llvm-cov/Inputs/test_-a_-b.cpp.gcov b/test/tools/llvm-cov/Inputs/test_-a_-b.cpp.gcov
> new file mode 100644
> index 0000000..ae21037
> --- /dev/null
> +++ b/test/tools/llvm-cov/Inputs/test_-a_-b.cpp.gcov
> @@ -0,0 +1,134 @@
> + -: 0:Source:test.cpp
> + -: 0:Graph:test.gcno
> + -: 0:Data:test.gcda
> + -: 0:Runs:2
> + -: 0:Programs:1
> + -: 1:#include "test.h"
> + -: 2:#include <cstdlib>
> + -: 3:
> + -: 4:bool on = false;
> + -: 5:int len = 42;
> + -: 6:double grid[10][10] = {0};
> + -: 7:const char * hello = "world";
> + -: 8:const char * world = "hello";
> + -: 9:
> +function _ZN1A1BEv called 8589934592 returned 100% blocks executed 100%
> +8589934592: 10:void A::B() {}
> +8589934592: 10-block 0
> + -: 11:
> +function _Z7uselessv called 0 returned 0% blocks executed 0%
> + #####: 12:void useless() {}
> + $$$$$: 12-block 0
> + -: 13:
> +function _Z12more_uselessv called 0 returned 0% blocks executed 0%
> + -: 14:double more_useless() {
> + #####: 15: return 0;
> + $$$$$: 15-block 0
> + -: 16:}
> + -: 17:
> +function _Z3foov called 2 returned 100% blocks executed 100%
> + -: 18:int foo() {
> + 2: 19: on = true;
> + 2: 20: return 3;
> + 2: 20-block 0
> + -: 21:}
> + -: 22:
> +function _Z3barv called 0 returned 0% blocks executed 0%
> + -: 23:int bar() {
> + #####: 24: len--;
> + #####: 25: return foo() + 45;
> + $$$$$: 25-block 0
> + -: 26:}
> + -: 27:
> +function _Z6assignii called 8 returned 100% blocks executed 100%
> + 8: 28:void assign(int ii, int jj) {
> + 8: 29: grid[ii][jj] = (ii+1) * (jj+1);
> + 8: 30:}
> + 8: 30-block 0
> + -: 31:
> +function _Z15initialize_gridv called 2 returned 100% blocks executed 100%
> + -: 32:void initialize_grid() {
> + 6: 33: for (int ii = 0; ii < 2; ii++)
> + 2: 33-block 0
> + 6: 33-block 1
> +branch 0 taken 67%
> +branch 1 taken 33%
> + 4: 33-block 2
> + 12: 34: for (int jj = 0; jj < 2; jj++)
> + 4: 34-block 0
> + 12: 34-block 1
> +branch 0 taken 67%
> +branch 1 taken 33%
> + 8: 34-block 2
> + 8: 35: assign(ii, jj);
> + 8: 35-block 0
> + 4: 35-block 1
> + 2: 36:}
> + 2: 36-block 0
> + -: 37:
> +function main called 2 returned 100% blocks executed 94%
> + -: 38:int main() {
> + 2: 39: initialize_grid();
> + -: 40:
> + 2: 41: int a = 2;
> + 2: 42: on = rand() % 2;
> + 2: 43: if (on) {
> + 2: 43-block 0
> +branch 0 taken 100%
> +branch 1 taken 0%
> + 2: 44: foo();
> + 2: 45: ++a;
> + 2: 46: } else {
> + 2: 46-block 0
> + #####: 47: bar();
> + #####: 48: a += rand();
> + $$$$$: 48-block 0
> + -: 49: }
> + -: 50:
> + 22: 51: for (int ii = 0; ii < 10; ++ii) {
> + 2: 51-block 0
> + 22: 51-block 1
> +branch 0 taken 91%
> +branch 1 taken 9%
> + 20: 51-block 2
> + 20: 52: switch (rand() % 5) {
> + 20: 52-block 0
> +branch 0 taken 20%
> +branch 1 taken 0%
> +branch 2 taken 10%
> +branch 3 taken 30%
> +branch 4 taken 40%
> + -: 53: case 0:
> + 4: 54: a += rand();
> + 4: 55: break;
> + 4: 55-block 0
> + -: 56: case 1:
> + -: 57: case 2:
> + 2: 58: a += rand() / rand();
> + 2: 59: break;
> + 2: 59-block 0
> + -: 60: case 3:
> + 6: 61: a -= rand();
> + 6: 62: break;
> + 6: 62-block 0
> + -: 63: default:
> + 8: 64: a = -1;
> + 8: 65: }
> + 8: 65-block 0
> + 20: 66: }
> + 20: 66-block 0
> + -: 67:
> + 2: 68: A thing;
> +8589934594: 69: for (uint64_t ii = 0; ii < 4294967296; ++ii)
> + 2: 69-block 0
> +8589934594: 69-block 1
> +branch 0 taken 99%
> +branch 1 taken 1%
> +8589934592: 69-block 2
> +8589934592: 70: thing.B();
> +8589934592: 70-block 0
> + -: 71:
> + 2: 72: return a + 8 + grid[2][3] + len;
> + 2: 72-block 0
> + -: 73: return more_useless();
> + -: 74:}
> diff --git a/test/tools/llvm-cov/Inputs/test_-a_-b.h.gcov b/test/tools/llvm-cov/Inputs/test_-a_-b.h.gcov
> new file mode 100644
> index 0000000..f3dabcb
> --- /dev/null
> +++ b/test/tools/llvm-cov/Inputs/test_-a_-b.h.gcov
> @@ -0,0 +1,12 @@
> + -: 0:Source:./test.h
> + -: 0:Graph:test.gcno
> + -: 0:Data:test.gcda
> + -: 0:Runs:2
> + -: 0:Programs:1
> +function _ZN1AC1Ev called 2 returned 100% blocks executed 100%
> +function _ZN1AC2Ev called 2 returned 100% blocks executed 100%
> + 2: 1:struct A {
> + 2: 1-block 0
> + 2: 1-block 1
> + -: 2: virtual void B();
> + -: 3:};
> diff --git a/test/tools/llvm-cov/llvm-cov.test b/test/tools/llvm-cov/llvm-cov.test
> index 43725a3..bf0fb07 100644
> --- a/test/tools/llvm-cov/llvm-cov.test
> +++ b/test/tools/llvm-cov/llvm-cov.test
> @@ -13,6 +13,10 @@ RUN: llvm-cov -gcno=test.gcno -gcda=test.gcda -a
> RUN: diff -aub test_-a.cpp.gcov test.cpp.gcov
> RUN: diff -aub test_-a.h.gcov test.h.gcov
>
> +RUN: llvm-cov -gcno=test.gcno -gcda=test.gcda -a -b
> +RUN: diff -aub test_-a_-b.cpp.gcov test.cpp.gcov
> +RUN: diff -aub test_-a_-b.h.gcov test.h.gcov
> +
> RUN: not llvm-cov -gcno=test_read_fail.gcno -gcda=test.gcda
>
> RUN: not llvm-cov -gcno=test.gcno -gcda=test_file_checksum_fail.gcda
> diff --git a/tools/llvm-cov/llvm-cov.cpp b/tools/llvm-cov/llvm-cov.cpp
> index 235670b..6c6f33c 100644
> --- a/tools/llvm-cov/llvm-cov.cpp
> +++ b/tools/llvm-cov/llvm-cov.cpp
> @@ -33,6 +33,9 @@ InputGCDA("gcda", cl::desc("<input gcda file>"), cl::init(""));
> static cl::opt<bool>
> AllBlocks("a", cl::init(false), cl::desc("display all block info"));
>
> +static cl::opt<bool>
> +BranchProb("b", cl::init(false), cl::desc("display branch info"));
> +
> //===----------------------------------------------------------------------===//
> int main(int argc, char **argv) {
> // Print a stack trace if we signal out.
> @@ -75,6 +78,6 @@ int main(int argc, char **argv) {
>
> FileInfo FI;
> GF.collectLineCounts(FI);
> - FI.print(InputGCNO, InputGCDA, GCOVOptions(AllBlocks));
> + FI.print(InputGCNO, InputGCDA, GCOVOptions(AllBlocks, BranchProb));
> return 0;
> }
More information about the llvm-commits
mailing list