On Fri, Jan 4, 2013 at 2:32 PM, Xinliang David Li <[email protected]> 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 <[email protected]> wrote:
>> Attached new patch.
>>
>> Thanks,
>> -Sri.
>>
>> On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu <[email protected]> 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 <[email protected]>
>>> 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 <[email protected]>
>>>> 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 <[email protected]>
>>>>> wrote:
>>>>>> I have committed this patch.
>>>>>>
>>>>>> Thanks,
>>>>>> -Sri.
>>>>>>
>>>>>> On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu <[email protected]> wrote:
>>>>>>> Looks good to me for google/gcc-4_7 branch.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -Rong
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam <[email protected]>
>>>>>>> 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.
>>>>>>>
>>>>>>>
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" } } */
