diff mbox

[google,gcc-4_8] Handle Split functions in the Function Reordering Plugin

Message ID CAAs8HmzLgS6JLdH7UUaTDdMAKm4zZ_wA8m9cuj6ZWpv4gUqe_A@mail.gmail.com
State New
Headers show

Commit Message

Sriraman Tallam Feb. 12, 2014, 12:51 a.m. UTC
On Tue, Feb 11, 2014 at 4:39 PM, Teresa Johnson <tejohnson@google.com> wrote:
> On Tue, Feb 11, 2014 at 4:32 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> On Tue, Feb 11, 2014 at 3:29 PM, Teresa Johnson <tejohnson@google.com> wrote:
>>>
>>> On Feb 11, 2014 2:37 PM, "Sriraman Tallam" <tmsriram@google.com> wrote:
>>>>
>>>> On Tue, Feb 11, 2014 at 1:32 PM, Teresa Johnson <tejohnson@google.com>
>>>> wrote:
>>>> > On Mon, Feb 10, 2014 at 7:15 PM, Sriraman Tallam <tmsriram@google.com>
>>>> > wrote:
>>>> >> Hi Teresa,
>>>> >>
>>>> >>    I have attached a patch to recognize and reorder split functions in
>>>> >> the function reordering plugin. Please review.
>>>> >>
>>>> >> Thanks
>>>> >> Sri
>>>> >
>>>> >> 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
>>>> >> +         spliandection.  */
>>>
>>>> >> +      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;
>>>> >
>>>> > Where is s->split_section->computed_weight set
>>>>
>>>> I removed this line as it is not useful. Details:
>>>>
>>>> In general, the computed_weight for sections is assigned in
>>>> set_node_type in line:
>>>>  s->computed_weight = n->computed_weight;
>>>>
>>>> The computed_weight is obtained by adding the weights of all incoming
>>>> edges. However, for the cold part of split functions, this can never
>>>> be non-zero as the split cold part is never called and so this line is
>>>> not useful.
>>>>
>>>>
>>>> >
>>>> >> +              s_comdat->max_count = s->split_section->max_count;
>>>> >> +              s_comdat = s_comdat->comdat_group;
>>>> >> +            }
>>>> >> +     }
>>>> >> +
>>>> >
>>>> > ...
>>>> >
>>>> >
>>>> >> +      /* 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))
>>>> >
>>>> > The double-negative in the above comment is a bit confusing. Can we
>>>> > make this similar to the check in the earlier split comdat case. I.e.
>>>> > something like (essentially swapping the if condition and assert):
>>>>
>>>> Changed. New patch attached.
>>>
>>> The comment is fixed but the checks stayed the same and seem out of order
>>> now. Teresa
>>
>> Ah!, sorry. Changed and patch attached.
>
> The assert in the else clause is unnecessary (since you have landed
> there after doing that same check already). It would be good to have
> asserts in both the if and else clauses on the new section_name (see
> my suggested code in the initial response).

Ok, I overlooked the code you suggested in the original response,
sorry about that. I have included those asserts you suggested in both
places where we swap the new section with the existing section.

Thanks
Sri

>
> Teresa
>
>>
>> Sri
>>
>>
>>>
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>> >
>>>> >           /* If the existing section is cold, the newly detected split
>>>> > must
>>>> >              be hot.  */
>>>> >           if (is_prefix_of (".text.unlikely", kept->full_name))
>>>> >             {
>>>> >               assert (!is_prefix_of (".text.unlikely", section_name));
>>>> >               ...
>>>> >             }
>>>> >           else
>>>> >             {
>>>> >               assert (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;
>>>> >> +     }
>>>> > ...
>>>> >
>>>> > Thanks,
>>>> > Teresa
>>>> >
>>>> >
>>>> > --
>>>> > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Comments

Teresa Johnson Feb. 12, 2014, 12:54 a.m. UTC | #1
On Tue, Feb 11, 2014 at 4:51 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> On Tue, Feb 11, 2014 at 4:39 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Tue, Feb 11, 2014 at 4:32 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> On Tue, Feb 11, 2014 at 3:29 PM, Teresa Johnson <tejohnson@google.com> wrote:
>>>>
>>>> On Feb 11, 2014 2:37 PM, "Sriraman Tallam" <tmsriram@google.com> wrote:
>>>>>
>>>>> On Tue, Feb 11, 2014 at 1:32 PM, Teresa Johnson <tejohnson@google.com>
>>>>> wrote:
>>>>> > On Mon, Feb 10, 2014 at 7:15 PM, Sriraman Tallam <tmsriram@google.com>
>>>>> > wrote:
>>>>> >> Hi Teresa,
>>>>> >>
>>>>> >>    I have attached a patch to recognize and reorder split functions in
>>>>> >> the function reordering plugin. Please review.
>>>>> >>
>>>>> >> Thanks
>>>>> >> Sri
>>>>> >
>>>>> >> 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
>>>>> >> +         spliandection.  */
>>>>
>>>>> >> +      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;
>>>>> >
>>>>> > Where is s->split_section->computed_weight set
>>>>>
>>>>> I removed this line as it is not useful. Details:
>>>>>
>>>>> In general, the computed_weight for sections is assigned in
>>>>> set_node_type in line:
>>>>>  s->computed_weight = n->computed_weight;
>>>>>
>>>>> The computed_weight is obtained by adding the weights of all incoming
>>>>> edges. However, for the cold part of split functions, this can never
>>>>> be non-zero as the split cold part is never called and so this line is
>>>>> not useful.
>>>>>
>>>>>
>>>>> >
>>>>> >> +              s_comdat->max_count = s->split_section->max_count;
>>>>> >> +              s_comdat = s_comdat->comdat_group;
>>>>> >> +            }
>>>>> >> +     }
>>>>> >> +
>>>>> >
>>>>> > ...
>>>>> >
>>>>> >
>>>>> >> +      /* 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))
>>>>> >
>>>>> > The double-negative in the above comment is a bit confusing. Can we
>>>>> > make this similar to the check in the earlier split comdat case. I.e.
>>>>> > something like (essentially swapping the if condition and assert):
>>>>>
>>>>> Changed. New patch attached.
>>>>
>>>> The comment is fixed but the checks stayed the same and seem out of order
>>>> now. Teresa
>>>
>>> Ah!, sorry. Changed and patch attached.
>>
>> The assert in the else clause is unnecessary (since you have landed
>> there after doing that same check already). It would be good to have
>> asserts in both the if and else clauses on the new section_name (see
>> my suggested code in the initial response).
>
> Ok, I overlooked the code you suggested in the original response,
> sorry about that. I have included those asserts you suggested in both
> places where we swap the new section with the existing section.

Ok, thanks! Looks good.
Teresa

>
> Thanks
> Sri
>
>>
>> Teresa
>>
>>>
>>> Sri
>>>
>>>
>>>>
>>>>>
>>>>> Thanks
>>>>> Sri
>>>>>
>>>>> >
>>>>> >           /* If the existing section is cold, the newly detected split
>>>>> > must
>>>>> >              be hot.  */
>>>>> >           if (is_prefix_of (".text.unlikely", kept->full_name))
>>>>> >             {
>>>>> >               assert (!is_prefix_of (".text.unlikely", section_name));
>>>>> >               ...
>>>>> >             }
>>>>> >           else
>>>>> >             {
>>>>> >               assert (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;
>>>>> >> +     }
>>>>> > ...
>>>>> >
>>>>> > Thanks,
>>>>> > Teresa
>>>>> >
>>>>> >
>>>>> > --
>>>>> > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
diff mbox

Patch

Index: gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_8.C
===================================================================
--- gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_8.C	(revision 207700)
+++ gcc/testsuite/g++.dg/tree-prof/func_reorder_gold_plugin_8.C	(working copy)
@@ -15,5 +15,5 @@  int main()
 }
 
 /* { dg-final-use { scan-file-not linker.dump "Callgraph group" } }  */
