[google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Sriraman Tallam]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Hi Rong,

    Please review this code. This code allows the function reordering
plugin to separate hot and cold code into different ELF segments.
This would allow optimizations like mapping the hot code alone to huge
pages.

    With this patch, by default, the plugin maps .text.unlikely
sections into a separate ELF segment.  This can be turned off with
plugin option "--segment=none".

    The include/plugin-api.h changes are a backport from trunk.

Thanks,
-Sri.

[-- Attachment #2: reordering_plugin_patch.txt --]
[-- Type: text/plain, Size: 9991 bytes --]

Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 194505)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -74,6 +74,9 @@ static ld_plugin_get_input_section_name get_input_
 static ld_plugin_get_input_section_contents get_input_section_contents = NULL;
 static ld_plugin_update_section_order update_section_order = NULL;
 static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
+static ld_plugin_allow_unique_segment_for_sections 
+    allow_unique_segment_for_sections = NULL;
+static ld_plugin_unique_segment_for_sections unique_segment_for_sections = NULL;
 
 /* The file where the final function order will be stored.
    It can be set by using the  plugin option  as --plugin-opt
@@ -86,6 +89,10 @@ static int is_api_exist = 0;
 /* The plugin does nothing when no-op is 1.  */
 static int no_op = 0;
 
+/* The plugin does not create a new segment for unlikely code if
+   no_segment is set.  */
+static int no_segment = 0;
+
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
 {
@@ -96,12 +103,14 @@ void get_filename (const char *name)
 /* Process options to plugin.  Options with prefix "group=" are special.
    They specify the type of grouping. The option "group=none" makes the
    plugin do nothing.   Options with prefix "file=" set the output file
-   where the final function order must be stored.  */
+   where the final function order must be stored.  Option "segment=none"
+   does not place the cold code in a separate ELF segment.  */
 void
 process_option (const char *name)
 {
   const char *option_group = "group=";
   const char *option_file = "file=";
+  const char *option_segment = "segment=";
 
   /* Check if option is "group="  */
   if (strncmp (name, option_group, strlen (option_group)) == 0)
@@ -120,6 +129,16 @@ process_option (const char *name)
       return;
     }
 
+  /* Check if options is "segment=none"  */
+  if (strncmp (name, option_segment, strlen (option_segment)) == 0)
+    {
+      if (strcmp (name + strlen (option_segment), "none") == 0)
+	no_segment = 1;
+      else
+	no_segment = 0;
+      return;
+    }
+
   /* Unknown option, set no_op to 1.  */
   no_op = 1;
   fprintf (stderr, "Unknown option to function reordering plugin :%s\n",
@@ -169,6 +188,13 @@ onload (struct ld_plugin_tv *tv)
 	case LDPT_ALLOW_SECTION_ORDERING:
 	  allow_section_ordering = *entry->tv_u.tv_allow_section_ordering;
 	  break;
+	case LDPT_ALLOW_UNIQUE_SEGMENT_FOR_SECTIONS:
+	  allow_unique_segment_for_sections
+	      = *entry->tv_u.tv_allow_unique_segment_for_sections;
+	  break;
+	case LDPT_UNIQUE_SEGMENT_FOR_SECTIONS:
+	  unique_segment_for_sections = *entry->tv_u.tv_unique_segment_for_sections;
+	  break;
         default:
           break;
         }
@@ -183,7 +209,9 @@ onload (struct ld_plugin_tv *tv)
       && get_input_section_name != NULL
       && get_input_section_contents != NULL
       && update_section_order != NULL
-      && allow_section_ordering != NULL)
+      && allow_section_ordering != NULL
+      && allow_unique_segment_for_sections != NULL
+      && unique_segment_for_sections != NULL)
     is_api_exist = 1;
   else
     return LDPS_OK;
