[PATCH] D93073: [llvm-cov gcov] Replace Donald B. Johnson's cycle enumeration with iterative cycle finding

[Xinhao Yuan via Phabricator via llvm-commits]

xinhaoyuan accepted this revision.
xinhaoyuan added a comment.
This revision is now accepted and ready to land.

LGTM. Left some style nitpicks.



================
Comment at: llvm/lib/ProfileData/GCOV.cpp:429
+GCOVBlock::augmentOneCycle(GCOVBlock *src,
+                           std::vector<std::pair<GCOVBlock *, size_t>> &stack) {
+  GCOVBlock *u;
----------------
nit: you can define stack as a local variable. e.g.

`std::vector<std::pair<GCOVBlock *, size_t>> stack = {{src, 0}};`

making it an argument probably is more efficient, but I think the performance benefit is marginal - leaving it to you.


================
Comment at: llvm/lib/ProfileData/GCOV.cpp:446
+    GCOVArc *succ = u->succ[i];
+    // Ignore saturaged arcs (cycleCount has been reduced to 0) and visited
+    // blocks. Ignore self arcs to guard against bad input (.gcno has no
----------------
saturated


================
Comment at: llvm/lib/ProfileData/GCOV.cpp:467
+    }
+    return flow;
   }
----------------
MaskRay wrote:
> xinhaoyuan wrote:
> > when returned with a cycle found. "incoming" of the blocks on the cycle would not be cleared. can it lead to undefined behaviors in the subsequent traversal?
> Good catch! I'll set the visited bits for these blocks.
On second thought, I think "traversable" of the blocks should be be all false at L495, since the loop exits only when all blocks are traversed with no loop found. but I think it is fine to keep the cleanup as a safe guard.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93073/new/

https://reviews.llvm.org/D93073