Message ID | Y3Hy1ckL3ZluEOSi@tucnak |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5EBF83835781 for <patchwork@sourceware.org>; Mon, 14 Nov 2022 07:49:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5EBF83835781 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1668412158; bh=v+eMMTNbY+U9aohzxk9ZIKWUQYZ1pL5L1HpCWcl84eQ=; h=Date:To:Cc:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=qcO7o8NJmZKQ4ADRMvtOSDcg6qvJenz01ZznmRb/53MvF8yU3FCTKiKPmzAEyMgfa eAZgO/WbghAgO3N1FrVYhn8fM+G71Za9/DC34n/D1oebpp4jQ+bMano6K0MPFHetHZ JgUNIjTXVuIv7n6r0ozgDYT956u8QSuaPUJA6/h0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 88BFB385841D for <gcc-patches@gcc.gnu.org>; Mon, 14 Nov 2022 07:48:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 88BFB385841D Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-621-In_-65CsOQmyGpkORcuoTQ-1; Mon, 14 Nov 2022 02:48:45 -0500 X-MC-Unique: In_-65CsOQmyGpkORcuoTQ-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8561C185A78F; Mon, 14 Nov 2022 07:48:44 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 46651492B06; Mon, 14 Nov 2022 07:48:44 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 2AE7mdF02822429 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Mon, 14 Nov 2022 08:48:39 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 2AE7mcpG2822428; Sun, 13 Nov 2022 19:48:38 -1200 Date: Sun, 13 Nov 2022 19:48:37 -1200 To: Richard Biener <rguenther@suse.de>, Jeff Law <jeffreyalaw@gmail.com>, Uros Bizjak <ubizjak@gmail.com> Cc: gcc-patches@gcc.gnu.org, Florian Weimer <fweimer@redhat.com>, "H.J. Lu" <hjl.tools@gmail.com> Subject: [PATCH] libatomic: Handle AVX+CX16 AMD like Intel for 16b atomics [PR104688] Message-ID: <Y3Hy1ckL3ZluEOSi@tucnak> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jakub Jelinek <jakub@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
libatomic: Handle AVX+CX16 AMD like Intel for 16b atomics [PR104688]
|
|
Commit Message
Jakub Jelinek
Nov. 14, 2022, 7:48 a.m. UTC
Hi! Working virtually out of Baker Island. We got a response from AMD in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104688#c10 so the following patch starts treating AMD with AVX and CMPXCHG16B ISAs like Intel by using vmovdqa for atomic load/store in libatomic. Ok for trunk if it passes bootstrap/regtest? 2022-11-13 Jakub Jelinek <jakub@redhat.com> PR target/104688 * config/x86/init.c (__libat_feat1_init): Revert 2022-03-17 change - on x86_64 no longer clear bit_AVX if CPU vendor is not Intel. Jakub
Comments
On Mon, Nov 14, 2022 at 8:48 AM Jakub Jelinek <jakub@redhat.com> wrote: > > Hi! > > Working virtually out of Baker Island. > > We got a response from AMD in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104688#c10 > so the following patch starts treating AMD with AVX and CMPXCHG16B > ISAs like Intel by using vmovdqa for atomic load/store in libatomic. > > Ok for trunk if it passes bootstrap/regtest? > > 2022-11-13 Jakub Jelinek <jakub@redhat.com> > > PR target/104688 > * config/x86/init.c (__libat_feat1_init): Revert 2022-03-17 change > - on x86_64 no longer clear bit_AVX if CPU vendor is not Intel. > > --- libatomic/config/x86/init.c.jj 2022-03-17 18:48:56.708723194 +0100 > +++ libatomic/config/x86/init.c 2022-11-13 18:23:26.315440071 -1200 > @@ -34,18 +34,6 @@ __libat_feat1_init (void) > unsigned int eax, ebx, ecx, edx; > FEAT1_REGISTER = 0; > __get_cpuid (1, &eax, &ebx, &ecx, &edx); > -#ifdef __x86_64__ > - if ((FEAT1_REGISTER & (bit_AVX | bit_CMPXCHG16B)) > - == (bit_AVX | bit_CMPXCHG16B)) > - { > - /* Intel SDM guarantees that 16-byte VMOVDQA on 16-byte aligned address > - is atomic, but so far we don't have this guarantee from AMD. */ > - unsigned int ecx2 = 0; > - __get_cpuid (0, &eax, &ebx, &ecx2, &edx); > - if (ecx2 != signature_INTEL_ecx) > - FEAT1_REGISTER &= ~bit_AVX; We still need this, but also bypass it for AMD signature. There are other vendors than Intel and AMD. OK with the above addition. Thanks, Uros. > - } > -#endif > /* See the load in load_feat1. */ > __atomic_store_n (&__libat_feat1, FEAT1_REGISTER, __ATOMIC_RELAXED); > return FEAT1_REGISTER; > > Jakub >
On Mon, 2022-11-14 at 08:55 +0100, Uros Bizjak via Gcc-patches wrote: > On Mon, Nov 14, 2022 at 8:48 AM Jakub Jelinek <jakub@redhat.com> > wrote: > > > > Hi! > > > > Working virtually out of Baker Island. > > > > We got a response from AMD in > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104688#c10 > > so the following patch starts treating AMD with AVX and CMPXCHG16B > > ISAs like Intel by using vmovdqa for atomic load/store in libatomic. > > > > Ok for trunk if it passes bootstrap/regtest? > > > > 2022-11-13 Jakub Jelinek <jakub@redhat.com> > > > > PR target/104688 > > * config/x86/init.c (__libat_feat1_init): Revert 2022-03-17 > > change > > - on x86_64 no longer clear bit_AVX if CPU vendor is not > > Intel. > > > > --- libatomic/config/x86/init.c.jj 2022-03-17 > > 18:48:56.708723194 +0100 > > +++ libatomic/config/x86/init.c 2022-11-13 18:23:26.315440071 -1200 > > @@ -34,18 +34,6 @@ __libat_feat1_init (void) > > unsigned int eax, ebx, ecx, edx; > > FEAT1_REGISTER = 0; > > __get_cpuid (1, &eax, &ebx, &ecx, &edx); > > -#ifdef __x86_64__ > > - if ((FEAT1_REGISTER & (bit_AVX | bit_CMPXCHG16B)) > > - == (bit_AVX | bit_CMPXCHG16B)) > > - { > > - /* Intel SDM guarantees that 16-byte VMOVDQA on 16-byte > > aligned address > > - is atomic, but so far we don't have this guarantee from > > AMD. */ > > - unsigned int ecx2 = 0; > > - __get_cpuid (0, &eax, &ebx, &ecx2, &edx); > > - if (ecx2 != signature_INTEL_ecx) > > - FEAT1_REGISTER &= ~bit_AVX; > > We still need this, but also bypass it for AMD signature. There are > other vendors than Intel and AMD. Mayshao: how about the status of this feature on Zhaoxin product lines? IIRC they support AVX (but disabled by default in GCC for Lujiazui), but we don't know if they make the guarantee about atomicity of 16B aligned access. > > OK with the above addition. > > Thanks, > Uros. > > > - } > > -#endif > > /* See the load in load_feat1. */ > > __atomic_store_n (&__libat_feat1, FEAT1_REGISTER, > > __ATOMIC_RELAXED); > > return FEAT1_REGISTER; > > > > Jakub > >
On Mon, Nov 14, 2022 at 04:19:48PM +0800, Xi Ruoyao wrote: > > > --- libatomic/config/x86/init.c.jj 2022-03-17 > > > 18:48:56.708723194 +0100 > > > +++ libatomic/config/x86/init.c 2022-11-13 18:23:26.315440071 -1200 > > > @@ -34,18 +34,6 @@ __libat_feat1_init (void) > > > unsigned int eax, ebx, ecx, edx; > > > FEAT1_REGISTER = 0; > > > __get_cpuid (1, &eax, &ebx, &ecx, &edx); > > > -#ifdef __x86_64__ > > > - if ((FEAT1_REGISTER & (bit_AVX | bit_CMPXCHG16B)) > > > - == (bit_AVX | bit_CMPXCHG16B)) > > > - { > > > - /* Intel SDM guarantees that 16-byte VMOVDQA on 16-byte > > > aligned address > > > - is atomic, but so far we don't have this guarantee from > > > AMD. */ > > > - unsigned int ecx2 = 0; > > > - __get_cpuid (0, &eax, &ebx, &ecx2, &edx); > > > - if (ecx2 != signature_INTEL_ecx) > > > - FEAT1_REGISTER &= ~bit_AVX; > > > > We still need this, but also bypass it for AMD signature. There are > > other vendors than Intel and AMD. > > Mayshao: how about the status of this feature on Zhaoxin product lines? > IIRC they support AVX (but disabled by default in GCC for Lujiazui), but > we don't know if they make the guarantee about atomicity of 16B aligned > access. I did the change on the assumption that only Intel and AMD implement AVX. Looking around, I'm afraid Zhaoxin Zhangjiang/Wudaokou/Lujiazui and VIA Eden C and VIA Nano C CPUs do support AVX too, the question is if they implement CMPXCHG16B too. From what is in i386-common.cc, none of non-Intel CPUs in there have PTA_AVX and only Lujiazui has CX16. But that doesn't need to match what the HW actually does and one can just compile with -mcx16 -mavx -m64 rather than using some -march=whatever. Sure, can change the check so that it checks for AMD too for now and therefore discard the sync.md patch, the question is whom do we talk at Zhaoxin and VIA and if there are any further other CX16+AVX CPUs Jakub
On Mon, 2022-11-14 at 09:34 +0100, Jakub Jelinek wrote: > > Mayshao: how about the status of this feature on Zhaoxin product lines? > > IIRC they support AVX (but disabled by default in GCC for Lujiazui), but > > we don't know if they make the guarantee about atomicity of 16B aligned > > access. > > I did the change on the assumption that only Intel and AMD implement AVX. > Looking around, I'm afraid Zhaoxin Zhangjiang/Wudaokou/Lujiazui > and VIA Eden C and VIA Nano C CPUs do support AVX too, the question is > if they implement CMPXCHG16B too. According to r13-713, at least Lujiazui has CX16. > From what is in i386-common.cc, none of non-Intel CPUs in there have > PTA_AVX and only Lujiazui has CX16. But that doesn't need to match what > the HW actually does and one can just compile with -mcx16 -mavx -m64 > rather than using some -march=whatever. > > Sure, can change the check so that it checks for AMD too for now and > therefore discard the sync.md patch, the question is whom do we talk at > Zhaoxin and VIA and if there are any further other CX16+AVX CPUs > > Jakub >
--- libatomic/config/x86/init.c.jj 2022-03-17 18:48:56.708723194 +0100 +++ libatomic/config/x86/init.c 2022-11-13 18:23:26.315440071 -1200 @@ -34,18 +34,6 @@ __libat_feat1_init (void) unsigned int eax, ebx, ecx, edx; FEAT1_REGISTER = 0; __get_cpuid (1, &eax, &ebx, &ecx, &edx); -#ifdef __x86_64__ - if ((FEAT1_REGISTER & (bit_AVX | bit_CMPXCHG16B)) - == (bit_AVX | bit_CMPXCHG16B)) - { - /* Intel SDM guarantees that 16-byte VMOVDQA on 16-byte aligned address - is atomic, but so far we don't have this guarantee from AMD. */ - unsigned int ecx2 = 0; - __get_cpuid (0, &eax, &ebx, &ecx2, &edx); - if (ecx2 != signature_INTEL_ecx) - FEAT1_REGISTER &= ~bit_AVX; - } -#endif /* See the load in load_feat1. */ __atomic_store_n (&__libat_feat1, FEAT1_REGISTER, __ATOMIC_RELAXED); return FEAT1_REGISTER;