@@ -216,6 +244,7 @@ claim_file_hook (const struct ld_plugin_input_file
     {
       /* Inform the linker to prepare for section reordering.  */
       (*allow_section_ordering) ();
+      (*allow_unique_segment_for_sections) ();
       is_ordering_specified = 1;
     }
 
@@ -259,6 +288,11 @@ claim_file_hook (const struct ld_plugin_input_file
 /* This function is called by the linker after all the symbols have been read.
    At this stage, it is fine to tell the linker the desired function order.  */
 
+/* These globals are set to the start and end of the unlikely function sections
+   in the section list, which can then be mapped to a separate segment.  */
+extern int unlikely_segment_start;
+extern int unlikely_segment_end;
+
 enum ld_plugin_status
 all_symbols_read_hook (void)
 {
@@ -302,7 +336,14 @@ all_symbols_read_hook (void)
       && strcmp (out_file, "stderr") != 0)
     fclose (fp);
   /* Pass the new order of functions to the linker.  */
-  update_section_order (section_list, num_entries);
+  update_section_order (section_list, unlikely_segment_start);
+  assert (num_entries > unlikely_segment_end);
+  update_section_order (section_list, num_entries - unlikely_segment_end);
+  /* Map all unlikely code into a new segment.  */
+  if (no_segment == 0)
+    unique_segment_for_sections (".text.unlikely_executed", 0, 0x1000,
+				 section_list + unlikely_segment_start,
+				 unlikely_segment_end - unlikely_segment_start);
   cleanup ();
   return LDPS_OK;
 }
Index: function_reordering_plugin/callgraph.c
===================================================================
--- function_reordering_plugin/callgraph.c	(revision 194505)
+++ function_reordering_plugin/callgraph.c	(working copy)
@@ -356,8 +356,16 @@ parse_callgraph_section_contents (void *file_handl
       Node *callee_node;
 
       callee = get_next_string (&contents, &read_length);
+      curr_length += read_length;
+
+      /* We can have multiple header lines; such a situation arises when
+         we've linked objects into a shared library, and we use that
+         library as input to the linker for something else.  Deal
+         gracefully with such cases.  */
+      if (strncmp (callee, "Function ", HEADER_LEN) == 0)
+	continue;
+
       callee = canonicalize_function_name (file_handle, callee);
-      curr_length += read_length;
       callee_node = get_function_node (callee);
 
       assert (curr_length < length);
@@ -516,6 +524,7 @@ const char *section_types[] = {".text.hot.",
    according to priority, higher priority (lower number), and then laid
    out in priority order.  */
 const int section_priority[] = {0, 3, 4, 2, 1};
+const int UNLIKELY_SECTION_INDEX = 2;
 
 /* Maps the function name corresponding to section SECTION_NAME to the
    object handle and the section index.  */
@@ -587,15 +596,16 @@ map_section_name_to_index (char *section_name, voi
     }
 }
 
-/* If SECN is NULL find the section corresponding to function name NAME.
-   If it is a comdat, get all the comdat sections in the group.  Chain these
-   sections to SECTION_END.  Set SECTION_START if it is NULL.  */
+/* Add section S to the chain SECTION_START ... SECTION_END.
+   If it is a comdat, get all the comdat sections in the group.
+   Chain these sections to SECTION_END.  Set SECTION_START if it
+   is NULL.  */
 
 static void
 write_out_node (Section_id *s, Section_id **section_start,
 	        Section_id **section_end)
 {
-  assert (s != NULL);
+  assert (s != NULL && s->processed == 0);
   s->processed = 1;
   if (*section_start == NULL)
     {
@@ -618,6 +628,9 @@ write_out_node (Section_id *s, Section_id **sectio
     }
 }
 
+int unlikely_segment_start = 0;
+int unlikely_segment_end = 0;
+
 /* Visit each node and print the chain of merged nodes to the file.  Update
    HANDLES and SHNDX to contain the ordered list of sections.  */
 
@@ -704,6 +717,8 @@ get_layout (FILE *fp, void*** handles,
   for (i = 0; i < NUM_SECTION_TYPES + 1; ++i)
     {
       s_it = section_start[i];
+      if (i == UNLIKELY_SECTION_INDEX + 1)
+	unlikely_segment_start = position;
       while (s_it)
         {
 	  assert (position < num_sections);
@@ -714,6 +729,8 @@ get_layout (FILE *fp, void*** handles,
             fprintf (fp, "%s\n", s_it->full_name);
 	  s_it = s_it->group;
         }
+      if (i == UNLIKELY_SECTION_INDEX + 1)
+	unlikely_segment_end = position;
     }
   return position;
 }
Index: include/plugin-api.h
===================================================================
--- include/plugin-api.h	(revision 194505)
+++ include/plugin-api.h	(working copy)
@@ -325,6 +325,33 @@ enum ld_plugin_level
   LDPL_FATAL
 };
 
+/* The linker's interface for specifying that a subset of sections is
+   to be mapped to a unique segment.  If the plugin wants to call
+   unique_segment_for_sections, it must call this function from a
+   claim_file_handler or when it is first loaded.  */
+
+typedef
+enum ld_plugin_status
+(*ld_plugin_allow_unique_segment_for_sections) (void);
+
+/* The linker's interface for specifying that a specific set of sections
+   must be mapped to a unique segment.  ELF segments do not have names
+   and the NAME is used as the name of the newly created output section
+   that is then placed in the unique PT_LOAD segment.  FLAGS is used to
+   specify if any additional segment flags need to be set.  For instance,
+   a specific segment flag can be set to identify this segment.  Unsetting
+   segment flags that would be set by default is not possible.  The
+   parameter SEGMENT_ALIGNMENT when non-zero will override the default.  */
+
+typedef
+enum ld_plugin_status
+(*ld_plugin_unique_segment_for_sections) (
+    const char* segment_name,
+    uint64_t segment_flags,
+    uint64_t segment_alignment,
+    const struct ld_plugin_section * section_list,
+    unsigned int num_sections);
+
 /* Values for the tv_tag field of the transfer vector.  */
 
 enum ld_plugin_tag
@@ -354,7 +381,9 @@ enum ld_plugin_tag
   LDPT_GET_INPUT_SECTION_CONTENTS,
   LDPT_UPDATE_SECTION_ORDER,
   LDPT_ALLOW_SECTION_ORDERING,
-  LDPT_GET_SYMBOLS_V2
+  LDPT_GET_SYMBOLS_V2,
+  LDPT_ALLOW_UNIQUE_SEGMENT_FOR_SECTIONS,
+  LDPT_UNIQUE_SEGMENT_FOR_SECTIONS
 };
 
 /* The plugin transfer vector.  */
@@ -384,6 +413,8 @@ struct ld_plugin_tv
     ld_plugin_get_input_section_contents tv_get_input_section_contents;
     ld_plugin_update_section_order tv_update_section_order;
     ld_plugin_allow_section_ordering tv_allow_section_ordering;
+    ld_plugin_allow_unique_segment_for_sections tv_allow_unique_segment_for_sections; 
+    ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections;
   } tv_u;
 };
 

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Sriraman Tallam]

