This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Detect whether target can use -fprofile-update=atomic
- From: David Edelsohn <dje dot gcc at gmail dot com>
- To: Martin Liška <mliska at suse dot cz>, Nathan Sidwell <nathan at acm dot org>, Jakub Jelinek <jakub at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jan Hubicka <jh at suse dot cz>, Andreas Schwab <schwab at suse dot de>, Richard Biener <richard dot guenther at gmail dot com>
- Date: Tue, 6 Sep 2016 06:57:05 -0400
- Subject: Re: [PATCH] Detect whether target can use -fprofile-update=atomic
- Authentication-results: sourceware.org; auth=none
- References: <cover.1470041316.git.mliska@suse.cz> <53c4f874443d63de99393a9816041ba75f1d605e.1470041316.git.mliska@suse.cz> <95628fa2-8719-bb82-d1ab-ca8d46355020@acm.org> <0ec1ad3f-0fc7-507e-878c-907adae1c011@suse.cz> <ceb330f5-707e-79f8-a49a-2eafbce1dad8@acm.org> <0e248978-3717-1d78-b3b1-9fc60ded5c8e@suse.cz> <882f0f98-9b33-a764-833b-ca61796f3143@acm.org> <87f2bc4f-c4df-eadd-aec6-a937ed0ccaba@acm.org> <1253ac69-3301-f185-e43a-a34cadf8f51e@suse.cz> <67fda6d2-9b3e-a0d1-effc-34e1115030b2@acm.org> <b62f3336-b332-048f-ed2f-9392824f0b84@suse.cz> <1ff3cc75-7cee-79f3-395b-ef7a4d286a3d@acm.org> <04a05835-4666-4d7d-c1a9-d4bcc4ea924a@suse.cz> <19c4a81d-6ecd-8c6e-b641-e257c1959baf@suse.cz> <43f60306-d6d6-47e4-758c-a806f661d6a0@acm.org> <07cadca7-913a-acad-2f49-482b19e97a72@suse.cz>
On Tue, Sep 6, 2016 at 6:45 AM, Martin Liška <mliska@suse.cz> wrote:
>>> Proper fix contains of 2 parts:
>>> a) compiler emission must verify that -fprofile-update=atomic is doable for a given target; it's done
>>> via a new function can_generate_atomic_builtin
>>> b) libgcc must detect whether __atomic_fetch_add_x can be expanded on the target; that requires configure
>>> support and if the target is not capable to expand these, we must conditionally remove all gcov_.*profiler_atomic
>>> functions from libgcov.a.
>>
>> I'm fine with the coverage-pecific changes, but the new hooks etc are not something I can approve.
>>
>> gcc/ChangeLog:
>>
>> 2016-08-12 Martin Liska <mliska@suse.cz>
>>
>> * optabs.c (can_generate_atomic_builtin): New function.
>> * optabs.h (can_generate_atomic_builtin): Declare the function.
>> Need GWM or similar review
>>
>> * tree-profile.c (tree_profiling): Detect whether target can use
>> -fprofile-update=atomic.
>> ok
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-08-12 Martin Liska <mliska@suse.cz>
>>
>> * gcc.dg/profile-update-warning.c: New test.
>> ok
>>
>> libgcc/ChangeLog:
>>
>> 2016-08-16 Martin Liska <mliska@suse.cz>
>>
>> * acinclude.m4: New file.
>> * config.in: New macro defines.
>> * configure: Regenerated.
>> * configure.ac: Detect atomic operations.
>> need GWM or similar review
>>
>> * libgcov-profiler.c: Detect GCOV_SUPPORTS_ATOMIC and
>> conditionaly enable/disable *_atomic functions.
>> OK.
>>
>> nathan
>
> Hi Nathan.
>
> Thanks for review, I'm CCing Jakub and Richard for the review.
> The patch should fix very similar issue spotted on AIX target by David.
What about Jakub's comment in the PR?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77378#c6
The proposed patch seems wrong or at least incomplete. The recent
change is imposing a 64 bit DImode counter when producing 32 bit code.
PowerPC does support 64 bit atomic operations in 32 bit mode.
Was there a design decision that profile counters always should be 64
bits? Either 32 bit targets won't support multi-threaded profiling or
32 bit targets can overflow the counter sooner. Which is worse?
Which is more likely?
Thanks, David