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 <ahyan...@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 <ahyan...@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 <tejohn...@google.com> wrote:
>>> On Fri, Aug 8, 2014 at 3:22 PM, Yi Yang <ahyan...@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 <de...@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 <ahyan...@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 <ahyan...@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 | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
