[PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609]

[Carlos O'Donell]

On 07/06/2017 11:44 AM, Alexander Monakov wrote:
> On Thu, 6 Jul 2017, H.J. Lu wrote:
> 
>> On Thu, Jul 6, 2017 at 8:32 AM, Alexander Monakov <amonakov@ispras.ru> wrote:
>>> On Thu, 6 Jul 2017, Alexander Monakov wrote:
>>> Actually, looking at elf/dl-tls.c, I see that __tls_get_addr doesn't need
>>> aligned stack, and its callees update_get_addr and tls_get_addr_tail are
>>> already marked noinline, so isn't it sufficient to additionally mark those
>>> with __attribute__((force_align_arg_pointer)) on x86-64?
>>>
>>
>> Please take a look at the current master branch.  I have pushed a fix
>> today:
>>
>> https://sourceware.org/git/?p=glibc.git;a=commit;h=031e519c95c069abe4e4c7c59e2b4b67efccdee5
> 
> Right, but my point was that by simply adding the attribute to two existing
> noinline functions all that new complexity could have been avoided.
> 
> What's the point of the proposed new ___tls_get_addr? Avoiding realigning
> the stack in cases when Glibc has to do rather complex work anyway? Is it
> even possible to measure the difference?

Option (b) has been chosen for this release, so that's what we're doing for
the release, but we can continue to discuss the optimization here.

(1) Cost.

No matter what you do it's extra instructions on the hot path forever.

(2) Small changes add up.

Measuring small changes like this is hard, and what about the sum of the
impact on a whole system, or systems? Power? CPU cycles? All of those have
costs for something as fundamental as __tls_get_addr. The extra instructions
do have i-cache pressure (which might be worked around by using an 'unlikely'
section far away to store code, which complicates jumping to it).

(3) Better benchmarks.

It would be nice to add a TLS microbenchmark into the suite and see if
it's measurable in a longer running test. Objective data would help settle
the decision on if the optimization is worth the developer complexity.

-- 
Cheers,
Carlos.