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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to