-/* { dg-final-use { scan-file linker.dump "=== Unlikely sections start ===\n.*\.text\.hot\._Z3foov.* entry count = 1 computed = 1 max count = 1\n.*=== Unlikely sections end ===" } }  */
-/* { dg-final-use { remove-build-file "linker.dump" } }  */
+/* { dg-final-use { scan-file linker.dump "=== Unlikely sections start ===\n.*\.text\.hot\._Z3foov.* entry count = 1 computed = 1 max count = 1 split = 0\n.*=== Unlikely sections end ===" } }  */
+/*  dg-final-use { remove-build-file "linker.dump" } }  */
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" } } */
Index: function_reordering_plugin/callgraph.c
===================================================================
--- function_reordering_plugin/callgraph.c	(revision 207700)
+++ function_reordering_plugin/callgraph.c	(working copy)
@@ -550,6 +550,27 @@  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)
+            {
+	      /* Set the different weights for comdat candidates.  No need to se
+		 computed_weight as it is zero for split sections.  A split cold
+		 section is never called, it is only jumped into from the parent
+		 section.  */
+              s_comdat->weight = s->split_section->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 +720,131 @@  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))
+	    {
+	      assert (!is_prefix_of (".text.unlikely", section_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
+	    {
+	      assert (is_prefix_of (".text.unlikely", section_name));
+	      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;
+	  /* If the existing section is cold, the newly detected split must 
+	     be hot.  */
+          if (is_prefix_of (".text.unlikely", kept->full_name))
+	    {
+	      assert (!is_prefix_of (".text.unlikely", section_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", section_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 +1138,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: function_reordering_plugin/callgraph.h
===================================================================
--- function_reordering_plugin/callgraph.h	(revision 207700)
+++ 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;
 }