[PATCH] D74953: [profile] Don't dump counters when forking and don't reset when calling exec** functions

[Marco Castelluccio via Phabricator via llvm-commits]

marco-c accepted this revision.
marco-c added a comment.
This revision is now accepted and ready to land.

Could you test the last iteration of the patch on Mozilla's CI (with the workaround for the mismatch in LLVM version used by Rust)?



================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:635
 void GCOVProfiler::AddFlushBeforeForkAndExec() {
-  SmallVector<Instruction *, 2> ForkAndExecs;
+  SmallVector<std::pair<bool, CallInst *>, 2> ForkAndExecs;
   for (auto &F : M->functions()) {
----------------
calixte wrote:
> marco-c wrote:
> > Since we are now mostly doing different things on forks and execs, we could remove this vector and just do the operations directly instead of adding to the vec.
> If we make the insertions of new code in the second for loop, we'll invalidate the iterator used in this loop. 
M->getOrInsertFunction is what would invalidate the iterator?

You could clean things up a bit by having two vectors then, one for forks and one for execs.


================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:686
+      // Since the process is replaced by a new one we need to write out gcdas
+      // No need to reset the counters since there'll be lost after the exec**
+      FunctionType *FTy = FunctionType::get(Builder.getVoidTy(), {}, false);
----------------
Replace with `// No need to reset the counters since they'll be lost after the exec**`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74953