[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-28 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. In D58737#1412846 , @rnk wrote: > Oops, forgot to respond to this... > > In D58737#1412734 , @vsk wrote: > > > I'm confused by this wording re: comdats in the LangRef: "All global > > objects t

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355044: [InstrProf] Use separate comdat group for data and counters (authored by rnk, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D58737?vs=18

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D58737#1412847 , @xur wrote: > lgtm. > > BTW, I'm in the process of committing D54175 > . After that patch, PGO instrumentation can > be called after the main inlining. I don't think it will confli

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Rong Xu via Phabricator via cfe-commits
xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. lgtm. BTW, I'm in the process of committing D54175 . After that patch, PGO instrumentation can be called after the main inlining. I don't think it will conflict any

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Oops, forgot to respond to this... In D58737#1412734 , @vsk wrote: > I'm confused by this wording re: comdats in the LangRef: "All global objects > that specify this key will only end up in the final object file if the linker > choo

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 188635. rnk marked an inline comment as done. rnk added a comment. - share code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58737/new/ https://reviews.llvm.org/D58737 Files: clang/test/Profile/cxx-templates.cp

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked 2 inline comments as done. rnk added a comment. In D58737#1412734 , @vsk wrote: > I'm confused by this wording re: comdats in the LangRef: "All global objects > that specify this key will only end up in the final object file if the linker > ch

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Rong Xu via Phabricator via cfe-commits
xur added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:756 + // For COFF, the comdat group name must be the name of a symbol in the + // group. Use the counter variable name. + Cmdt = M->getOrInsertComdat( So

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment. I'm confused by this wording re: comdats in the LangRef: "All global objects that specify this key will only end up in the final object file if the linker chooses that key over some other key". Why can multiple global objects with the same comdat key end up in the final obj

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

2019-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: xur, vsk. Herald added subscribers: Sanitizers, cfe-commits, hiraditya, eraman. Herald added projects: clang, Sanitizers, LLVM. I hadn't realized that instrumentation runs before inlining, so we can't use the function as the comdat group. Doing so ca