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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
