Re: Ping: Re: [PATCH 1/2] gcc: Remove unneeded global flag.

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Jeff Law <law@redhat.com> [2016-10-28 09:58:14 -0600]:

> On 09/15/2016 08:24 AM, Andrew Burgess wrote:
> > * Jakub Jelinek <jakub@redhat.com> [2016-09-14 15:07:56 +0200]:
> > 
> > > On Wed, Sep 14, 2016 at 02:00:48PM +0100, 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.
> > > 
> > > I wonder if user_defined_section_attribute instead shouldn't be moved
> > > into struct function and be handled as a per-function flag then.
> > 
> > That would certainly solve the problem I'm trying to address.  But I
> > wonder, how is that different to looking for a section attribute on
> > the function DECL?
> I'm not sure it is significantly different.  It seems like it's just an
> implementation detail.  I'd err on the side of putting this into the struct
> function rather than on the DECL node simply to keep the size of DECL nodes
> from increasing.  Even if you can find suitable free flag bits, those can
> likely be better used for other purposes.

I didn't add anything to the DECL, the information we need is already
there.  The relevant chunk of the patch is:

@@ -2890,7 +2889,7 @@ pass_partition_blocks::gate (function *fun)
             we are going to omit the reordering.  */
          && optimize_function_for_speed_p (fun)
          && !DECL_COMDAT_GROUP (current_function_decl)
-	  && !user_defined_section_attribute);
+	  && !lookup_attribute ("section", DECL_ATTRIBUTES (fun->decl)));
 unsigned

I have not made any changes to add anything new to the DECL.  I guess
an argument _could_ be made that looking up an attribute is too
expensive to be used in a pass::gate function (I haven't looked into
how expensive it is) but I figured that initially at least it's better
to reuse the data we already have around than to add a new flag that
duplicates something we already have.

> I'm still pondering the actual patch.  It's not forgotten.

Would it help clarify things if I added some printf style tracing and
posted a trace?  This might help highlight how
USER_DEFINED_SECTION_ATTRIBUTE is set in a different phase of
compilation and so can't possibly be of any use when deciding whether
or not to perform the pass or not.

I'm still keen to see this merged, so any extra leg work I can do to
help move this forward, please let me know; I'm happy to help.

Thanks,
Andrew