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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/ProfileData/GCOV.cpp:429
+GCOVBlock::augmentOneCycle(GCOVBlock *src,
+                           std::vector<std::pair<GCOVBlock *, size_t>> &stack) {
+  GCOVBlock *u;
----------------
xinhaoyuan wrote:
> 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.
Yeah: to reduce memory allocations. I'll keep it.


================
Comment at: llvm/lib/ProfileData/GCOV.cpp:467
+    }
+    return flow;
   }
----------------
xinhaoyuan wrote:
> 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.
Blocks which haven't been fully explored may have their traversable bits set. L495 is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93073