Re: Gold linker patch to split unlikely text into a separate segment

["H.J. Lu" <hjl dot tools at gmail dot com>]

On 10/5/17, Sriraman Tallam <tmsriram@google.com> wrote:
> On Wed, Oct 4, 2017 at 5:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On 10/4/17, Sriraman Tallam <tmsriram@google.com> wrote:
>>> Hi,
>>>
>>>      This patch adds an option to gold to create a new ELF segment for
>>> code determined unlikely by the compiler.  Currently, this can be done
>>> with a linker plugin but I was wondering if it is fine to have this
>>> support in gold with an option for ease of use.
>>>
>>>      The advantages of doing this are:
>>>
>>> * We could only map the hot segment to huge pages to keep iTLB misses
>>> low
>>> * We could munlock the unlikely segment
>>>
>>> Is this alright?
>>>
>>> ChangeLog entry:
>>>
>>> * options.h (text_unlikely_segment): New option.
>>> * layout.cc (Layout::layout): Create new output section
>>> for .text.unlikely sections with the new option.
>>> (Layout::segment_precedes): Check for the new option
>>> when segment flags match.
>>>
>>> Patch attached.
>>>
>>
>> This is an interesting approach.  Do you have some performace
>> numbers?
>
> With function re-ordering of hot code, segment splitting and mapping
> only hot code to huge pages, we see a reduction in iTLB misses
> translating to performance improvements of 0.5 to 1% on some of our
> benchmarks.

Please include this info in your commit log.

>>
>> 2 Comments:
>>
>> 1.  I'd prefer "-z text-unlikely-segment" with '-', instead of '_'.
>> 2.  Need a testcase.
>
> Done and patch attached.
>

LGTM.  But I can't approve it.

Thanks.

-- 
H.J.