[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread Marco Castelluccio via Phabricator via cfe-commits
marco-c added a comment. Yes, the behavior changed very recently, I would be surprised if somebody came to depend on it. It's more likely that some clients are depending on the old behavior. - Marco. Il 26/06/2018 22:43, Stephen Hines via Phabricator ha scritto: > srhines added a comment. > >

Re: [PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread Marco Castelluccio via cfe-commits
Yes, the behavior changed very recently, I would be surprised if somebody came to depend on it. It's more likely that some clients are depending on the old behavior. - Marco. Il 26/06/2018 22:43, Stephen Hines via Phabricator ha scritto: > srhines added a comment. > > In

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread Stephen Hines via Phabricator via cfe-commits
srhines added a comment. In https://reviews.llvm.org/D45454#1144099, @davidxl wrote: > GCC's behavior is not documented and it also has changed over the years. > > Unless there is a bug, changing LLVM's gcov_flush visibility can potentially > break clients that depend on this behavior. I

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. GCC's behavior is not documented and it also has changed over the years. Unless there is a bug, changing LLVM's gcov_flush visibility can potentially break clients that depend on this behavior. https://reviews.llvm.org/D45454

Re: [PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread Marco Castelluccio via cfe-commits
Wouldn't it be better to keep compatibility with GCC and make __gcov_flush have default visibility? - Marco. Il 26/06/2018 00:21, Xinliang David Li ha scritto: > I don't have an objection having another interface which is just a > simple wrapper to __gcov_flush but with default visibility. Also

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-26 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh updated this revision to Diff 152957. chh edited the summary of this revision. https://reviews.llvm.org/D45454 Files: lib/profile/GCDAProfiling.c test/profile/Inputs/instrprof-dlopen-dlclose-main.c Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c

Re: [PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-25 Thread Xinliang David Li via cfe-commits
I don't have an objection having another interface which is just a simple wrapper to __gcov_flush but with default visibility. Also clearly document its usage and behavior. David On Mon, Jun 25, 2018 at 10:12 AM, Chih-Hung Hsieh via Phabricator via llvm-commits wrote: > chh added a comment. >

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-25 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. In https://reviews.llvm.org/D45454#1142197, @marco-c wrote: > In https://reviews.llvm.org/D45454#1070884, @belleyb wrote: > > > @chh I had a chance to try out your proposed changes. It's not causing us > > any trouble. In fact, `__gcov_flush()` is not even used at all (at

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-06-25 Thread Marco Castelluccio via Phabricator via cfe-commits
marco-c added a comment. In https://reviews.llvm.org/D45454#1070884, @belleyb wrote: > @chh I had a chance to try out your proposed changes. It's not causing us any > trouble. In fact, `__gcov_flush()` is not even used at all (at least in LLVM > 5.0.1).. I can recompile llvm, compiler_rt and

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-18 Thread Benoit Belley via Phabricator via cfe-commits
belleyb added a comment. @chh I had a chance to try out your proposed changes. It's not causing us any trouble. In fact, __gcov_flush() is not even used at all (at least in LLVM 5.0.1).. I can recompile llvm, compiler_rt and clang and re-run all the tests with __gcov_flush commented out! No

Re: [PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-11 Thread Chih-hung Hsieh via cfe-commits
Should we have two versions of `__gcov_flush`? One version is visible outside a .so file, we use dlsym to find and call it to dump profile data of .so files, like Android. One version is hidden and must be called from another wrapper function in a .so file. An application that wants to flush .so

Re: [PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-11 Thread Xinliang David Li via cfe-commits
On Wed, Apr 11, 2018 at 11:31 AM, Chih-Hung Hsieh via Phabricator via llvm-commits wrote: > chh added a comment. > > Yes, calling `__gcov_flush` within .so files are different, > but it's a revert of https://reviews.llvm.org/D38124. > I think

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-11 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. Yes, calling `__gcov_flush` within .so files are different, but it's a revert of https://reviews.llvm.org/D38124. I think https://bugs.llvm.org/show_bug.cgi?id=27224 can be fixed by hiding only llvm_gcda_* functions, without any change to `__gcov_flush`. (1) Before

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-11 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. The behavior will change if the shared libraries are not 'dlopen'ed but loaded at program startup time. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-10 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. If we use the unit test case, call `__gcov_flush` from the main function, and dump static variables in GCDAProfiling.c, we can see that `__gcov_flush` is resolved to the same copy for func.shared, func2.shared, and main. However, when `__gcov_flush` is called from main and

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-10 Thread David Li via Phabricator via cfe-commits
davidxl added a comment. With this change, gcov_flush will resolve to the copy in the main executable for each shared library -- this is not the desired the behavior. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list

[PATCH] D45454: Make __gcov_flush visible outside a shared library

2018-04-09 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh created this revision. chh added reviewers: sylvestre.ledru, davidxl, rnk, void. __gcov_flush is not called from other functions in GCDAProfiling.c, but have been used by Android, which needs an interface function to flush profile data of multiple .so files. See