From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x236.google.com (mail-oi1-x236.google.com [IPv6:2607:f8b0:4864:20::236]) by sourceware.org (Postfix) with ESMTPS id E214D3848027 for ; Tue, 2 Mar 2021 14:21:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E214D3848027 Received: by mail-oi1-x236.google.com with SMTP id m25so9078901oie.12 for ; Tue, 02 Mar 2021 06:21:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LcNKSQoENbRK1EYlizriUQHiFhWhg8BQGn7EqsTAGFQ=; b=qfzNYW9SKQV+FI9riyMhNiSMgl18ekp+n3kiD1588O/2NyJwmMq+7c1gO9WZVoU6xy wQvmdFFgKi/Z5UBMUjIS+9B8UQMrhp2ioWOkbcmW5QKoEf0nKsJWwD0YBrI++D75vQlB +R7bItUqUWuQHVeeGpvIL378lm+WhAPhIoGIHB/GDBTU8+jhliwNprDl8JvNcmWsNVE6 jbqS6T94zFUGLHnhQl+9RdY1pU1qkcwplhysPb0OS98xeCZH9OohMooYDiFihzXiGyya 5jsBZE9EuJwLp7XM9cMpwjqQx+ony7gD6RuyO8o0cOF8+qaPh4sZjHYfpzYw9pbz0/b/ hpQw== X-Gm-Message-State: AOAM532jYips1RVqBmnYzona/mEa2NULJWWXrFh5kQjv/TIbqWKUt8Kw 45WsTTlOqkvmd5zboTKZKPvQwu02TCedwHTc+WmgaPtzsiE= X-Google-Smtp-Source: ABdhPJySHOUDr/i9NzBWpPUqIvAX1TyjY6wAVbgbgGFO/ByVuk7coZSHvwkxCFrRBSk3/5YRfkQM2dkRqiR0ll62kPA= X-Received: by 2002:aca:5783:: with SMTP id l125mr3389679oib.79.1614694896296; Tue, 02 Mar 2021 06:21:36 -0800 (PST) MIME-Version: 1.0 References: <20210202191209.4036619-1-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Tue, 2 Mar 2021 06:21:00 -0800 Message-ID: Subject: [PATCH v2] x86_64: Update THREAD_SETMEM/THREAD_SETMEM_NC for IMM64 To: "Carlos O'Donell" Cc: GNU C Library Content-Type: multipart/mixed; boundary="000000000000b7213005bc8e73cf" X-Spam-Status: No, score=-3034.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, 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: Tue, 02 Mar 2021 14:21:38 -0000 --000000000000b7213005bc8e73cf Content-Type: text/plain; charset="UTF-8" On Mon, Mar 1, 2021 at 5:30 AM Carlos O'Donell wrote: > > On 2/2/21 2:12 PM, H.J. Lu via Libc-alpha wrote: > > Since there is only "movq imm32s, mem64" and no "movq imm64, mem64", use > > "movq reg64, mem64" to store 64-bit constant in TCB. > > --- > > sysdeps/x86_64/nptl/tls.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h > > index 20f0958780..9ec8703e45 100644 > > --- a/sysdeps/x86_64/nptl/tls.h > > +++ b/sysdeps/x86_64/nptl/tls.h > > @@ -269,6 +269,11 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80, > > asm volatile ("movl %0,%%fs:%P1" : \ > > : IMM_MODE (value), \ > > "i" (offsetof (struct pthread, member))); \ > > + else if (__builtin_constant_p (value) \ > > + && (int64_t) (int32_t) (uintptr_t) value != (uintptr_t) value) \ > > + asm volatile ("movq %0,%%fs:%P1" : \ > > + : "r" (value), \ > > + "i" (offsetof (struct pthread, member))); \ > > (1) Move conditional into 'else /* 8 */' section. > > The blocks of conditionals are predicated on the size of the member we are > about to write to. > > In the block below... > > > else /* 8 */ \ > > ... here, we are about to write value into a member that is size 8. > > Your code changes the logical construction of the code, but in way that makes > it more difficult to understand. > > We previously had: > > if (sizeof() == 1) > > else if (sizeof() == 4) > > else /* Assume 8 */ > > In your case we must already be in the 'else /* Assume 8 */' because otherwise > we've be writing a 64-bit constant into a < 64-bit structure member. > > I think we should put your code into the else clause. > > 272 else /* 8 */ \ > 273 { \ > > if (__builtin_constant_p (value) > && ([the value is a >32-bit constant]) > asm volatile ([use movq reg64, mem64]); > else > > 274 asm volatile ("movq %q0,%%fs:%P1" : \ > 275 : IMM_MODE ((uint64_t) cast_to_integer (value)), \ > 276 "i" (offsetof (struct pthread, member))); \ > 277 }}) > > (2) What if gcc can't prove it's constant? > > If the constant is >32-bit, but gcc can't prove it's constant, then don't we > try to put a >32-bit constant into a 32-bit immediate? > > Shouldn't the code be structured the other way around? > > e.g. > > else /* 8 */ > { > if (__builtin_constant_p (value) > && ([the value is a <= 32-bit constant]) > asm volatile ([use movq imm32, mem64]); > else > asm volatile ([use movq reg64, mem64]); > } > > This way the code is always correct? I changed it to if (!__builtin_constant_p (value) \ || (int64_t) (int32_t) (uintptr_t) value == (uintptr_t) value) \ asm volatile ("movq %q0,%%fs:%P1" : \ : IMM_MODE ((uint64_t) cast_to_integer (value)), \ "i" (offsetof (struct pthread, member))); \ else \ asm volatile ("movq %0,%%fs:%P1" : \ : "r" (value), \ "i" (offsetof (struct pthread, member))); \ > > { \ > > asm volatile ("movq %q0,%%fs:%P1" : \ > > @@ -294,6 +299,12 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80, > > : IMM_MODE (value), \ > > "i" (offsetof (struct pthread, member[0])), \ > > "r" (idx)); \ > > + else if (__builtin_constant_p (value) \ > > + && (int64_t) (int32_t) (uintptr_t) value != (uintptr_t) value) \ > > + asm volatile ("movq %0,%%fs:%P1(,%q2,8)" : \ > > + : "r" (value), \ > > + "i" (offsetof (struct pthread, member[0])), \ > > + "r" (idx)); \ > > else /* 8 */ \ > > { \ > > asm volatile ("movq %q0,%%fs:%P1(,%q2,8)" : \ > > > Here is the v2 patch. OK for master? Thanks. -- H.J. --000000000000b7213005bc8e73cf Content-Type: text/x-patch; charset="US-ASCII"; name="v2-0001-x86_64-Update-THREAD_SETMEM-THREAD_SETMEM_NC-for-.patch" Content-Disposition: attachment; filename="v2-0001-x86_64-Update-THREAD_SETMEM-THREAD_SETMEM_NC-for-.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kls3k15i0 RnJvbSA0ZmJjOWFiNjc5MzNkNjYyNTA2YTAxNjU4ZDRjODFhOTIyZTUwZmRmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiAiSC5KLiBMdSIgPGhqbC50b29sc0BnbWFpbC5jb20+CkRhdGU6 IEZyaSwgOCBKYW4gMjAyMSAxNTozODoxNCAtMDgwMApTdWJqZWN0OiBbUEFUQ0ggdjJdIHg4Nl82 NDogVXBkYXRlIFRIUkVBRF9TRVRNRU0vVEhSRUFEX1NFVE1FTV9OQyBmb3IgSU1NNjQKClNpbmNl IHRoZXJlIGlzIG9ubHkgIm1vdnEgaW1tMzJzLCBtZW02NCIgYW5kIG5vICJtb3ZxIGltbTY0LCBt ZW02NCIsIHVzZQoibW92cSByZWc2NCwgbWVtNjQiIHRvIHN0b3JlIDY0LWJpdCBjb25zdGFudC4K LS0tCiBzeXNkZXBzL3g4Nl82NC9ucHRsL3Rscy5oIHwgMjcgKysrKysrKysrKysrKysrKysrKyst LS0tLS0tCiAxIGZpbGUgY2hhbmdlZCwgMjAgaW5zZXJ0aW9ucygrKSwgNyBkZWxldGlvbnMoLSkK CmRpZmYgLS1naXQgYS9zeXNkZXBzL3g4Nl82NC9ucHRsL3Rscy5oIGIvc3lzZGVwcy94ODZfNjQv bnB0bC90bHMuaAppbmRleCAyMGYwOTU4NzgwLi4wZGJkNjYyMDljIDEwMDY0NAotLS0gYS9zeXNk ZXBzL3g4Nl82NC9ucHRsL3Rscy5oCisrKyBiL3N5c2RlcHMveDg2XzY0L25wdGwvdGxzLmgKQEAg LTI3MSw5ICsyNzEsMTUgQEAgX1N0YXRpY19hc3NlcnQgKG9mZnNldG9mICh0Y2JoZWFkX3QsIF9f Z2xpYmNfdW51c2VkMikgPT0gMHg4MCwKIAkJICAgICAgICJpIiAob2Zmc2V0b2YgKHN0cnVjdCBw dGhyZWFkLCBtZW1iZXIpKSk7CSAgICAgIFwKICAgICAgZWxzZSAvKiA4ICovCQkJCQkJCSAgICAg IFwKICAgICAgICB7CQkJCQkJCQkgICAgICBcCi0JIGFzbSB2b2xhdGlsZSAoIm1vdnEgJXEwLCUl ZnM6JVAxIiA6CQkJCSAgICAgIFwKLQkJICAgICAgIDogSU1NX01PREUgKCh1aW50NjRfdCkgY2Fz dF90b19pbnRlZ2VyICh2YWx1ZSkpLAkgICAgICBcCi0JCQkgImkiIChvZmZzZXRvZiAoc3RydWN0 IHB0aHJlYWQsIG1lbWJlcikpKTsJICAgICAgXAorCSBpZiAoIV9fYnVpbHRpbl9jb25zdGFudF9w ICh2YWx1ZSkJCQkJICAgICAgXAorCSAgICAgfHwgKGludDY0X3QpIChpbnQzMl90KSAodWludHB0 cl90KSB2YWx1ZSA9PSAodWludHB0cl90KSB2YWx1ZSkgICBcCisJICAgYXNtIHZvbGF0aWxlICgi bW92cSAlcTAsJSVmczolUDEiIDoJCQkJICAgICAgXAorCQkJIDogSU1NX01PREUgKCh1aW50NjRf dCkgY2FzdF90b19pbnRlZ2VyICh2YWx1ZSkpLCAgICAgXAorCQkJICAgImkiIChvZmZzZXRvZiAo c3RydWN0IHB0aHJlYWQsIG1lbWJlcikpKTsJICAgICAgXAorCSBlbHNlCQkJCQkJCQkgICAgICBc CisJICAgYXNtIHZvbGF0aWxlICgibW92cSAlMCwlJWZzOiVQMSIgOgkJCQkgICAgICBcCisJCQkg OiAiciIgKHZhbHVlKSwJCQkJCSAgICAgIFwKKwkJCSAgICJpIiAob2Zmc2V0b2YgKHN0cnVjdCBw dGhyZWFkLCBtZW1iZXIpKSk7CSAgICAgIFwKICAgICAgICB9fSkKIAogCkBAIC0yOTYsMTAgKzMw MiwxNyBAQCBfU3RhdGljX2Fzc2VydCAob2Zmc2V0b2YgKHRjYmhlYWRfdCwgX19nbGliY191bnVz ZWQyKSA9PSAweDgwLAogCQkgICAgICAgInIiIChpZHgpKTsJCQkJCSAgICAgIFwKICAgICAgZWxz ZSAvKiA4ICovCQkJCQkJCSAgICAgIFwKICAgICAgICB7CQkJCQkJCQkgICAgICBcCi0JIGFzbSB2 b2xhdGlsZSAoIm1vdnEgJXEwLCUlZnM6JVAxKCwlcTIsOCkiIDoJCQkgICAgICBcCi0JCSAgICAg ICA6IElNTV9NT0RFICgodWludDY0X3QpIGNhc3RfdG9faW50ZWdlciAodmFsdWUpKSwJICAgICAg XAotCQkJICJpIiAob2Zmc2V0b2YgKHN0cnVjdCBwdGhyZWFkLCBtZW1iZXJbMF0pKSwJICAgICAg XAotCQkJICJyIiAoaWR4KSk7CQkJCQkgICAgICBcCisJIGlmICghX19idWlsdGluX2NvbnN0YW50 X3AgKHZhbHVlKQkJCQkgICAgICBcCisJICAgICB8fCAoaW50NjRfdCkgKGludDMyX3QpICh1aW50 cHRyX3QpIHZhbHVlID09ICh1aW50cHRyX3QpIHZhbHVlKSAgIFwKKwkgICBhc20gdm9sYXRpbGUg KCJtb3ZxICVxMCwlJWZzOiVQMSgsJXEyLDgpIiA6CQkJICAgICAgXAorCQkJIDogSU1NX01PREUg KCh1aW50NjRfdCkgY2FzdF90b19pbnRlZ2VyICh2YWx1ZSkpLCAgICAgXAorCQkJICAgImkiIChv ZmZzZXRvZiAoc3RydWN0IHB0aHJlYWQsIG1lbWJlclswXSkpLAkgICAgICBcCisJCQkgICAiciIg KGlkeCkpOwkJCQkJICAgICAgXAorCSBlbHNlCQkJCQkJCQkgICAgICBcCisJICAgYXNtIHZvbGF0 aWxlICgibW92cSAlMCwlJWZzOiVQMSgsJXEyLDgpIiA6CQkJICAgICAgXAorCQkJIDogInIiICh2 YWx1ZSksCQkJCQkgICAgICBcCisJCQkgICAiaSIgKG9mZnNldG9mIChzdHJ1Y3QgcHRocmVhZCwg bWVtYmVyWzBdKSksCSAgICAgIFwKKwkJCSAgICJyIiAoaWR4KSk7CQkJCQkgICAgICBcCiAgICAg ICAgfX0pCiAKIAotLSAKMi4yOS4yCgo= --000000000000b7213005bc8e73cf--