From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id 207A8385C414 for ; Sun, 11 Jul 2021 09:48:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 207A8385C414 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=vHhxROt0QczJN6NCUVa6HNSVKwW1NA3AXawdI/mAnvw=; b=BYmBADNdjCLww/TOrJJq/anhma zRtakpMDI3shu1q13ENzQ6LAtSXgGZ/6JDwAc8bKRdL3aMorfdOqzReTyPGYcJJuOCFd4wyDyHTJr yinaqPKcOyHK1H1s2PRrhitbtEpuRvxeYLDy42/rY0x5LKGKbbDLIbcHPOIHtXMEfOFTy5h33BE9L EhicRs5/RHeoV6fB3C+WhajjFXkmMSmNSkMdwaBuXNnu413J7NijOHPkRFjPd0TTb0bmCTODuPdlq 8DdIzc2ErRB/oRKYqc0EUCtndADYd2q4QrVVHTocPtbJWXB7lWHH3rBz3HbzwekxQuum32G1aarpK Vg8XYidQ==; Received: from host86-169-60-32.range86-169.btcentralplus.com ([86.169.60.32]:53842 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1m2W4S-0001VN-3l; Sun, 11 Jul 2021 05:48:20 -0400 From: "Roger Sayle" To: "'GCC Patches'" Subject: [PATCH] PR tree-optimization/101403: Incorrect folding of ((T)bswap(x))>>C Date: Sun, 11 Jul 2021 10:48:17 +0100 Message-ID: <00de01d77639$e1302040$a39060c0$@nextmovesoftware.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_00DF_01D77642.42F6F940" X-Mailer: Microsoft Outlook 16.0 Thread-Index: Add2OTnLxPnEmrZ3RMCWL5Fs78g8tA== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Jul 2021 09:48:22 -0000 This is a multipart message in MIME format. ------=_NextPart_000_00DF_01D77642.42F6F940 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit My sincere apologies for the breakage. My recent patch to fold bswapN(x)>>C where the constant C was large enough that the result only contains bits from the low byte, and can therefore avoid the byte swap contains a minor logic error. The pattern contains a convert? allowing an extension to occur between the bswap and the shift. The logic is correct if there's no extension, or the extension has the same sign as the shift, but I'd mistakenly convinced myself that these couldn't have different signedness. (T)bswap16(x)>>12 is (T)((unsigned char)x>>4) or (T)((signed char)x>>4). The bug is that for zero-extensions to signed type T, we need to use the unsigned char variant [the signedness of the byte shift is not (always) the same as the signedness of T and the original shift]. Then because I'm now paranoid, I've also added a clause to handle the hypothetical (but in practice impossible) sign-extension to an unsigned type T, which can implemented as (T)(x<<8)>>12. This patch has been tested on x86_64-pc-linux-gnu with a "make bootstrap" and "make -k check" with no new failures, and a new testcase to confirm it fixes the regression. Ok for mainline? 2021-07-11 Roger Sayle gcc/ChangeLog PR tree-optimization/101403 * gcc/match.pd ((T)bswap(X)>>C): Correctly handle cases where signedness of the shift is not the same as the signedness of the type extension. gcc/testsuite/ChangeLog PR tree-optimization/101403 * gcc.dg/pr101403.c: New test case. Sorry again, Roger -- Roger Sayle NextMove Software Cambridge, UK ------=_NextPart_000_00DF_01D77642.42F6F940 Content-Type: text/plain; name="patchb2.txt" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="patchb2.txt" diff --git a/gcc/match.pd b/gcc/match.pd=0A= index 30680d4..beb8d27 100644=0A= --- a/gcc/match.pd=0A= +++ b/gcc/match.pd=0A= @@ -3659,19 +3659,31 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)=0A= {=0A= unsigned HOST_WIDE_INT prec =3D TYPE_PRECISION (TREE_TYPE (@2));=0A= unsigned HOST_WIDE_INT bits =3D tree_to_uhwi (@1);=0A= + /* If the bswap was extended before the original shift, this=0A= + byte (shift) has the sign of the extension, not the sign of=0A= + the original shift. */=0A= + tree st =3D TYPE_PRECISION (type) > prec ? TREE_TYPE (@2) : type;=0A= }=0A= - (if (bits + 8 =3D=3D prec)=0A= - (if (TYPE_UNSIGNED (type))=0A= - (convert (convert:unsigned_char_type_node @0))=0A= - (convert (convert:signed_char_type_node @0)))=0A= - (if (bits < prec && bits + 8 > prec)=0A= - (with =0A= - {=0A= - tree nst =3D build_int_cst (integer_type_node, bits & 7);=0A= - tree bt =3D TYPE_UNSIGNED (type) ? unsigned_char_type_node=0A= - : signed_char_type_node;=0A= - }=0A= - (convert (rshift:bt (convert:bt @0) {nst;}))))))))=0A= + /* Special case: logical right shift of sign-extended bswap.=0A= + (unsigned)(short)bswap16(x)>>12 is (unsigned)((short)x<<8)>>12. */=0A= + (if (TYPE_PRECISION (type) > prec=0A= + && !TYPE_UNSIGNED (TREE_TYPE (@2))=0A= + && TYPE_UNSIGNED (type)=0A= + && bits < prec && bits + 8 >=3D prec)=0A= + (with { tree nst =3D build_int_cst (integer_type_node, prec - 8); = }=0A= + (rshift (convert (lshift:st (convert:st @0) {nst;})) @1))=0A= + (if (bits + 8 =3D=3D prec)=0A= + (if (TYPE_UNSIGNED (st))=0A= + (convert (convert:unsigned_char_type_node @0))=0A= + (convert (convert:signed_char_type_node @0)))=0A= + (if (bits < prec && bits + 8 > prec)=0A= + (with =0A= + {=0A= + tree nst =3D build_int_cst (integer_type_node, bits & 7);=0A= + tree bt =3D TYPE_UNSIGNED (st) ? unsigned_char_type_node=0A= + : signed_char_type_node;=0A= + }=0A= + (convert (rshift:bt (convert:bt @0) {nst;})))))))))=0A= /* bswap(x) & C1 can sometimes be simplified to (x >> C2) & C1. */=0A= (simplify=0A= (bit_and (convert? (bswap@2 @0)) INTEGER_CST@1)=0A= ------=_NextPart_000_00DF_01D77642.42F6F940 Content-Type: text/plain; name="pr101403.c" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="pr101403.c" /* { dg-do run } */=0A= /* { dg-options "-O2" } */=0A= unsigned int foo (unsigned int a)=0A= {=0A= unsigned int u;=0A= unsigned short b =3D __builtin_bswap16 (a);=0A= return b >> (u, 12);=0A= }=0A= =0A= int main (void)=0A= {=0A= unsigned int x =3D foo (0x80);=0A= if (x !=3D 0x0008)=0A= __builtin_abort ();=0A= return 0;=0A= }=0A= =0A= ------=_NextPart_000_00DF_01D77642.42F6F940--