From patchwork Tue Jul 4 16:25:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 21407 Received: (qmail 14904 invoked by alias); 4 Jul 2017 16:25:45 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 13128 invoked by uid 89); 4 Jul 2017 16:25:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f45.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=GJZzaNQM1w0Ra9gMBGnZ/2nJxe5j6Qiv8rDXZBj+9mU=; b=czEHii69mtSgonQJCUsIqMgTYLsr6gx4x5k0r4MtsB289lLjPI/ov4Obm49qbQV2Qt WSQBwToyfy5AiVCooJS7azjObPAB259/hycwQno4TK2+ozySCD+5j+DR5wvfaC7q0VzK 9o111IT7QENHuRYDvXAtyVdPP1kGslfUtg5jTMyMGB1/S5i2gH21qQQnTCAUmgnI8F3e W/I0Qv/g8pGRqFu0qUMhy8k3Yzbk124wJyY7RMOxnqhmyottKXkwS5aoGu0DYL3G1p+P s9Fj+UbiafHoO304xMz1RIb2TGtvHvF9AXDg2nEOmlnLnvEVQuu66HvFq5RFqk0XdosW qZ0Q== X-Gm-Message-State: AKS2vOx4tTqaRYKLyrgaORpUJxsb9rCB9KxTIEdtjXgz+5x5IEtncbzn gSx0n2RtlDbF0wmxeZVIsKhZu6bd1Q== X-Received: by 10.202.166.203 with SMTP id t72mr29933021oij.40.1499185540625; Tue, 04 Jul 2017 09:25:40 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <14a88e5f-a8c6-cd78-a363-1368d175f605@redhat.com> <20170703202551.GA17398@gmail.com> <3e579057-ebc3-a407-40c2-3956a017ff4e@redhat.com> <9266eaa7-4bf3-816a-1f9c-616be12b3502@redhat.com> From: "H.J. Lu" Date: Tue, 4 Jul 2017 09:25:39 -0700 Message-ID: Subject: Re: [PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609] To: Florian Weimer Cc: GNU C Library , Nicholas Miell , Jakub Jelinek On Tue, Jul 4, 2017 at 8:55 AM, H.J. Lu wrote: > On Tue, Jul 4, 2017 at 8:47 AM, Florian Weimer wrote: >> On 07/04/2017 05:16 PM, H.J. Lu wrote: >> >>>> Furthermore, the code GCC generates for stack realignment is really bad, >>> >>> GCC generates very good code for stack realignment when >>> -maccumulate-outgoing-args is used. >> >> I wonder why that isn't enabled by the force attribute. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81312 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81313 > >>>> # define __tls_get_addr __tls_get_addr_default >>>> +# include >>>> + >>>> +# undef __tls_get_addr_default >>> ^^^^^^^ Shouldn't it be __tls_get_addr? >> >> Looks this way. I'd have to re-test and see if it makes a difference. >> >>>> -typedef struct dl_tls_index >>>> -{ >>>> - uint64_t ti_module; >>>> - uint64_t ti_offset; >>>> -} tls_index; >> >>> Is this sysdeps/x86_64/dl-tlsdesc.h change related to this? >> >> It is because the patch includes both and from >> the same file, causing a multiple definition error without the removal >> of the conflicting definition. > > It is a cleanup. But I don't believe it is required here. > >>> __tls_get_addr_compat: >>> + .type __tls_get_addr_compat,@function >>> + .global __tls_get_addr_compat >>> + strong_alias (__tls_get_addr_compat, __tls_get_addr) >>> >>> We can use ENTRY/END here. Why do we need __tls_get_addr_compat? >>> Can we just have __tls_get_addr? >> >> That confuses the linker once we add the .symver directive. > > I don't think it is a case. We just define a different __tls_get_addr for > x86-64. > >>> Since we are talking performance here, we should add __tls_get_addr_slow >>> to only handle slow paths. >> >> I'd prefer something that adds a new symbol version for the non-aligning >> implementation, so that we eventually move away from the aligning one. > > I thought about it. There is no easy way to do it without linker help. We > can add ___tls_get_addr, similar to i386, which will take an aligned > stack. Linker must support ___tls_get_addr. Then we can use weakref > to redirect __tls_get_addr to ___tls_get_addr if linker supports > ___tls_get_addr and GCC doesn't. > Something like this. From 04186257d5d6fd0b6b8c69e86558209c3fa25902 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Tue, 4 Jul 2017 09:24:09 -0700 Subject: [PATCH] Add ___tls_get_addr --- sysdeps/unix/sysv/linux/x86_64/64/ld.abilist | 2 ++ sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist | 2 ++ sysdeps/x86_64/Versions | 6 ++++++ sysdeps/x86_64/dl-tls.c | 6 +++--- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist index 07cab4b..884e52c 100644 --- a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist @@ -6,6 +6,8 @@ GLIBC_2.2.5 calloc F GLIBC_2.2.5 free F GLIBC_2.2.5 malloc F GLIBC_2.2.5 realloc F +GLIBC_2.26 GLIBC_2.26 A +GLIBC_2.26 ___tls_get_addr F GLIBC_2.3 GLIBC_2.3 A GLIBC_2.3 __tls_get_addr F GLIBC_2.4 GLIBC_2.4 A diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist index 236357b..d42a7a4 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist +++ b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist @@ -7,3 +7,5 @@ GLIBC_2.16 calloc F GLIBC_2.16 free F GLIBC_2.16 malloc F GLIBC_2.16 realloc F +GLIBC_2.26 GLIBC_2.26 A +GLIBC_2.26 ___tls_get_addr F diff --git a/sysdeps/x86_64/Versions b/sysdeps/x86_64/Versions index a437f85..b8c25c9 100644 --- a/sysdeps/x86_64/Versions +++ b/sysdeps/x86_64/Versions @@ -10,3 +10,9 @@ libm { exp2l; } } +ld { + GLIBC_2.26 { + # The alternative x86-64 runtime interface to TLS with aligned stack. + ___tls_get_addr; + } +} diff --git a/sysdeps/x86_64/dl-tls.c b/sysdeps/x86_64/dl-tls.c index 567224e..3584805 100644 --- a/sysdeps/x86_64/dl-tls.c +++ b/sysdeps/x86_64/dl-tls.c @@ -25,13 +25,13 @@ /* Define __tls_get_addr within elf/dl-tls.c under a different name. */ -extern __typeof__ (__tls_get_addr) __tls_get_addr_default; +extern __typeof__ (__tls_get_addr) ___tls_get_addr; -# define __tls_get_addr __tls_get_addr_default +# define __tls_get_addr ___tls_get_addr # include # undef __tls_get_addr -hidden_ver (__tls_get_addr_default, __tls_get_addr) +hidden_ver (___tls_get_addr, __tls_get_addr) /* Only handle slow paths for __tls_get_addr. */ attribute_hidden -- 2.9.4