I have committed this patch.

Thanks,
-Sri.

On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
> Looks good to me for google/gcc-4_7 branch.
>
> Thanks,
>
> -Rong
>
>
> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
> wrote:
>>
>> Hi Rong,
>>
>>     Please review this code. This code allows the function reordering
>> plugin to separate hot and cold code into different ELF segments.
>> This would allow optimizations like mapping the hot code alone to huge
>> pages.
>>
>>     With this patch, by default, the plugin maps .text.unlikely
>> sections into a separate ELF segment.  This can be turned off with
>> plugin option "--segment=none".
>>
>>     The include/plugin-api.h changes are a backport from trunk.
>>
>> Thanks,
>> -Sri.
>
>

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Sriraman Tallam]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

Hi Rong,

  The following patch modifies the behaviour of the linker plugin to
not create a separate segment for cold sections by default. Separate
segments can be created with the plugin option "segment=cold". Is this
alright to commit?

Thanks,
-Sri.

On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> I have committed this patch.
>
> Thanks,
> -Sri.
>
> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>> Looks good to me for google/gcc-4_7 branch.
>>
>> Thanks,
>>
>> -Rong
>>
>>
>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>> wrote:
>>>
>>> Hi Rong,
>>>
>>>     Please review this code. This code allows the function reordering
>>> plugin to separate hot and cold code into different ELF segments.
>>> This would allow optimizations like mapping the hot code alone to huge
>>> pages.
>>>
>>>     With this patch, by default, the plugin maps .text.unlikely
>>> sections into a separate ELF segment.  This can be turned off with
>>> plugin option "--segment=none".
>>>
>>>     The include/plugin-api.h changes are a backport from trunk.
>>>
>>> Thanks,
>>> -Sri.
>>
>>

[-- Attachment #2: reordering_plugin_patch.txt --]
[-- Type: text/plain, Size: 3878 bytes --]

Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 194878)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -34,8 +34,12 @@ along with this program; see the file COPYING3.  I
    This plugin dumps the final layout order of the functions in a file
    called "final_layout.txt".  To change the output file, pass the new
    file name with --plugin-opt.  To dump to stderr instead, just pass
-   stderr to --plugin-opt.  */
+   stderr to --plugin-opt.  
 
+   This plugin also allows placing all functions found cold in a separate
+   segment.  This can be enabled with the linker option:
+   --plugin-opt,segment=cold.  */
+
 #if HAVE_STDINT_H
 #include <stdint.h>
 #endif
@@ -89,9 +93,10 @@ static int is_api_exist = 0;
 /* The plugin does nothing when no-op is 1.  */
 static int no_op = 0;
 
