llvm-cov: Added edge struct for traversal in block.
Justin Bogner
mail at justinbogner.com
Mon Dec 2 14:21:50 PST 2013
LGTM, with one minor change:
Yuchen Wu <yuchenericwu at hotmail.com> writes:
> llvm-cov: Added edge struct for traversal in block.
>
> Added GCOVEdge which are simple structs owned by the GCOVFunction that
> stores the source and destination GCOVBlocks, as well as the counts.
> Changed GCOVBlocks so that it stores a vector of source GCOVEdges and a
> vector of destination GCOVEdges, rather than just the block number.
>
> Storing the block number was only useful for knowing the number of edges
> and for debug info. Using a struct is useful for traversing the edges,
> especially back edges which may be needed later.
>
> From bbd10e29ed053fb76bf88fba331ce4edd79dfc35 Mon Sep 17 00:00:00 2001
> From: Yuchen Wu <yuchen_wu at apple.com>
> Date: Thu, 14 Nov 2013 12:52:25 -0800
> Subject: [PATCH 22/26] llvm-cov: Added edge struct for traversal in block.
>
> Added GCOVEdge which are simple structs owned by the GCOVFunction that
> stores the source and destination GCOVBlocks, as well as the counts.
> Changed GCOVBlocks so that it stores a vector of source GCOVEdges and a
> vector of destination GCOVEdges, rather than just the block number.
>
> Storing the block number was only useful for knowing the number of edges
> and for debug info. Using a struct is useful for traversing the edges,
> especially back edges which may be needed later.
> ---
> include/llvm/Support/GCOV.h | 36 +++++++++++++++++++++++++++-----
> lib/IR/GCOV.cpp | 50 ++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 69 insertions(+), 17 deletions(-)
>
> diff --git a/include/llvm/Support/GCOV.h b/include/llvm/Support/GCOV.h
> index 3f7e29b..c385f45 100644
> --- a/include/llvm/Support/GCOV.h
> +++ b/include/llvm/Support/GCOV.h
> @@ -204,6 +204,14 @@ private:
> uint32_t ProgramCount;
> };
>
> +struct GCOVEdge {
> + GCOVEdge(GCOVBlock *S, GCOVBlock *D): Src(S), Dst(D), Count(0) {}
> +
> + GCOVBlock *Src;
> + GCOVBlock *Dst;
> + uint64_t Count;
> +};
> +
> /// GCOVFunction - Collects function information.
> class GCOVFunction {
> public:
> @@ -219,25 +227,43 @@ private:
> StringRef Name;
> StringRef Filename;
> SmallVector<GCOVBlock *, 16> Blocks;
> + SmallVector<GCOVEdge *, 16> Edges;
> };
>
> /// GCOVBlock - Collects block information.
> class GCOVBlock {
> public:
> + typedef SmallVectorImpl<GCOVEdge *>::const_iterator EdgeIterator;
> +
> GCOVBlock(GCOVFunction &P, uint32_t N) :
> - Parent(P), Number(N), Counter(0), Edges(), Lines() {}
> + Parent(P), Number(N), Counter(0), SrcEdges(), DstEdges(), Lines() {}
> ~GCOVBlock();
> - void addEdge(uint32_t N) { Edges.push_back(N); }
> + 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
> + DstEdges.push_back(Edge);
> + }
> void addLine(uint32_t N) { Lines.push_back(N); }
> - void addCount(uint64_t N) { Counter += N; }
> - size_t getNumEdges() const { return Edges.size(); }
> + void addCount(size_t Index, uint64_t N);
> + size_t getNumSrcEdges() const { return SrcEdges.size(); }
> + size_t getNumDstEdges() const { return DstEdges.size(); }
> +
> + EdgeIterator src_begin() const { return SrcEdges.begin(); }
> + EdgeIterator src_end() const { return SrcEdges.end(); }
> + EdgeIterator dst_begin() const { return DstEdges.begin(); }
> + EdgeIterator dst_end() const { return DstEdges.end(); }
> +
> void dump() const;
> void collectLineCounts(FileInfo &FI);
> private:
> GCOVFunction &Parent;
> uint32_t Number;
> uint64_t Counter;
> - SmallVector<uint32_t, 16> Edges;
> + SmallVector<GCOVEdge *, 16> SrcEdges;
> + SmallVector<GCOVEdge *, 16> DstEdges;
> SmallVector<uint32_t, 16> Lines;
> };
>
> diff --git a/lib/IR/GCOV.cpp b/lib/IR/GCOV.cpp
> index 6d5b5d1..69064a8 100644
> --- a/lib/IR/GCOV.cpp
> +++ b/lib/IR/GCOV.cpp
> @@ -114,6 +114,7 @@ void GCOVFile::collectLineCounts(FileInfo &FI) {
> /// ~GCOVFunction - Delete GCOVFunction and its content.
> GCOVFunction::~GCOVFunction() {
> DeleteContainerPointers(Blocks);
> + DeleteContainerPointers(Edges);
> }
>
> /// read - Read a function from the buffer. Return false if buffer cursor
> @@ -142,20 +143,22 @@ bool GCOVFunction::read(GCOVBuffer &Buff, GCOV::GCOVFormat Format) {
>
> // This for loop adds the counts for each block. A second nested loop is
> // required to combine the edge counts that are contained in the GCDA file.
> - for (uint32_t Line = 0; Count > 0; ++Line) {
> - if (Line >= Blocks.size()) {
> + for (uint32_t BlockNo = 0; Count > 0; ++BlockNo) {
> + // The last block is always reserved for exit block
> + if (BlockNo >= Blocks.size()-1) {
> errs() << "Unexpected number of edges.\n";
> return false;
> }
> - GCOVBlock &Block = *Blocks[Line];
> - for (size_t Edge = 0, End = Block.getNumEdges(); Edge < End; ++Edge) {
> + GCOVBlock &Block = *Blocks[BlockNo];
> + for (size_t EdgeNo = 0, End = Block.getNumDstEdges(); EdgeNo < End;
> + ++EdgeNo) {
> if (Count == 0) {
> errs() << "Unexpected number of edges.\n";
> return false;
> }
> uint64_t ArcCount;
> if (!Buff.readInt64(ArcCount)) return false;
> - Block.addCount(ArcCount);
> + Block.addCount(EdgeNo, ArcCount);
> --Count;
> }
> }
> @@ -190,7 +193,10 @@ bool GCOVFunction::read(GCOVBuffer &Buff, GCOV::GCOVFormat Format) {
> for (uint32_t i = 0, e = EdgeCount; i != e; ++i) {
> uint32_t Dst;
> if (!Buff.readInt(Dst)) return false;
> - Blocks[BlockNo]->addEdge(Dst);
> + GCOVEdge *Edge = new GCOVEdge(Blocks[BlockNo], Blocks[Dst]);
> + Edges.push_back(Edge);
> + Blocks[BlockNo]->addDstEdge(Edge);
> + Blocks[Dst]->addSrcEdge(Edge);
> if (!Buff.readInt(Dummy)) return false; // Edge flag
> }
> }
> @@ -249,10 +255,21 @@ void GCOVFunction::collectLineCounts(FileInfo &FI) {
>
> /// ~GCOVBlock - Delete GCOVBlock and its content.
> GCOVBlock::~GCOVBlock() {
> - Edges.clear();
> + SrcEdges.clear();
> + DstEdges.clear();
> Lines.clear();
> }
>
> +/// addCount - Add to block counter while also storing the edge count. If the
> +/// edge destination is the exit block, increment the exit block's count too.
> +void GCOVBlock::addCount(size_t Index, uint64_t N) {
> + assert(Index < DstEdges.size()); // up to the caller to ensure index is valid
> + DstEdges[Index]->Count = N;
> + Counter += N;
> + if (!DstEdges[Index]->Dst->getNumDstEdges())
> + DstEdges[Index]->Dst->Counter += N;
> +}
> +
This comment is a bit confusing, and the name Index isn't clear enough.
I find it a bit more obvious if you call it Dst or DstIndex.
I'm not sure how much clearer this is, but maybe something like:
Add the edge count for Dst to the block counter. If Dst is an exit
block, also track the exit count.
> /// collectLineCounts - Collect line counts. This must be used after
> /// reading .gcno and .gcda files.
> void GCOVBlock::collectLineCounts(FileInfo &FI) {
> @@ -264,11 +281,20 @@ void GCOVBlock::collectLineCounts(FileInfo &FI) {
> /// dump - Dump GCOVBlock content on standard out for debugging purposes.
> void GCOVBlock::dump() const {
> dbgs() << "Block : " << Number << " Counter : " << Counter << "\n";
> - if (!Edges.empty()) {
> - dbgs() << "\tEdges : ";
> - for (SmallVectorImpl<uint32_t>::const_iterator I = Edges.begin(), E = Edges.end();
> - I != E; ++I)
> - dbgs() << (*I) << ",";
> + if (!SrcEdges.empty()) {
> + dbgs() << "\tSource Edges : ";
> + for (EdgeIterator I = SrcEdges.begin(), E = SrcEdges.end(); I != E; ++I) {
> + const GCOVEdge *Edge = *I;
> + dbgs() << Edge->Src->Number << " (" << Edge->Count << "), ";
> + }
> + dbgs() << "\n";
> + }
> + if (!DstEdges.empty()) {
> + dbgs() << "\tDestination Edges : ";
> + for (EdgeIterator I = DstEdges.begin(), E = DstEdges.end(); I != E; ++I) {
> + const GCOVEdge *Edge = *I;
> + dbgs() << Edge->Dst->Number << " (" << Edge->Count << "), ";
> + }
> dbgs() << "\n";
> }
> if (!Lines.empty()) {
More information about the llvm-commits
mailing list