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 <tejohn...@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 <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