-/* The plugin does not create a new segment for unlikely code if
-   no_segment is set.  */
-static int no_segment = 0;
+/* The plugin creates a new segment for unlikely code if unlikely_segment
+   is set.  This can be set with the linker option:
+   "--plugin-opt,segment=cold".  */
+static int unlikely_segment = 0;
 
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
@@ -132,10 +137,11 @@ process_option (const char *name)
   /* Check if options is "segment=none"  */
   if (strncmp (name, option_segment, strlen (option_segment)) == 0)
     {
-      if (strcmp (name + strlen (option_segment), "none") == 0)
-	no_segment = 1;
-      else
-	no_segment = 0;
+      const char *option_val = name + strlen (option_segment);
+      if (strcmp (option_val, "none") == 0)
+	unlikely_segment = 0;
+      else if (strcmp (option_val, "cold") == 0)
+	unlikely_segment = 1;
       return;
     }
 
@@ -244,7 +250,10 @@ claim_file_hook (const struct ld_plugin_input_file
     {
       /* Inform the linker to prepare for section reordering.  */
       (*allow_section_ordering) ();
-      (*allow_unique_segment_for_sections) ();
+      /* Inform the linker to allow certain sections to be placed in
+	 a separate segment.  */
+      if (unlikely_segment == 1)
+        (*allow_unique_segment_for_sections) ();
       is_ordering_specified = 1;
     }
 
@@ -335,15 +344,29 @@ all_symbols_read_hook (void)
   if (out_file != NULL
       && strcmp (out_file, "stderr") != 0)
     fclose (fp);
-  /* Pass the new order of functions to the linker.  */
-  update_section_order (section_list, unlikely_segment_start);
-  assert (num_entries >= unlikely_segment_end);
-  update_section_order (section_list, num_entries - unlikely_segment_end);
-  /* Map all unlikely code into a new segment.  */
-  if (no_segment == 0)
-    unique_segment_for_sections (".text.unlikely_executed", 0, 0x1000,
-				 section_list + unlikely_segment_start,
-				 unlikely_segment_end - unlikely_segment_start);
+
+  if (unlikely_segment == 1)
+    {
+      /* Pass the new order of functions to the linker.  */
+      /* Fix the order of all sections upto the beginning of the
+	 unlikely section.  */
+      update_section_order (section_list, unlikely_segment_start);
+      assert (num_entries >= unlikely_segment_end);
+      /* Fix the order of all sections after the end of the unlikely
+	 section.  */
+      update_section_order (section_list, num_entries - unlikely_segment_end);
+      /* Map all unlikely code into a new segment.  */
+      unique_segment_for_sections (
+	  ".text.unlikely_executed", 0, 0x1000,
+	  section_list + unlikely_segment_start,
+	  unlikely_segment_end - unlikely_segment_start);
+    }
+  else
+    {
+      /* Pass the new order of functions to the linker.  */
+      update_section_order (section_list, num_entries);
+    }
+
   cleanup ();
   return LDPS_OK;
 }

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Xinliang David Li]

Is it better to change the option to something like:

split_segment|nosplit-segment
or split_segment=yes|no


David

On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi Rong,
>
>   The following patch modifies the behaviour of the linker plugin to
> not create a separate segment for cold sections by default. Separate
> segments can be created with the plugin option "segment=cold". Is this
> alright to commit?
>
> Thanks,
> -Sri.
>
> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>> I have committed this patch.
>>
>> Thanks,
>> -Sri.
>>
>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>>> Looks good to me for google/gcc-4_7 branch.
>>>
>>> Thanks,
>>>
>>> -Rong
>>>
>>>
>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>>> wrote:
>>>>
>>>> Hi Rong,
>>>>
>>>>     Please review this code. This code allows the function reordering
>>>> plugin to separate hot and cold code into different ELF segments.
>>>> This would allow optimizations like mapping the hot code alone to huge
>>>> pages.
>>>>
>>>>     With this patch, by default, the plugin maps .text.unlikely
>>>> sections into a separate ELF segment.  This can be turned off with
>>>> plugin option "--segment=none".
>>>>
>>>>     The include/plugin-api.h changes are a backport from trunk.
>>>>
>>>> Thanks,
>>>> -Sri.
>>>
>>>

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Rong Xu]

The code looks fine to me. Please consider David's comments about the
option name.

-Rong

