Hi Teresa,
I have attached a patch to recognize and reorder split functions in
the function reordering plugin. Please review.
Thanks
Sri
This enhances the linker plugin to reorder functions that are split when
using -freorder-blocks-and-partition.
Index: function_reordering_plugin/callgraph.h
===================================================================
--- function_reordering_plugin/callgraph.h (revision 207671)
+++ function_reordering_plugin/callgraph.h (working copy)
@@ -236,6 +236,8 @@ typedef struct section_id_
is comdat hot and kept, pointer to the kept cold split
section. */
struct section_id_ *split_section;
+ /* If this is the cold part of a split section. */
+ char is_split_cold_section;
/* Check if this section has been considered for output. */
char processed;
} Section_id;
@@ -260,6 +262,7 @@ make_section_id (char *name, char *full_name,
s->computed_weight = 0;
s->max_count = 0;
s->split_section = NULL;
+ s->is_split_cold_section = 0;
return s;
}
Index: function_reordering_plugin/callgraph.c
===================================================================
--- function_reordering_plugin/callgraph.c (revision 207671)
+++ function_reordering_plugin/callgraph.c (working copy)
@@ -550,6 +550,25 @@ static void set_node_type (Node *n)
s->computed_weight = n->computed_weight;
s->max_count = n->max_count;
+ /* If s is split into a cold section, assign the split weight to the
+ max count of the split section. Use this also for the weight of the
+ split section. */
+ if (s->split_section)
+ {
+ s->split_section->max_count = s->split_section->weight =
n->split_weight;
+ /* If split_section is comdat, update all the comdat
+ candidates for weight. */
+ s_comdat = s->split_section->comdat_group;
+ while (s_comdat != NULL)
+ {
+ s_comdat->weight = s->split_section->weight;
+ s_comdat->computed_weight
+ = s->split_section->computed_weight;
+ s_comdat->max_count = s->split_section->max_count;
+ s_comdat = s_comdat->comdat_group;
+ }
+ }
+
/* If s is comdat, update all the comdat candidates for weight. */
s_comdat = s->comdat_group;
while (s_comdat != NULL)
@@ -699,26 +718,129 @@ map_section_name_to_index (char *section_name, voi
}
else
{
- /* The function already exists, it must be a COMDAT. Only one section
- in the comdat group will be kept, we don't know which. Chain all the
- comdat sections in the same comdat group to be emitted together later.
- Keep one section as representative (kept) and update its section_type
- to be equal to the type of the highest priority section in the
- group. */
+ /* Handle function splitting here. With function splitting, the split
+ function sections have the same name and they are in the same module.
+ Here, we only care about the section that is marked with prefix
+ like ".text.hot". The other section is cold. The plugin should not
+ be adding this cold section to the section_map. In get_layout it will
+ later be picked up when processing the non-callgraph sections and it
+ will laid out appropriately. */
Section_id *kept = (Section_id *)(*slot);
Section_id *section = make_section_id (function_name, section_name,
section_type, handle, shndx);
+ int is_split_function = 0;
+ Section_id *split_comdat = NULL;
+ /* Check if this is a split function. The modules are the same means this
+ is not comdat and we assume it is split. It can be split and comdat
+ too, in which case we have to search the comdat list of kept. */
+ if (kept->handle == handle)
+ is_split_function = 1;
+ else if (kept->comdat_group != NULL)
+ {
+ split_comdat = kept;
+ do
+ {
+ if (split_comdat->comdat_group->handle == handle)
+ break;
+ split_comdat = split_comdat->comdat_group;
+ }
+ while (split_comdat->comdat_group != NULL);
+ }
- /* Two comdats in the same group can have different priorities. This
- ensures that the "kept" comdat section has the priority of the higest
- section in that comdat group. This is necessary because the plugin
- does not know which section will be kept. */
- if (section_priority[kept->section_type]
- > section_priority[section_type])
- kept->section_type = section_type;
+ /* It is split and it is comdat. */
+ if (split_comdat != NULL
+ && split_comdat->comdat_group != NULL)
+ {
+ /* The comdat_section that is split. */
+ Section_id *comdat_section = split_comdat->comdat_group;
+ Section_id *cold_section = NULL;
+ /* If the existing section is cold, the newly detected split must
+ be hot. */
+ if (is_prefix_of (".text.unlikely", comdat_section->full_name))
+ {
+ cold_section = comdat_section;
+ /* Replace the comdat_section in the kept section list with the
+ new section. */
+ split_comdat->comdat_group = section;
+ section->comdat_group = comdat_section->comdat_group;
+ comdat_section->comdat_group = NULL;
+ }
+ else
+ {
+ cold_section = section;
+ }
+ assert (cold_section != NULL && cold_section->comdat_group == NULL);
+ cold_section->is_split_cold_section = 1;
+ /* The cold section must be added to the unlikely chain of comdat
+ groups. */
+ if (kept->split_section == NULL)
+ {
+ /* This happens if no comdat function in this group so far has
+ been split. */
+ kept->split_section = cold_section;
+ }
+ else
+ {
+ /* Add the cold_section to the unlikely chain of comdats. */
+ cold_section->comdat_group = kept->split_section->comdat_group;
+ kept->split_section->comdat_group = cold_section;
+ }
+ }
+ /* It is split and it is not comdat. */
+ else if (is_split_function)
+ {
+ Section_id *cold_section = NULL;
+ /* Function splitting means that the "hot" part is really the
+ relevant section and the other section is unlikely executed and
+ should not be part of the callgraph. */
- section->comdat_group = kept->comdat_group;
- kept->comdat_group = section;
+ /* Store the new section in the section list. */
+ section->next = first_section;
+ first_section = section;
+ /* The right section is not in the section_map if ".text.unlikely" is
+ not the new section. */
+ if (!is_prefix_of (".text.unlikely", section_name))
+ {
+ assert (is_prefix_of (".text.unlikely", kept->full_name));
+ /* The kept section was the unlikely section. Change the section
+ in section_map to be the new section which is the hot one. */
+ *slot = section;
+ /* Record the split cold section in the hot section. */
+ section->split_section = kept;
+ /* Comdats and function splitting are already handled. */
+ assert (kept->comdat_group == NULL);
+ cold_section = kept;
+ }
+ else
+ {
+ /* Record the split cold section in the hot section. */
+ assert (!is_prefix_of (".text.unlikely", kept->full_name));
+ kept->split_section = section;
+ cold_section = section;
+ }
+ assert (cold_section != NULL && cold_section->comdat_group == NULL);
+ cold_section->is_split_cold_section = 1;
+ }
+ else
+ {
+ /* The function already exists, it must be a COMDAT. Only one
section
+ in the comdat group will be kept, we don't know which. Chain all
+ the comdat sections in the same comdat group to be emitted
+ together later. Keep one section as representative (kept) and
+ update its section_type to be equal to the type of the highest
+ priority section in the group. */
+
+ /* Two comdats in the same group can have different priorities. This
+ ensures that the "kept" comdat section has the priority of the
+ highest section in that comdat group. This is necessary because
+ the plugin does not know which section will be kept. */
+ if (section_priority[kept->section_type]
+ > section_priority[section_type])
+ kept->section_type = section_type;
+
+ section->comdat_group = kept->comdat_group;
+ kept->comdat_group = section;
+ }
}
}
@@ -1012,8 +1134,10 @@ get_layout (FILE *fp, void*** handles,
if (fp != NULL)
{
fprintf (fp, "%s entry count = %llu computed = %llu "
- "max count = %llu\n", s_it->full_name, s_it->weight,
- s_it->computed_weight, s_it->max_count);
+ "max count = %llu split = %d\n",
+ s_it->full_name, s_it->weight,
+ s_it->computed_weight, s_it->max_count,
+ s_it->is_split_cold_section);
}
s_it = s_it->group;
}
Index:
gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_split_functions_1.C
===================================================================
--- gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_split_functions_1.C
(revision 0)
+++ gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_split_functions_1.C
(revision 0)
@@ -0,0 +1,58 @@
+/* Check if the gold function reordering plugin reorders split functions.
+ Check if foo is split and the cold section of foo is not next to its hot
+ section*/
+/* { dg-require-section-exclude "" } */
+/* { dg-require-linker-function-reordering-plugin "" } */
+/* { dg-require-effective-target freorder } */
+/* { dg-options "-O2 -freorder-functions=callgraph -ffunction-sections
-freorder-blocks-and-partition --save-temps -Wl,-plugin-opt,file=linker.dump" }
*/
+
+
+#define SIZE 10000
+
+const char *sarr[SIZE];
+const char *buf_hot;
+const char *buf_cold;
+
+__attribute__ ((noinline))
+int bar (int *arg)
+{
+ (*arg)++;
+ return 0;
+}
+
+__attribute__((noinline))
+void
+foo (int path)
+{
+ int i;
+ bar (&path);
+ if (path)
+ {
+ for (i = 0; i < SIZE; i++)
+ sarr[i] = buf_hot;
+ }
+ else
+ {
+ for (i = 0; i < SIZE; i++)
+ sarr[i] = buf_cold;
+ }
+}
+
+int
+main (int argc, char *argv[])
+{
+ buf_hot = "hello";
+ buf_cold = "world";
+ foo (argc);
+ return 0;
+}
+
+/* { dg-final-use { scan-assembler "\.string \"ColdWeight 0\"" } } */
+/* { dg-final-use { scan-assembler "\.section.*\.text\.hot\._Z3fooi" } } */
+/* { dg-final-use { scan-assembler "\.section.*\.text\.unlikely\._Z3fooi" } }
*/
+/* { dg-final-use { cleanup-saved-temps } } */
+/* { dg-final-use { scan-file linker.dump "Callgraph group : _Z3barPi _Z3fooi
main\n" } } */
+/* { dg-final-use { scan-file linker.dump "\.text\.unlikely\._Z3fooi .* split
= 1" } } */
+/* { dg-final-use { scan-file linker.dump
"\.text\.unlikely\._Z3fooi\[^\n\]*\n\.text\.unlikely\._Z3barPi\[^\n\]*\n" } }
*/
+/* { dg-final-use { scan-file linker.dump
"\.text\._Z3barPi\[^\n\]*\n\.text\.hot\._Z3fooi" } } */
+/* { dg-final-use { remove-build-file "linker.dump" } } */
