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

qiongsiwu1 added a comment. Thanks for chiming in @phosek ! In D155290#4587438 , @phosek wrote: > Can we avoid the use of `pthread_atfork` and the dependency on pthreads? > Using pthreads inside the profile runtime implementation is problematic in >

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

phosek added a comment. Can we avoid the use of `pthread_atfork` and the dependency on pthreads? Using pthreads inside the profile runtime implementation is problematic in the case where you want to instrument the libc itself which is used on some platforms and it's one of the reasons why we

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

qiongsiwu1 added a comment. In D155290#4582825 , @MaskRay wrote: > Using `%p` should not magically change the behavior and I am unsure the > result is better for PGO. > If we decide to provide such a mode, it needs to be opt-in by adding a new >

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

MaskRay added a comment. Actually, I am not sure changing `%p` to create multiple raw profile files should be the default. Currently, thanks to online profile merging we get just one raw profile file (though counters before fork may be doubly incremented). Using `%p` should not magically

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

MaskRay added a comment. In D155290#4576643 , @qiongsiwu1 wrote: > In D155290#4574797 , @MaskRay wrote: > >> In D155290#4572965 , @qiongsiwu1 >> wrote: >> >>> Ping for

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

MaskRay added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:94 static lprofFilename lprofCurFilename = {0, 0, 0, {0}, NULL, {0}, 0, 0, 0, PNS_unknown}; static int ProfileMergeRequested = 0;

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

qiongsiwu1 added a comment. In D155290#4574797 , @MaskRay wrote: > In D155290#4572965 , @qiongsiwu1 > wrote: > >> Ping for review. >> >> If there are no comments on whether we should add `-lpthread` for

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

MaskRay added a comment. In 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

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

qiongsiwu1 added a comment. 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)

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

qiongsiwu1 added a comment. Ping for review. Thanks! 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

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

qiongsiwu1 added a comment. Ping for review. Thanks so much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/ https://reviews.llvm.org/D155290 ___ cfe-commits mailing list

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

qiongsiwu1 added a comment. Ping for review. Thanks so much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/ https://reviews.llvm.org/D155290 ___ cfe-commits mailing list

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

qiongsiwu1 marked 2 inline comments as done. qiongsiwu1 added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440 + if (NeedsProfileRT || needsGCovInstrumentation(Args)) +CmdArgs.push_back("-lpthreads"); + w2yehia wrote: > w2yehia wrote: >

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

w2yehia added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440 + if (NeedsProfileRT || needsGCovInstrumentation(Args)) +CmdArgs.push_back("-lpthreads"); + The change in `compiler-rt/lib/profile/InstrProfilingFile.c` affects non-AIX

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

qiongsiwu1 added a comment. Ping for review. We'd like to get this in to LLVM 17 if possible. Please let me know if there are comments/suggestions. Thanks so much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/

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

qiongsiwu1 added a comment. Ping for review. Thanks so much! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/ https://reviews.llvm.org/D155290 ___ cfe-commits mailing list

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

qiongsiwu1 updated this revision to Diff 542042. qiongsiwu1 added a comment. Updating a test to see if it passes pre-commit CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/ https://reviews.llvm.org/D155290 Files:

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

qiongsiwu1 added a comment. Ping for review. Thanks!! 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

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

qiongsiwu1 updated this revision to Diff 541005. qiongsiwu1 added a comment. Minor comment spelling fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/ https://reviews.llvm.org/D155290 Files: clang/lib/Driver/ToolChains/AIX.cpp

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

qiongsiwu1 marked 2 inline comments as done. qiongsiwu1 added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:795 - /* When PNS >= OldPNS, the last one wins. */ - if (!FilenamePat || parseFilenamePattern(FilenamePat, CopyFilenamePat))

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

qiongsiwu1 updated this revision to Diff 540998. qiongsiwu1 added a comment. Revise the tests to use llvm tools to dump profile data to check for profile validity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155290/new/

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

davidxl added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:795 - /* When PNS >= OldPNS, the last one wins. */ - if (!FilenamePat || parseFilenamePattern(FilenamePat, CopyFilenamePat)) resetFilenameToDefault(); what is this

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

qiongsiwu1 created this revision. qiongsiwu1 added reviewers: w2yehia, davidxl, vsk, ellis. Herald added subscribers: wlei, Enna1, wenlei. Herald added a project: All. qiongsiwu1 requested review of this revision. Herald added subscribers: Sanitizers, cfe-commits, MaskRay. Herald added projects: