Re: [gold] enable sorting of text sections with the same prefix

[Alexander Ivchenko <aivchenk at gmail dot com>]

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

Attachment: enable_sorting_text_sections_by_name_4.patch
Description: Binary data