[GOOGLE] Do not partition the cold/hot sections of a function with "section" attribute
Teresa Johnson
tejohnson@google.com
Tue Aug 12 16:24:00 GMT 2014
On Mon, Aug 11, 2014 at 1:44 PM, Yi Yang <ahyangyi@google.com> wrote:
> I ported this to trunk.
>
> Shall I commit this patch to the Google 4_8/4_9 branches first?
Ok with me.
Teresa
>
> 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
--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
More information about the Gcc-patches
mailing list