| Message ID | a3ed605cdecc22d1c5ff91d90b4856574fea2139.1465577682.git.andrew.burgess@embecosm.com |
|---|---|
| State | New |
| Headers | show |
On 06/10/2016 10:56 AM, Andrew Burgess wrote: > The global flag `user_defined_section_attribute' is set while parsing C > code when the section attribute is encountered. The flag is set when > anything has the section attribute applied to it, functions or data. > > The only place this global was used was within the gate function for > partitioning blocks (pass_partition_blocks::gate), however, the > partitioning is done during compilation, while the flag is set earlier, > during the parsing. The flag is then cleared again during the final > compilation pass. > > The result is that if any function or data has a section attribute then > the flag will be set to true during the file parse pass. The first > compiled function will then skip the partition-blocks pass, and the flag > will be set back to false during the final-pass on the first function. > After then, the flag is never set to true again. > > The guarding of the partition-blocks pass does not appear to be > necessary, given that applying a section attribute correctly > overrides the hot/cold section partitioning (this is taken care if in > varasm.c). > > gcc/ChangeLog: > > * gcc/bb-reorder.c: Remove 'toplev.h' include. > (pass_partition_blocks::gate): No longer check > user_defined_section_attribute. > * gcc/c-family/c-common.c (handle_section_attribute): No longer > set user_defined_section_attribute. > * gcc/final.c (rest_of_handle_final): Likewise. > * gcc/toplev.c: Remove definition of user_defined_section_attribute. > * gcc/toplev.h: Remove declaration of > user_defined_section_attribute. user_defined_section_attribute was introduced as part of the hot/cold partitioning changes. https://gcc.gnu.org/ml/gcc-patches/2004-07/msg01545.html What's supposed to happen is hot/cold partitioning is supposed to be turned off for the function which has the a user defined section attribute. So proper behaviour is to set the flag to true when the attribute is parsed and turn it off when we're finished with the current function. The gate for hot/cold partitioning should check the value of the flag and avoid hot/cold partitioning when the flag is true. So AFAICT everything is working as it should. Keep in mind that multiple functions might have user defined section attributes. So what might be better to do here is introduce a test to verify proper behavior. Jeff
On Tue, Jun 21, 2016 at 08:55:15PM -0600, Jeff Law wrote: > user_defined_section_attribute was introduced as part of the hot/cold > partitioning changes. > > https://gcc.gnu.org/ml/gcc-patches/2004-07/msg01545.html > > > What's supposed to happen is hot/cold partitioning is supposed to be turned > off for the function which has the a user defined section attribute. The flag has been added before we had -funit-at-a-time, and IMNSHO it couldn't work properly even when it has been introduced, because if you had void foo (void) __attribute__((section ("..."))); void bar (void) { ... } void foo (void) { ... } then it would mistakenly apply to bar rather than foo. So, either we can reconstruct whether the current function decl has user section attribute, then perhaps during expansion we should set the flag to that or use such a test directly in the gate (e.g. would lookup_attribute ("section", DECL_ATTRIBUTES (current_function_decl)) DTRT?) and drop the flag, or we need some way to preserve that information per function. Jakub
diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c index 5fb60bd..04874da 100644 --- a/gcc/bb-reorder.c +++ b/gcc/bb-reorder.c @@ -106,7 +106,6 @@ #include "output.h" #include "expr.h" #include "params.h" -#include "toplev.h" /* user_defined_section_attribute */ #include "tree-pass.h" #include "cfgrtl.h" #include "cfganal.h" @@ -2889,8 +2888,7 @@ pass_partition_blocks::gate (function *fun) /* See gate_handle_reorder_blocks. We should not partition if we are going to omit the reordering. */ && optimize_function_for_speed_p (fun) - && !DECL_COMDAT_GROUP (current_function_decl) - && !user_defined_section_attribute); + && !DECL_COMDAT_GROUP (current_function_decl)); } unsigned diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 94005ff..66add18 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -7617,8 +7617,6 @@ handle_section_attribute (tree *node, tree ARG_UNUSED (name), tree args, goto fail; } - user_defined_section_attribute = true; - if (!VAR_OR_FUNCTION_DECL_P (decl)) { error ("section attribute not allowed for %q+D", *node); diff --git a/gcc/final.c b/gcc/final.c index 5b04311..256ce34 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -4454,8 +4454,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 a/gcc/toplev.c b/gcc/toplev.c index 543b8a3..868eecf 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -148,11 +148,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 a/gcc/toplev.h b/gcc/toplev.h index 06923cf..f62a172 100644 --- a/gcc/toplev.h +++ b/gcc/toplev.h @@ -74,11 +74,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;