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

login
register
mail settings
Submitter Sriraman Tallam
Date Jan. 5, 2013, 2:32 a.m.
Message ID <CAAs8HmwXdc=TxKd3fe24nGyhnqfX9poxzN+=H1dSZp=eXGNC0w@mail.gmail.com>
Download mbox | patch
Permalink /patch/209642/
State New
Headers show

Comments

Sriraman Tallam - Jan. 5, 2013, 2:32 a.m.
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.
>>>>>>>
>>>>>>>

Patch

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" } }  */