On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
> Is it better to change the option to something like:
>
> split_segment|nosplit-segment
> or split_segment=yes|no
>
>
> David
>
> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Hi Rong,
>>
>>   The following patch modifies the behaviour of the linker plugin to
>> not create a separate segment for cold sections by default. Separate
>> segments can be created with the plugin option "segment=cold". Is this
>> alright to commit?
>>
>> Thanks,
>> -Sri.
>>
>> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> I have committed this patch.
>>>
>>> Thanks,
>>> -Sri.
>>>
>>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>>>> Looks good to me for google/gcc-4_7 branch.
>>>>
>>>> Thanks,
>>>>
>>>> -Rong
>>>>
>>>>
>>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>>>> wrote:
>>>>>
>>>>> Hi Rong,
>>>>>
>>>>>     Please review this code. This code allows the function reordering
>>>>> plugin to separate hot and cold code into different ELF segments.
>>>>> This would allow optimizations like mapping the hot code alone to huge
>>>>> pages.
>>>>>
>>>>>     With this patch, by default, the plugin maps .text.unlikely
>>>>> sections into a separate ELF segment.  This can be turned off with
>>>>> plugin option "--segment=none".
>>>>>
>>>>>     The include/plugin-api.h changes are a backport from trunk.
>>>>>
>>>>> Thanks,
>>>>> -Sri.
>>>>
>>>>

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Sriraman Tallam]

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

Attached new patch.

Thanks,
-Sri.

On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu <xur@google.com> wrote:
> The code looks fine to me. Please consider David's comments about the
> option name.
>
> -Rong
>
> On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>> Is it better to change the option to something like:
>>
>> split_segment|nosplit-segment
>> or split_segment=yes|no
>>
>>
>> David
>>
>> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>> Hi Rong,
>>>
>>>   The following patch modifies the behaviour of the linker plugin to
>>> not create a separate segment for cold sections by default. Separate
>>> segments can be created with the plugin option "segment=cold". Is this
>>> alright to commit?
>>>
>>> Thanks,
>>> -Sri.
>>>
>>> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> I have committed this patch.
>>>>
>>>> Thanks,
>>>> -Sri.
>>>>
>>>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>>>>> Looks good to me for google/gcc-4_7 branch.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Rong
>>>>>
>>>>>
>>>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>>>>> wrote:
>>>>>>
>>>>>> Hi Rong,
>>>>>>
>>>>>>     Please review this code. This code allows the function reordering
>>>>>> plugin to separate hot and cold code into different ELF segments.
>>>>>> This would allow optimizations like mapping the hot code alone to huge
>>>>>> pages.
>>>>>>
>>>>>>     With this patch, by default, the plugin maps .text.unlikely
>>>>>> sections into a separate ELF segment.  This can be turned off with
>>>>>> plugin option "--segment=none".
>>>>>>
>>>>>>     The include/plugin-api.h changes are a backport from trunk.
>>>>>>
>>>>>> Thanks,
>>>>>> -Sri.
>>>>>
>>>>>

[-- Attachment #2: reordering_plugin_patch.txt --]
[-- Type: text/plain, Size: 4364 bytes --]

Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 194878)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -33,9 +33,13 @@ along with this program; see the file COPYING3.  I
 
    This plugin dumps the final layout order of the functions in a file
    called "final_layout.txt".  To change the output file, pass the new
-   file name with --plugin-opt.  To dump to stderr instead, just pass
-   stderr to --plugin-opt.  */
+   file name with --plugin-opt,file=<name>.  To dump to stderr instead,
+   just pass stderr as the file name.
 
+   This plugin also allows placing all functions found cold in a separate
+   segment.  This can be enabled with the linker option:
+   --plugin-opt,split_segment=yes.  */
+
 #if HAVE_STDINT_H
 #include <stdint.h>
 #endif
@@ -89,9 +93,10 @@ static int is_api_exist = 0;
 /* The plugin does nothing when no-op is 1.  */
 static int no_op = 0;
 
-/* The plugin does not create a new segment for unlikely code if
-   no_segment is set.  */
-static int no_segment = 0;
+/* The plugin creates a new segment for unlikely code if split_segment
+   is set.  This can be set with the linker option:
+   "--plugin-opt,split_segment=yes".  */
+static int split_segment = 0;
 
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
@@ -110,7 +115,7 @@ process_option (const char *name)
 {
   const char *option_group = "group=";
   const char *option_file = "file=";
-  const char *option_segment = "segment=";
+  const char *option_segment = "split_segment=";
 
   /* Check if option is "group="  */
   if (strncmp (name, option_group, strlen (option_group)) == 0)
@@ -129,13 +134,14 @@ process_option (const char *name)
       return;
     }
 
