public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Bernd Schmidt <bschmidt@redhat.com>
Cc: gcc-patches@gcc.gnu.org, Jeff Law <law@redhat.com>,
	Jakub Jelinek <jakub@redhat.com>
Subject: Re: Ping: Re: [PATCH 1/2] gcc: Remove unneeded global flag.
Date: Wed, 16 Nov 2016 20:09:00 -0000	[thread overview]
Message-ID: <20161116200930.GG5975@embecosm.com> (raw)
In-Reply-To: <03bef940-2b86-af7d-d2d2-b96b8283596f@redhat.com>

* Bernd Schmidt <bschmidt@redhat.com> [2016-11-03 13:01:32 +0100]:

> On 09/14/2016 03:00 PM, Andrew Burgess wrote:
> > In an attempt to get this patch merged (as I still think that its
> > correct) I've investigated, and documented a little more about how I
> > think things currently work.  I'm sure most people reading this will
> > already know this, but hopefully, if my understanding is wrong someone
> > can point it out.
> > gcc/ChangeLog:
> > 
> > 	* gcc/bb-reorder.c: Remove 'toplev.h' include.
> > 	(pass_partition_blocks::gate): No longer check
> > 	user_defined_section_attribute, instead check the function decl
> > 	for a section attribute.
> > 	* gcc/c-family/c-common.c (handle_section_attribute): No longer
> > 	set user_defined_section_attribute.
> > 	* gcc/final.c (rest_of_handle_final): Likewise.
> > 	* gcc/toplev.c: Remove definition of user_defined_section_attribute.
> > 	* gcc/toplev.h: Remove declaration of
> > 	user_defined_section_attribute.
> > 
> > gcc/testsuiteChangeLog:
> > 
> > 	* gcc.dg/tree-prof/section-attr-1.c: New file.
> > 	* gcc.dg/tree-prof/section-attr-2.c: New file.
> > 	* gcc.dg/tree-prof/section-attr-3.c: New file.
> 
> I think the explanation is perfectly reasonable and the patch looks good,
> except:
> 
> > +__attribute__((noinline))
> 
> Add noclone to all of these as well.

Thanks.  Considering Jeff said, I'm thinking about it, and you've said
yes, and given Jeff's not got back, I'm considering this patch
approved (with the fix you suggest).

My only remaining concern is the new tests, I've tried to restrict
them to targets that I suspect they'll pass on with:

    /* { dg-final-use { scan-assembler "\.section\[\t \]*\.text\.unlikely\[\\n\\r\]+\[\t \]*\.size\[\t \]*foo\.cold\.0" { target *-*-linux* *-*-gnu* } } } */

but I'm still nervous that I'm going to introduce test failures.  Is
there any advice / guidance I should follow before I commit, or are
folk pretty relaxed so long as I've made a reasonable effort?

Thanks,
Andrew

  reply	other threads:[~2016-11-16 20:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-10 16:56 [PATCH 0/2] Remove user_defined_section_attribute global Andrew Burgess
2016-06-10 16:56 ` [PATCH 1/2] gcc: Remove unneeded global flag Andrew Burgess
2016-06-22  2:55   ` Jeff Law
2016-06-22  6:02     ` Jakub Jelinek
2016-06-29 19:33     ` Andrew Burgess
2016-09-14 13:05       ` Ping: " Andrew Burgess
2016-09-14 13:08         ` Jakub Jelinek
2016-09-15 14:30           ` Andrew Burgess
2016-10-28 15:58             ` Jeff Law
2016-10-28 16:15               ` Andrew Burgess
2016-11-03 12:01         ` Bernd Schmidt
2016-11-16 20:09           ` Andrew Burgess [this message]
2016-11-16 21:00             ` Mike Stump
2016-11-16 22:12               ` Andrew Burgess
2016-11-17 17:59                 ` Jeff Law
2016-11-18 12:22                 ` Christophe Lyon
2016-11-19 21:59                   ` Andrew Burgess
2016-11-20 17:27                     ` Mike Stump
2016-11-21 12:47                       ` Christophe Lyon
2016-11-24 21:40                         ` Andrew Burgess
2016-11-28 22:09                           ` Jeff Law
2016-11-29 14:03                             ` Andrew Burgess
2016-11-29 17:36                               ` Jeff Law
2016-11-30 11:40                                 ` Andrew Burgess
2016-06-10 16:57 ` [PATCH 2/2] gcc: Update comment in bb-reorder.c Andrew Burgess
2016-06-22  2:59   ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161116200930.GG5975@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=law@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).