Re: [GOOGLE] Do not partition the cold/hot sections of a function with "section" attribute

[Yi Yang <ahyangyi at google dot com>]

I ported this to trunk.

Shall I commit this patch to the Google 4_8/4_9 branches first?

On Mon, Aug 11, 2014 at 12:46 PM, Teresa Johnson <tejohnson@google.com> wrote:
> Ok, thanks. This seems reasonable. Can you send the patch to trunk as well?
> Teresa
>
> On Mon, Aug 11, 2014 at 12:35 PM, Yi Yang <ahyangyi@google.com> wrote:
>> Patch v2
>>
>> ..
>>
>> diff --git gcc/bb-reorder.c gcc/bb-reorder.c
>> index fa6f62f..a1b3e65 100644
>> --- gcc/bb-reorder.c
>> +++ gcc/bb-reorder.c
>> @@ -95,7 +95,6 @@
>>  #include "expr.h"
>>  #include "params.h"
>>  #include "diagnostic-core.h"
>> -#include "toplev.h" /* user_defined_section_attribute */
>>  #include "tree-pass.h"
>>  #include "df.h"
>>  #include "bb-reorder.h"
>> @@ -2555,7 +2554,7 @@ gate_handle_partition_blocks (void)
>>       we are going to omit the reordering.  */
>>    && optimize_function_for_speed_p (cfun)
>>    && !DECL_ONE_ONLY (current_function_decl)
>> -  && !user_defined_section_attribute);
>> +  && !DECL_SECTION_NAME (current_function_decl));
>>  }
>>
>>  /* This function is the main 'entrance' for the optimization that
>> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
>> index 65c25bf..9923928 100644
>> --- gcc/c-family/c-common.c
>> +++ gcc/c-family/c-common.c
>> @@ -7378,8 +7378,6 @@ handle_section_attribute (tree *node, tree
>> ARG_UNUSED (name), tree args,
>>
>>    if (targetm_common.have_named_sections)
>>      {
>> -      user_defined_section_attribute = true;
>> -
>>        if ((TREE_CODE (decl) == FUNCTION_DECL
>>     || TREE_CODE (decl) == VAR_DECL)
>>    && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
>> diff --git gcc/final.c gcc/final.c
>> index 9af0b2b..38c90b2 100644
>> --- gcc/final.c
>> +++ gcc/final.c
>> @@ -4501,8 +4501,6 @@ rest_of_handle_final (void)
>>
>>    assemble_end_function (current_function_decl, fnname);
>>
>> -  user_defined_section_attribute = false;
>> -
>>    /* Free up reg info memory.  */
>>    free_reg_info ();
>>
>> diff --git gcc/toplev.c gcc/toplev.c
>> index 9b8d313..4c8c965 100644
>> --- gcc/toplev.c
>> +++ gcc/toplev.c
>> @@ -153,11 +153,6 @@ HOST_WIDE_INT random_seed;
>>     the support provided depends on the backend.  */
>>  rtx stack_limit_rtx;
>>
>> -/* True if the user has tagged the function with the 'section'
>> -   attribute.  */
>> -
>> -bool user_defined_section_attribute = false;
>> -
>>  struct target_flag_state default_target_flag_state;
>>  #if SWITCHABLE_TARGET
>>  struct target_flag_state *this_target_flag_state = &default_target_flag_state;
>> diff --git gcc/toplev.h gcc/toplev.h
>> index 0290be3..65e38e7 100644
>> --- gcc/toplev.h
>> +++ gcc/toplev.h
>> @@ -53,11 +53,6 @@ extern void target_reinit (void);
>>  /* A unique local time stamp, might be zero if none is available.  */
>>  extern unsigned local_tick;
>>
>> -/* True if the user has tagged the function with the 'section'
>> -   attribute.  */
>> -
>> -extern bool user_defined_section_attribute;
>> -
>>  /* See toplev.c.  */
>>  extern int flag_rerun_cse_after_global_opts;
>>
>> --
>>
>> On Mon, Aug 11, 2014 at 12:22 PM, Yi Yang <ahyangyi@google.com> wrote:
>>> Thanks for pointing out this!
>>>
>>> It seems to me that this user_defined_section_attribute variable is
>>> useless now and should be removed. It is intended to work in this way:
>>>
>>> for each function {
>>>    parse into tree (setting user_defined_section_attribute)
>>>    do tree passes
>>>    do RTL passes (using user_defined_section_attribute)
>>>    resetting (user_defined_section_attribute = false)
>>> }
>>>
>>> But now GCC works this way:
>>>
>>> for each function {
>>>    parse into tree (setting user_defined_section_attribute)
>>> }
>>> do IPA passes
>>> for each function {
>>>    do tree passes
>>>    do RTL passes (using user_defined_section_attribute)
>>>    resetting (user_defined_section_attribute = false)
>>> }
>>>
>>> Therefore the first function will use the actual
>>> user_defined_section_attribute of the last function, and all other
>>> functions will always see user_defined_section_attribute=0.
>>>
>>> I suggest removing user_defined_section_attribute and simply check
>>> !DECL_SECTION_NAME (current_function_decl).
>>>
>>> On Mon, Aug 11, 2014 at 8:00 AM, Teresa Johnson <tejohnson@google.com> wrote:
>>>> On Fri, Aug 8, 2014 at 3:22 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>>> Friendly ping.
>>>>
>>>> Sorry, was OOO.
>>>>
>>>> The solution of preventing splitting for named sections is good - but
>>>> it looks like there is already code that should prevent this. See the
>>>> user_defined_section_attribute check here - why is that not set? Looks
>>>> like it should be set in handle_section_attribute() in
>>>> c-family/c-common.c.
>>>>
>>>> Teresa
>>>>
>>>>>
>>>>>
>>>>> On Wed, Aug 6, 2014 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>>>>>>
>>>>>> OK for google-4_8 and google-4_9. David and Teresa may have further
>>>>>> comments.
>>>>>>
>>>>>> Dehao
>>>>>>
>>>>>> On Wed, Aug 6, 2014 at 3:36 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>>>> > This currently puts split sections together again in the specified
>>>>>> > section and breaks DWARF output. This patch disables the partitioning
>>>>>> > for such functions.
>>>>>> >
>>>>>> > --
>>>>>> >
>>>>>> > 2014-08-06  Yi Yang  <ahyangyi@google.com>
>>>>>> >
>>>>>> > gcc:
>>>>>> >     * bb-reorder.c (gate_handle_partition_blocks): Add a check for
>>>>>> > "section"
>>>>>> > attribute.
>>>>>> >
>>>>>> > diff --git gcc/bb-reorder.c gcc/bb-reorder.c
>>>>>> > index fa6f62f..09449c6 100644
>>>>>> > --- gcc/bb-reorder.c
>>>>>> > +++ gcc/bb-reorder.c
>>>>>> > @@ -2555,6 +2555,7 @@ gate_handle_partition_blocks (void)
>>>>>> >       we are going to omit the reordering.  */
>>>>>> >    && optimize_function_for_speed_p (cfun)
>>>>>> >    && !DECL_ONE_ONLY (current_function_decl)
>>>>>> > +  && !DECL_SECTION_NAME (current_function_decl)
>>>>>> >    && !user_defined_section_attribute);
>>>>>> >  }
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413