-  /* Check if options is "segment=none"  */
+  /* Check if options is "split_segment=[yes|no]"  */
   if (strncmp (name, option_segment, strlen (option_segment)) == 0)
     {
-      if (strcmp (name + strlen (option_segment), "none") == 0)
-	no_segment = 1;
-      else
-	no_segment = 0;
+      const char *option_val = name + strlen (option_segment);
+      if (strcmp (option_val, "no") == 0)
+	split_segment = 0;
+      else if (strcmp (option_val, "yes") == 0)
+	split_segment = 1;
       return;
     }
 
@@ -244,7 +250,10 @@ claim_file_hook (const struct ld_plugin_input_file
     {
       /* Inform the linker to prepare for section reordering.  */
       (*allow_section_ordering) ();
-      (*allow_unique_segment_for_sections) ();
+      /* Inform the linker to allow certain sections to be placed in
+	 a separate segment.  */
+      if (split_segment == 1)
+        (*allow_unique_segment_for_sections) ();
       is_ordering_specified = 1;
     }
 
@@ -335,15 +344,29 @@ all_symbols_read_hook (void)
   if (out_file != NULL
       && strcmp (out_file, "stderr") != 0)
     fclose (fp);
-  /* Pass the new order of functions to the linker.  */
-  update_section_order (section_list, unlikely_segment_start);
-  assert (num_entries >= unlikely_segment_end);
-  update_section_order (section_list, num_entries - unlikely_segment_end);
-  /* Map all unlikely code into a new segment.  */
-  if (no_segment == 0)
-    unique_segment_for_sections (".text.unlikely_executed", 0, 0x1000,
-				 section_list + unlikely_segment_start,
-				 unlikely_segment_end - unlikely_segment_start);
+
+  if (split_segment == 1)
+    {
+      /* Pass the new order of functions to the linker.  */
+      /* Fix the order of all sections upto the beginning of the
+	 unlikely section.  */
+      update_section_order (section_list, unlikely_segment_start);
+      assert (num_entries >= unlikely_segment_end);
+      /* Fix the order of all sections after the end of the unlikely
+	 section.  */
+      update_section_order (section_list, num_entries - unlikely_segment_end);
+      /* Map all unlikely code into a new segment.  */
+      unique_segment_for_sections (
+	  ".text.unlikely_executed", 0, 0x1000,
+	  section_list + unlikely_segment_start,
+	  unlikely_segment_end - unlikely_segment_start);
+    }
+  else
+    {
+      /* Pass the new order of functions to the linker.  */
+      update_section_order (section_list, num_entries);
+    }
+
   cleanup ();
   return LDPS_OK;
 }

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Xinliang David Li]

Looks good -- but better with followup :
1) give a warning when the parameter to the option is not allowed;
2) add test cases if possible.

David


On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> Attached new patch.
>
> Thanks,
> -Sri.
>
> On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu <xur@google.com> wrote:
>> The code looks fine to me. Please consider David's comments about the
>> option name.
>>
>> -Rong
>>
>> On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> Is it better to change the option to something like:
>>>
>>> split_segment|nosplit-segment
>>> or split_segment=yes|no
>>>
>>>
>>> David
>>>
>>> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>> Hi Rong,
>>>>
>>>>   The following patch modifies the behaviour of the linker plugin to
>>>> not create a separate segment for cold sections by default. Separate
>>>> segments can be created with the plugin option "segment=cold". Is this
>>>> alright to commit?
>>>>
>>>> Thanks,
>>>> -Sri.
>>>>
>>>> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>> I have committed this patch.
>>>>>
>>>>> Thanks,
>>>>> -Sri.
>>>>>
>>>>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>>>>>> Looks good to me for google/gcc-4_7 branch.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Rong
>>>>>>
>>>>>>
>>>>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Rong,
>>>>>>>
>>>>>>>     Please review this code. This code allows the function reordering
>>>>>>> plugin to separate hot and cold code into different ELF segments.
>>>>>>> This would allow optimizations like mapping the hot code alone to huge
>>>>>>> pages.
>>>>>>>
>>>>>>>     With this patch, by default, the plugin maps .text.unlikely
>>>>>>> sections into a separate ELF segment.  This can be turned off with
>>>>>>> plugin option "--segment=none".
>>>>>>>
>>>>>>>     The include/plugin-api.h changes are a backport from trunk.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -Sri.
>>>>>>
>>>>>>

Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments

[Sriraman Tallam]

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

