[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D155290#4576643 <https://reviews.llvm.org/D155290#4576643>, @qiongsiwu1 wrote:

> In D155290#4574797 <https://reviews.llvm.org/D155290#4574797>, @MaskRay wrote:
>
>> In D155290#4572965 <https://reviews.llvm.org/D155290#4572965>, @qiongsiwu1 wrote:
>>
>>> Ping for review.
>>>
>>> If there are no comments on whether we should add `-lpthread` for platforms other than AIX, I will land this patch soon, and let the official buildbots check for problems. If there are problems, I will let the bots run a bit (probably a day or so) before reverting to catch as many platforms I need to fix as possible, revert, and fix the patch.
>>>
>>> Thanks!
>>
>> I will take a look as well but please don't land this `-lpthread` change for non-AIX platforms. I will think about the implication.
>
> Thanks @MaskRay ! I am putting this patch on hold till I hear back. The patch does not alter non-AIX platforms at the moment.

The `pthread_atfork` undefined reference will cause a problem using glibc<2.34 where `pthread_atfork` lives in libpthread instead of libc.
I think non-AIX-non-Windows systems can live with an extra `-lpthread`. For ELF systems (not Apple's among non-AIX-non-Windows systems), `-lpthread` must be after `libclang_rt.profile.a`.



================
Comment at: compiler-rt/test/profile/Posix/instrprof-fork.c:22
+  pid = fork();
+  if (!pid) {
+    printf("%ld.profraw\n", (long)getpid());
----------------
MaskRay wrote:
> parent and child have the same logic. Use:
> ```
> if (pid == -1)
>   return 1;
> printf("%ld.profraw\n", (long)getpid());
> ```
Please ignore my previous comment. I think we want to have a deterministic parent / child output order. Consider adding a wait for the parent process so that we always see the child `printf("%ld.profraw\n", (long)getpid());` output before the parent one.

We should inspect more information of the two raw profile files. Create a function only called by the parent and another function only called by the child. Check their counters.

We also need a test if the child process spawns another process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290