This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold] enable sorting of text sections with the same prefix
@@ -3527,8 +3528,44 @@
Output_section::Input_section_sort_section_name_special_ordering_compare
return o1 < o2;
}
+ if (strcmp(parameters->options().sort_section(), "name") == 0)
+ {
+ // Within each prefix we sort by name.
+ int compare = s1.section_name().compare(s2.section_name());
+ if (compare != 0)
+ return compare < 0;
+ }
+
Why do you still need this? Maybe you forgot to remove this.
// This updates the section order index of input sections according to the
@@ -3600,8 +3637,14 @@ Output_section::sort_attached_input_sections()
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());
+ {
+ if (strcmp(parameters->options().sort_section(), "name") == 0)
+ std::sort(sort_list.begin(), sort_list.end(),
+ Input_section_sort_section_name_compare());
+ else
+ std::sort(sort_list.begin(), sort_list.end(),
+ Input_section_sort_section_prefix_special_ordering_compare());
+ }
So, you are using Input_section_sort_section_name_compare only for
.text, why is that? What about other sections. You can do something
like this:
if (strcmp(parameters->options().sort_section(), "name") == 0))
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_section_name_compare());
else if (strcmp(this->name(), ".text") == 0)
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_section_prefix_special_ordering_compare());
This will override the default sorting of ".text" with name sorting if
--sort-section=name is specified and will sort sections by name for
other sections like .data.
You also need test cases.
On Tue, Feb 5, 2013 at 5:30 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> You are right: strcmp in a sorting function is not a good idea..
> I did what you proposed in the first paragraph. Could you please take a look
>
> thanks,
> Alexander
>
> 2013/2/4 Sriraman Tallam <tmsriram@google.com>:
>> @@ -3527,8 +3528,16 @@
>> Output_section::Input_section_sort_section_name_special_ordering_compare
>> return o1 < o2;
>> }
>>
>> + if (strcmp(parameters->options().sort_section(), "name") == 0)
>> + {
>> + // Within each prefix we sort by name.
>> + int compare = s1.section_name().compare(s2.section_name());
>> + if (compare != 0)
>> + return compare < 0;
>> + }
>> +
>>
>> I think you can create a new compare function for this. You can rename
>> the existing function to something like
>> "Input_section_sort_section_prefix_special_ordering_compare" and make
>> a new "Input_section_sort_section_name_compare". Sorting by section
>> name also ensures the grouping of functions with prefixes like
>> ".text.hot", only the entire .text.hot could land up somewhere in the
>> middle of all functions.
>>
>> However, if you do want both special ordering and sorting by section
>> name, I think you should move the strcmp out of the compare function
>> as the compare function is called many times. A simple bool function
>> somewhere in layout would be fine I think.
>>
>> Thanks
>> Sri
>>
>> On Mon, Feb 4, 2013 at 4:15 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>> Hi,
>>>
>>> The attached patch implements --sort-section=name that sorts text
>>> sections by name within each
>>> prefix and also sorts .data and .sdata sections by name (as BFD does).
>>> That is enough for closing
>>> that http://sourceware.org/bugzilla/show_bug.cgi?id=14948
>>>
>>> thank you,
>>> Alexander
>>>
>>> 2013/1/29 Sriraman Tallam <tmsriram@google.com>:
>>>> On Tue, Jan 29, 2013 at 9:55 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>>>> Hello Sri,
>>>>>
>>>>> thank you for your input!
>>>>> Please, look at
>>>>> http://sourceware.org/bugzilla/show_bug.cgi?id=14948
>>>>
>>>> So, why not do this guarded by --sort-section=name just like BFD ld?
>>>> That way, it does not affect the default. Just my two cents.
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>>>
>>>>> thank you,
>>>>> Alexander
>>>>>
>>>>> 2013/1/29 Sriraman Tallam <tmsriram@google.com>:
>>>>>> Hi,
>>>>>>
>>>>>> On Tue, Jan 29, 2013 at 7:28 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch to gold: http://sourceware.org/ml/binutils/2013-01/msg00335.html
>>>>>>> disabled sorting of text sections with the same prefix like
>>>>>>> .text.hot0001, .text.hot0002
>>>>>>> which is very desirable.
>>>>>>
>>>>>> Sorting by section names in general for text sections turns out to be
>>>>>> a bad idea as we have seen many performance issues. That is the reason
>>>>>> why the patch you reference was created.
>>>>>>
>>>>>> The reason why some text sections are grouped to begin with is to
>>>>>> mimic the default GNU ld behaviour. I dont think GNU ld groups these
>>>>>> two sections. AFAIK, ".text.hot." is the prefix for hot text
>>>>>> sections. How were these two sections you mention created?Which
>>>>>> compiler is generating these two sections? Or, did you explicitly use
>>>>>> a section attribute?
>>>>>>
>>>>>> Thanks
>>>>>> Sri
>>>>>>
>>>>>> The attached patch fix this.
>>>>>>>
>>>>>>> OK for trunk?
>>>>>>>
>>>>>>> thank you,
>>>>>>> Alexander