On Fri, Jan 4, 2013 at 2:32 PM, Xinliang David Li <davidxl@google.com> wrote:
> Looks good -- but better with followup :
> 1) give a warning when the parameter to the option is not allowed;
> 2) add test cases if possible.

Made all the changes. Modified the test case to check if the segment
splitting API is invoked. The gold linker has a test case to check if
the segment API actually splits segments.

Thanks,
-Sri.

>
> David
>
>
> On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>> Attached new patch.
>>
>> Thanks,
>> -Sri.
>>
>> On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu <xur@google.com> wrote:
>>> The code looks fine to me. Please consider David's comments about the
>>> option name.
>>>
>>> -Rong
>>>
>>> On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li <davidxl@google.com> wrote:
>>>> Is it better to change the option to something like:
>>>>
>>>> split_segment|nosplit-segment
>>>> or split_segment=yes|no
>>>>
>>>>
>>>> David
>>>>
>>>> On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>> Hi Rong,
>>>>>
>>>>>   The following patch modifies the behaviour of the linker plugin to
>>>>> not create a separate segment for cold sections by default. Separate
>>>>> segments can be created with the plugin option "segment=cold". Is this
>>>>> alright to commit?
>>>>>
>>>>> Thanks,
>>>>> -Sri.
>>>>>
>>>>> On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>>> I have committed this patch.
>>>>>>
>>>>>> Thanks,
>>>>>> -Sri.
>>>>>>
>>>>>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <xur@google.com> wrote:
>>>>>>> Looks good to me for google/gcc-4_7 branch.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -Rong
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <tmsriram@google.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Rong,
>>>>>>>>
>>>>>>>>     Please review this code. This code allows the function reordering
>>>>>>>> plugin to separate hot and cold code into different ELF segments.
>>>>>>>> This would allow optimizations like mapping the hot code alone to huge
>>>>>>>> pages.
>>>>>>>>
>>>>>>>>     With this patch, by default, the plugin maps .text.unlikely
>>>>>>>> sections into a separate ELF segment.  This can be turned off with
>>>>>>>> plugin option "--segment=none".
>>>>>>>>
>>>>>>>>     The include/plugin-api.h changes are a backport from trunk.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> -Sri.
>>>>>>>
>>>>>>>

