From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 25AC33858024 for ; Thu, 1 Apr 2021 13:51:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 25AC33858024 Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-531-wh_4YE71PQiH9DDQYvb5_A-1; Thu, 01 Apr 2021 09:51:47 -0400 X-MC-Unique: wh_4YE71PQiH9DDQYvb5_A-1 Received: by mail-qv1-f70.google.com with SMTP id fb10so3376634qvb.20 for ; Thu, 01 Apr 2021 06:51:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=L/iKE+Xe4dhbtNaxxmkgtpvyz0RR8giTdD4R2nKn6J0=; b=qkXyrAnwPD6KcUth72P5sXnOB2fOYSX8ZUiJBubFLR9n4Gqyex91Ck0wVoI3a1JYGL 2mC/fj1zN5DkCnvXPJyekrghgtJC/mjeUa2MQys654v0hIEKXYxXm9upoN1pVvreMUt8 yfGPGsNXfL8357wKDkuJHqE0fbaOdgjAm4ryD5fSOqPfHnoqWj45QpBCE6Y2gKjbwinR 2gktwyk+lSIx4nza+IZxCV/B/eQKkStyX6GRZ2oVuKVMzKniyQBOONlkt0tk4kkqUM/T a+jgng3wIYn3DzlqkZXtMhBUX2c4L16QpZvpGLJMzukXVZO0lqxyg3l1XKXvu2UZsJMP krEA== X-Gm-Message-State: AOAM533iOYinoBL2p1tETwUSBl2bYgqONiV0+aNQDITn7ZIMlioD0AXW xBp0LrTc6WP/N1MZ59pMsI2fA1vkVCkcGTNDFpRhUvjwQKP2fGRMcyonfv3TkaCn7olzya0Po9r vGQuAPVJ4YPWyU99Vz3EyhvyR81Pq5CxkRFhobukJR9Am1Vir0wW3e5MFE1nhrltRawGnmQ== X-Received: by 2002:a37:6348:: with SMTP id x69mr8136426qkb.154.1617285106833; Thu, 01 Apr 2021 06:51:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyhmKRiemFnKmB34KrSI2A7CnTZ73aSw827DtRmx5npepu3rxOpGtQh5gPs265KTnRLLZjsZw== X-Received: by 2002:a37:6348:: with SMTP id x69mr8136405qkb.154.1617285106539; Thu, 01 Apr 2021 06:51:46 -0700 (PDT) Received: from [192.168.1.16] (198-84-214-74.cpe.teksavvy.com. [198.84.214.74]) by smtp.gmail.com with ESMTPSA id f8sm4001921qkk.23.2021.04.01.06.51.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Apr 2021 06:51:45 -0700 (PDT) Subject: Re: [PATCH v6] x86_64: Correct THREAD_SETMEM/THREAD_SETMEM_NC for movq [BZ #27591] To: "H.J. Lu" , libc-alpha@sourceware.org References: <20210319183536.1663081-1-hjl.tools@gmail.com> From: Carlos O'Donell Organization: Red Hat Message-ID: <2f275d3b-d3eb-cf6b-fce0-c2085b4c3eb3@redhat.com> Date: Thu, 1 Apr 2021 09:51:44 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20210319183536.1663081-1-hjl.tools@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Apr 2021 13:51:52 -0000 On 3/19/21 2:35 PM, H.J. Lu via Libc-alpha wrote: > config/i386/constraints.md in GCC has > > (define_constraint "e" > "32-bit signed integer constant, or a symbolic reference known > to fit that range (for immediate operands in sign-extending x86-64 > instructions)." > (match_operand 0 "x86_64_immediate_operand")) > > Since movq takes a signed 32-bit immediate or a register source operand, > use "er", instead of "nr"/"ir", constraint for 32-bit signed integer > constant or register on movq. v6, and this version looks amazing, much better than before and I think we've arrived a good solution to this problem. Thank you for your efforts and working through the details. Reviewed-by: Carlos O'Donell > --- > sysdeps/x86_64/Makefile | 2 + > sysdeps/x86_64/nptl/tls.h | 10 ++++- > sysdeps/x86_64/tst-x86-64-tls-1.c | 64 +++++++++++++++++++++++++++++++ > 3 files changed, 74 insertions(+), 2 deletions(-) > create mode 100644 sysdeps/x86_64/tst-x86-64-tls-1.c > > diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile > index d1d7cb9d2e..06a444b6af 100644 > --- a/sysdeps/x86_64/Makefile > +++ b/sysdeps/x86_64/Makefile > @@ -183,6 +183,8 @@ ifeq (no,$(build-hardcoded-path-in-tests)) > tests-container += tst-glibc-hwcaps-cache > endif > > +tests-internal += tst-x86-64-tls-1 OK. Test is internal with use of tls.h header. > + > endif # $(subdir) == elf > > ifeq ($(subdir),csu) > diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h > index 20f0958780..a78c4f4d01 100644 > --- a/sysdeps/x86_64/nptl/tls.h > +++ b/sysdeps/x86_64/nptl/tls.h > @@ -271,8 +271,11 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80, > "i" (offsetof (struct pthread, member))); \ > else /* 8 */ \ > { \ > + /* Since movq takes a signed 32-bit immediate or a register source \ > + operand, use "er" constraint for 32-bit signed integer constant \ > + or register. */ \ > asm volatile ("movq %q0,%%fs:%P1" : \ > - : IMM_MODE ((uint64_t) cast_to_integer (value)), \ > + : "er" ((uint64_t) cast_to_integer (value)), \ OK. Great change, very clear, and uses immediate or register to get it to work. > "i" (offsetof (struct pthread, member))); \ > }}) > > @@ -296,8 +299,11 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80, > "r" (idx)); \ > else /* 8 */ \ > { \ > + /* Since movq takes a signed 32-bit immediate or a register source \ > + operand, use "er" constraint for 32-bit signed integer constant \ > + or register. */ \ > asm volatile ("movq %q0,%%fs:%P1(,%q2,8)" : \ > - : IMM_MODE ((uint64_t) cast_to_integer (value)), \ > + : "er" ((uint64_t) cast_to_integer (value)), \ > "i" (offsetof (struct pthread, member[0])), \ OK. > "r" (idx)); \ > }}) > diff --git a/sysdeps/x86_64/tst-x86-64-tls-1.c b/sysdeps/x86_64/tst-x86-64-tls-1.c > new file mode 100644 > index 0000000000..354635884e > --- /dev/null > +++ b/sysdeps/x86_64/tst-x86-64-tls-1.c > @@ -0,0 +1,64 @@ > +/* Test THREAD_SETMEM and THREAD_SETMEM_NC for IMM64. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > + > +static int > +do_test (void) > +{ > + unsigned long long int saved_ssp_base, ssp_base; > + saved_ssp_base = THREAD_GETMEM (THREAD_SELF, header.ssp_base); > + > + THREAD_SETMEM (THREAD_SELF, header.ssp_base, (1ULL << 57) - 1); > + ssp_base = THREAD_GETMEM (THREAD_SELF, header.ssp_base); > + if (ssp_base != ((1ULL << 57) - 1)) > + FAIL_EXIT1 ("THREAD_SETMEM: 0x%llx != 0x%llx", > + ssp_base, (1ULL << 57) - 1); > + > + THREAD_SETMEM (THREAD_SELF, header.ssp_base, -1ULL); > + ssp_base = THREAD_GETMEM (THREAD_SELF, header.ssp_base); > + if (ssp_base != -1ULL) > + FAIL_EXIT1 ("THREAD_SETMEM: 0x%llx != 0x%llx", ssp_base, -1ULL); > + > + THREAD_SETMEM (THREAD_SELF, header.ssp_base, saved_ssp_base); > +#ifndef __ILP32__ > + struct pthread_key_data *saved_specific, *specific; > + saved_specific = THREAD_GETMEM_NC (THREAD_SELF, specific, 1); > + > + uintptr_t value = (1UL << 57) - 1; > + THREAD_SETMEM_NC (THREAD_SELF, specific, 1, > + (struct pthread_key_data *) value); > + specific = THREAD_GETMEM_NC (THREAD_SELF, specific, 1); > + if (specific != (struct pthread_key_data *) value) > + FAIL_EXIT1 ("THREAD_GETMEM_NC: %p != %p", > + specific, (struct pthread_key_data *) value); > + > + THREAD_SETMEM_NC (THREAD_SELF, specific, 1, > + (struct pthread_key_data *) -1UL); > + specific = THREAD_GETMEM_NC (THREAD_SELF, specific, 1); > + if (specific != (struct pthread_key_data *) -1UL) > + FAIL_EXIT1 ("THREAD_GETMEM_NC: %p != %p", > + specific, (struct pthread_key_data *) -1UL); > + > + THREAD_SETMEM_NC (THREAD_SELF, specific, 1, saved_specific); > +#endif > + return 0; > +} > + > +#include OK. > -- Cheers, Carlos.