diff mbox

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

Message ID CAAs8HmxS+hKy7CWdhtbp93cRSo6aF5NdVzgOep=yoiYqF2exOA@mail.gmail.com
State New
Headers show

Commit Message

Sriraman Tallam Jan. 4, 2013, 1:41 a.m. UTC 
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.
>>
>>

Comments

Xinliang David Li Jan. 4, 2013, 5:14 a.m. UTC | #1 
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.
>>>
>>>
Rong Xu Jan. 4, 2013, 5:12 p.m. UTC | #2 
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.
>>>>
>>>>
diff mbox

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)
@@ -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;
 }