Performance bug while ordering .text sections by default in gold.
Sriraman Tallam
tmsriram@google.com
Wed Jan 23 22:51:00 GMT 2013
On Wed, Jan 23, 2013 at 8:29 AM, Ian Lance Taylor <iant@google.com> wrote:
> On Tue, Jan 22, 2013 at 11:35 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>
>> This patch to gold :
>> http://sourceware.org/ml/binutils/2012-12/msg00227.html introduces
>> unnecessary behaviour while ordering text sections and this is causing
>> performance regressions in some benchmarks. The intent of this patch
>> was to group .text sections with special prefixes like ".text.hot",
>> ".text.unlikely" , ".text.startup" together, which it achieves.
>> However, it does other undesired things too like sorting section names
>> of those sections without these special prefixes. This is because it
>> uses the compare function "Input_section_sort_compare" which was used
>> to sort ".ctors" and ".dtors". Sorting by section names can cause
>> undesired ordering like splitting functions in the same module.
>>
>> I am fixing this by making ".text" default sorting use the sort
>> function Input_section_sort_section_order_index_compare. I think this
>> is better as the different comparison criteria are much less and is
>> all applicable to ".text" under different contexts.
>
> Why reuse Input_section_sort_section_order_index_compare?
>
> It seems to me that you've identified a need for a new kind of
> ordering. Use a new function for that.
Attached new patch with the changes. Also added a new option to gold
"--notextreorder" to disable the default sorting.
Thanks
Sri
>
> Ian
-------------- next part --------------
* layout.cc (Layout::layout): Check for option --notextreorder.
(Layout::make_output_section): Ditto.
* options.h (notextreorder): New option.
* output.cc (Input_section_sort_compare): Remove special ordering
of section names.
(Output_section::
Input_section_sort_section_name_special_ordering_compare::
operator()): New function.
(Output_section::sort_attached_input_sections): Use new sort function
for .text.
* output.h (Input_section_sort_section_name_special_ordering_compare):
New struct.
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.244
diff -u -u -p -r1.244 layout.cc
--- layout.cc 18 Jan 2013 17:43:57 -0000 1.244
+++ layout.cc 23 Jan 2013 22:48:41 -0000
@@ -1149,7 +1149,8 @@ Layout::layout(Sized_relobj_file<size, b
// By default the GNU linker sorts some special text sections ahead
// of others. We are compatible.
- if (!this->script_options_->saw_sections_clause()
+ if (!parameters->options().notextreorder()
+ && !this->script_options_->saw_sections_clause()
&& !this->is_section_ordering_specified()
&& !parameters->options().relocatable()
&& Layout::special_ordering_of_input_section(name) >= 0)
@@ -1646,7 +1647,8 @@ Layout::make_output_section(const char*
// sections before other .text sections. We are compatible. We
// need to know that this might happen before we attach any input
// sections.
- if (!this->script_options_->saw_sections_clause()
+ if (!parameters->options().notextreorder()
+ && !this->script_options_->saw_sections_clause()
&& !this->is_section_ordering_specified()
&& !parameters->options().relocatable()
&& strcmp(name, ".text") == 0)
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.182
diff -u -u -p -r1.182 options.h
--- options.h 18 Jan 2013 17:44:31 -0000 1.182
+++ options.h 23 Jan 2013 22:48:41 -0000
@@ -878,6 +878,12 @@ class General_options
DEFINE_dirlist(library_path, options::TWO_DASHES, 'L',
N_("Add directory to search path"), N_("DIR"));
+ DEFINE_bool(notextreorder, options::TWO_DASHES, '\0', false,
+ N_("Disable the default text section reordering where text "
+ "sections with prefixes .text{unlikely,exit,startup,hot}"
+ "are grouped"),
+ NULL);
+
DEFINE_bool(nostdlib, options::ONE_DASH, '\0', false,
N_(" Only search directories specified on the command line."),
NULL);
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.180
diff -u -u -p -r1.180 output.cc
--- output.cc 7 Jan 2013 21:36:56 -0000 1.180
+++ output.cc 23 Jan 2013 22:48:41 -0000
@@ -3389,19 +3389,6 @@ Output_section::Input_section_sort_compa
return s1.index() < s2.index();
}
- // Some input section names have special ordering requirements.
- int o1 = Layout::special_ordering_of_input_section(s1.section_name().c_str());
- int o2 = Layout::special_ordering_of_input_section(s2.section_name().c_str());
- if (o1 != o2)
- {
- if (o1 < 0)
- return false;
- else if (o2 < 0)
- return true;
- else
- return o1 < o2;
- }
-
// A section with a priority follows a section without a priority.
bool s1_has_priority = s1.has_priority();
bool s2_has_priority = s2.has_priority();
@@ -3508,6 +3495,42 @@ Output_section::Input_section_sort_secti
return s1_secn_index < s2_secn_index;
}
+// Return true if S1 should come before S2. This is the sort comparison
+// function for .text to sort sections with prefixes
+// .text.{unlikely,exit,startup,hot} before other sections.
+bool
+Output_section::Input_section_sort_section_name_special_ordering_compare
+ ::operator()(
+ const Output_section::Input_section_sort_entry& s1,
+ const Output_section::Input_section_sort_entry& s2) const
+{
+ // We sort all the sections with no names to the end.
+ if (!s1.section_has_name() || !s2.section_has_name())
+ {
+ if (s1.section_has_name())
+ return true;
+ if (s2.section_has_name())
+ return false;
+ return s1.index() < s2.index();
+ }
+
+ // Some input section names have special ordering requirements.
+ int o1 = Layout::special_ordering_of_input_section(s1.section_name().c_str());
+ int o2 = Layout::special_ordering_of_input_section(s2.section_name().c_str());
+ if (o1 != o2)
+ {
+ if (o1 < 0)
+ return false;
+ else if (o2 < 0)
+ return true;
+ else
+ return o1 < o2;
+ }
+
+ // Keep input order otherwise.
+ return s1.index() < s2.index();
+}
+
// This updates the section order index of input sections according to the
// the order specified in the mapping from Section id to order index.
@@ -3576,6 +3599,9 @@ Output_section::sort_attached_input_sect
|| this->type() == elfcpp::SHT_FINI_ARRAY)
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_init_fini_compare());
+ else if (strcmp (this->name(), ".text") == 0)
+ std::sort(sort_list.begin(), sort_list.end(),
+ Input_section_sort_section_name_special_ordering_compare());
else
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_compare());
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.145
diff -u -u -p -r1.145 output.h
--- output.h 10 Jan 2013 00:18:14 -0000 1.145
+++ output.h 23 Jan 2013 22:48:41 -0000
@@ -4200,6 +4200,15 @@ class Output_section : public Output_dat
const Input_section_sort_entry&) const;
};
+ // This is the sort comparison function for .text to sort sections with
+ // prefixes .text.{unlikely,exit,startup,hot} before other sections.
+ struct Input_section_sort_section_name_special_ordering_compare
+ {
+ bool
+ operator()(const Input_section_sort_entry&,
+ const Input_section_sort_entry&) const;
+ };
+
// Fill data. This is used to fill in data between input sections.
// It is also used for data statements (BYTE, WORD, etc.) in linker
// scripts. When we have to keep track of the input sections, we
More information about the Binutils
mailing list