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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 65A7E3851C2B for ; Tue, 10 Nov 2020 23:48:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 65A7E3851C2B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-580-EvjPyE7nMw-svpZkemgXDg-1; Tue, 10 Nov 2020 18:48:12 -0500 X-MC-Unique: EvjPyE7nMw-svpZkemgXDg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4EA7564082; Tue, 10 Nov 2020 23:48:11 +0000 (UTC) Received: from localhost.localdomain (ovpn-114-181.phx2.redhat.com [10.3.114.181]) by smtp.corp.redhat.com (Postfix) with ESMTP id B22555C1C4; Tue, 10 Nov 2020 23:48:10 +0000 (UTC) Subject: Re: [PATCH] Better __ashlDI3, __ashrDI3 and __lshrDI3 functions, plus fixed __bswapsi2 function To: Stefan Kanthak , gcc-patches@gcc.gnu.org References: <116F1589A8244FB494091BCD4E6398AB@H270> From: Jeff Law Message-ID: Date: Tue, 10 Nov 2020 16:48:10 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: <116F1589A8244FB494091BCD4E6398AB@H270> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, HTML_MESSAGE, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Nov 2020 23:48:16 -0000 On 11/10/20 10:59 AM, Stefan Kanthak via Gcc-patches wrote: > The implementation of the __ashlDI3(), __ashrDI3() and __lshrDI3() functions > is rather bad, it yields bad machine code at least on i386 and AMD64. > Since GCC knows how to shift integers twice the register size these functions > can be written as one-liners. > > The implementation of the __bswapsi2() function uses SIGNED instead of > unsigned mask values; cf. __bswapdi2() > > Stefan Kanthak > > libgcc2.diff > > --- -/libgcc/libgcc2.h > +++ +/libgcc/libgcc2.h > @@ -391,7 +391,7 @@ > extern DWtype __negdi2 (DWtype); > #endif > > -extern DWtype __lshrdi3 (DWtype, shift_count_type); > +extern UDWtype __lshrdi3 (UDWtype, shift_count_type); > extern DWtype __ashldi3 (DWtype, shift_count_type); > extern DWtype __ashrdi3 (DWtype, shift_count_type); > > --- -/libgcc/libgcc2.c > +++ +/libgcc/libgcc2.c > @@ -398,30 +398,10 @@ > /* Unless shift functions are defined with full ANSI prototypes, > parameter b will be promoted to int if shift_count_type is smaller than an int. */ > #ifdef L_lshrdi3 > -DWtype > -__lshrdi3 (DWtype u, shift_count_type b) > +UDWtype > +__lshrdi3 (UDWtype u, shift_count_type b) As has been pointed out, you can't implement these routines by doing the operation in the same mode as the argument.  It's potentially self-recursive. THe whole point of these routines is to provide double-word capabilities on targets that don't have intrinsic double-word capabilities. That's why they're written in a non-obvious way using word sized operations. > > @@ -486,10 +425,10 @@ > SItype > __bswapsi2 (SItype u) > { > - return ((((u) & 0xff000000) >> 24) > - | (((u) & 0x00ff0000) >> 8) > - | (((u) & 0x0000ff00) << 8) > - | (((u) & 0x000000ff) << 24)); > + return ((((u) & 0xff000000u) >> 24) > + | (((u) & 0x00ff0000u) >> 8) > + | (((u) & 0x0000ff00u) << 8) > + | (((u) & 0x000000ffu) << 24)); What's the point of this change?  I'm not sure how the signedness of the constant really matters here. jeff