[-- Attachment #2: reordering_plugin_patch.txt --]
[-- Type: text/plain, Size: 6299 bytes --]

Index: function_reordering_plugin/function_reordering_plugin.c
===================================================================
--- function_reordering_plugin/function_reordering_plugin.c	(revision 194878)
+++ function_reordering_plugin/function_reordering_plugin.c	(working copy)
@@ -33,9 +33,13 @@ along with this program; see the file COPYING3.  I
 
    This plugin dumps the final layout order of the functions in a file
    called "final_layout.txt".  To change the output file, pass the new
-   file name with --plugin-opt.  To dump to stderr instead, just pass
-   stderr to --plugin-opt.  */
+   file name with --plugin-opt,file=<name>.  To dump to stderr instead,
+   just pass stderr as the file name.
 
+   This plugin also allows placing all functions found cold in a separate
+   segment.  This can be enabled with the linker option:
+   --plugin-opt,split_segment=yes.  */
+
 #if HAVE_STDINT_H
 #include <stdint.h>
 #endif
@@ -89,9 +93,10 @@ static int is_api_exist = 0;
 /* The plugin does nothing when no-op is 1.  */
 static int no_op = 0;
 
-/* The plugin does not create a new segment for unlikely code if
-   no_segment is set.  */
-static int no_segment = 0;
+/* The plugin creates a new segment for unlikely code if split_segment
+   is set.  This can be set with the linker option:
+   "--plugin-opt,split_segment=yes".  */
+static int split_segment = 0;
 
 /* Copies new output file name out_file  */
 void get_filename (const char *name)
@@ -110,7 +115,7 @@ process_option (const char *name)
 {
   const char *option_group = "group=";
   const char *option_file = "file=";
-  const char *option_segment = "segment=";
+  const char *option_segment = "split_segment=";
 
   /* Check if option is "group="  */
   if (strncmp (name, option_group, strlen (option_group)) == 0)
@@ -121,22 +126,26 @@ process_option (const char *name)
 	no_op = 0;
       return;
     }
-
   /* Check if option is "file=" */
-  if (strncmp (name, option_file, strlen (option_file)) == 0)
+  else if (strncmp (name, option_file, strlen (option_file)) == 0)
     {
       get_filename (name + strlen (option_file));
       return;
     }
-
-  /* Check if options is "segment=none"  */
-  if (strncmp (name, option_segment, strlen (option_segment)) == 0)
+  /* Check if options is "split_segment=[yes|no]"  */
+  else if (strncmp (name, option_segment, strlen (option_segment)) == 0)
     {
-      if (strcmp (name + strlen (option_segment), "none") == 0)
-	no_segment = 1;
-      else
-	no_segment = 0;
-      return;
+      const char *option_val = name + strlen (option_segment);
+      if (strcmp (option_val, "no") == 0)
+	{
+	  split_segment = 0;
+	  return;
+	}
+      else if (strcmp (option_val, "yes") == 0)
+	{
+	  split_segment = 1;
+	  return;
+	}
     }
 
   /* Unknown option, set no_op to 1.  */
@@ -244,7 +253,10 @@ claim_file_hook (const struct ld_plugin_input_file
     {
       /* Inform the linker to prepare for section reordering.  */
       (*allow_section_ordering) ();
-      (*allow_unique_segment_for_sections) ();
+      /* Inform the linker to allow certain sections to be placed in
+	 a separate segment.  */
+      if (split_segment == 1)
+        (*allow_unique_segment_for_sections) ();
       is_ordering_specified = 1;
     }
 
@@ -332,18 +344,35 @@ all_symbols_read_hook (void)
       section_list[i].shndx = shndx[i];
     }
 
+  if (split_segment == 1)
+    {
+      /* Pass the new order of functions to the linker.  */
+      /* Fix the order of all sections upto the beginning of the
+	 unlikely section.  */
+      update_section_order (section_list, unlikely_segment_start);
+      assert (num_entries >= unlikely_segment_end);
+      /* Fix the order of all sections after the end of the unlikely
+	 section.  */
+      update_section_order (section_list, num_entries - unlikely_segment_end);
+      /* Map all unlikely code into a new segment.  */
+      unique_segment_for_sections (
+	  ".text.unlikely_executed", 0, 0x1000,
+	  section_list + unlikely_segment_start,
+	  unlikely_segment_end - unlikely_segment_start);
+      if (fp != NULL)
+	fprintf (fp, "Moving %u section(s) to new segment\n",
+		 unlikely_segment_end - unlikely_segment_start);
+    }
+  else
+    {
+      /* Pass the new order of functions to the linker.  */
+      update_section_order (section_list, num_entries);
+    }
+
   if (out_file != NULL
       && strcmp (out_file, "stderr") != 0)
     fclose (fp);
-  /* Pass the new order of functions to the linker.  */
-  update_section_order (section_list, unlikely_segment_start);
-  assert (num_entries >= unlikely_segment_end);
-  update_section_order (section_list, num_entries - unlikely_segment_end);
-  /* Map all unlikely code into a new segment.  */
-  if (no_segment == 0)
-    unique_segment_for_sections (".text.unlikely_executed", 0, 0x1000,
-				 section_list + unlikely_segment_start,
-				 unlikely_segment_end - unlikely_segment_start);
+
   cleanup ();
   return LDPS_OK;
 }
Index: gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C
===================================================================
--- gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C	(revision 194878)
+++ gcc/testsuite/g++.dg/tree-prof/callgraph-profiles.C	(working copy)
@@ -2,7 +2,7 @@
    with -freorder-functions=. */
 /* { dg-require-section-exclude "" } */
 /* { dg-require-linker-function-reordering-plugin "" } */
-/* { dg-options "-O2 -freorder-functions=callgraph -ffunction-sections --save-temps -Wl,-plugin-opt,file=callgraph-profiles.C.dump" } */
+/* { dg-options "-O2 -freorder-functions=callgraph -ffunction-sections --save-temps -Wl,-plugin-opt,file=callgraph-profiles.C.dump -Wl,-plugin-opt,split_segment=yes" } */
 
 int
 notcalled ()
@@ -36,5 +36,6 @@ int main ()
 /* { dg-final-use { scan-assembler "\.string \"1000\"" } } */
 /* { dg-final-use { scan-file callgraph-profiles.C.dump "Callgraph group : main _Z3barv _Z3foov\n" } }  */
 /* { dg-final-use { scan-file callgraph-profiles.C.dump "\.text\.*\.main\n.text\.*\._Z3barv\n\.text\.*\._Z3foov\n\.text\.*\._Z9notcalledv" } }  */
+/* { dg-final-use { scan-file callgraph-profiles.C.dump "Moving 1 section\\(s\\) to new segment" } }  */
 /* { dg-final-use { cleanup-saved-temps } }  */
 /* { dg-final-use { remove-build-file "callgraph-profiles.C.dump" } }  */