calixte added inline comments.
================
Comment at: compiler-rt/lib/profile/GCDAProfiling.c:671
+ pid_t child_pid = getpid();
+ if (child_pid != parent_pid) {
+ // The pid changed so we've a fork
----------------
marco-c wrote:
> Nit: do we need this check or can we just use the earlier one on pid == 0?
I added this check to be sure we had a true fork in case of someone wrote its
own fork function.
================
Comment at: compiler-rt/lib/profile/GCDAProfiling.c:675
+ // No need to lock here since we just forked and cannot have any other
+ // threads.
+ struct fn_node *curr = reset_fn_list.head;
----------------
marco-c wrote:
> What if we have a thread in another process making modifications to the list?
When forking, only the thread in the parent process is copied so in the child
process we've only one thread.
When forking, the parent and the child process share the same memory until some
change (Copy-on-Write), so even if in the parent process a list is changed then
the memory will be copied before the change and then the child process will
have an unchanged list.
================
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()) {
----------------
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.
================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:671
+ // We split just after the fork to have a counter for the lines after
+ // Anyway there's a bug:
+ // void foo() { fork(); }
----------------
marco-c wrote:
> Isn't this bug fixed by splitting the block?
With the example shew here, the split is in foo but not in bar, blah() should
be in another block
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
