From patchwork Thu Aug 3 19:34:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bill Schmidt X-Patchwork-Id: 797429 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-459784-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="NgJkRQgh"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xNgHN4xcGz9s78 for ; Fri, 4 Aug 2017 05:34:38 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:from:to:cc:references:date:mime-version:in-reply-to :content-type:content-transfer-encoding:message-id; q=dns; s= default; b=wGwIUfb5UX6bWJrQVbClQgkEFluFSbpG3M+W/Mb9GVYVOlvSE8R71 qsh+xfg9AEU4zJYUlI4Gy0MMzf0Ve8zQJpPcbjy/kE29PMU7MBB1YJuwDNPIIlq4 Bna7WGqiuDx8NJxT0CKZqGyIWFC1HZoFet7EYef0IESxg1j+UWvuSo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:from:to:cc:references:date:mime-version:in-reply-to :content-type:content-transfer-encoding:message-id; s=default; bh=SX5FW0ZhSufF7mDRbQ7gnJQtS7g=; b=NgJkRQghUcP53tuowHotlPiAsvEO Ucp6pFUdrIK8NrEJnKhs3cTz4FO8q96XF6XoWls2P6j8RUi3eIkclKFtUTj65/qe XUuLmukiVAbvZfgpmToAFVDURG8c76MMRvLg/WSvVvIiWQdtN/L24CecKI4Ge6vS Q4w2WMOIM2Y1KRc= Received: (qmail 120437 invoked by alias); 3 Aug 2017 19:34:31 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 119004 invoked by uid 89); 3 Aug 2017 19:34:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=incorporated X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Aug 2017 19:34:28 +0000 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v73JYIoW026915 for ; Thu, 3 Aug 2017 15:34:26 -0400 Received: from e18.ny.us.ibm.com (e18.ny.us.ibm.com [129.33.205.208]) by mx0a-001b2d01.pphosted.com with ESMTP id 2c47asgjyt-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 03 Aug 2017 15:34:26 -0400 Received: from localhost by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 3 Aug 2017 15:34:24 -0400 Received: from b01cxnp23033.gho.pok.ibm.com (9.57.198.28) by e18.ny.us.ibm.com (146.89.104.205) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 3 Aug 2017 15:34:23 -0400 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v73JYNQe59900034; Thu, 3 Aug 2017 19:34:23 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8A1E812403F; Thu, 3 Aug 2017 15:31:47 -0400 (EDT) Received: from bigmac.rchland.ibm.com (unknown [9.10.86.172]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP id 5400F124035; Thu, 3 Aug 2017 15:31:47 -0400 (EDT) Subject: Re: [PATCH] Fix PR81503 (SLSR invalid fold) From: Bill Schmidt To: Jakub Jelinek Cc: GCC Patches , Richard Biener References: <7bd4dcbb-cce0-82bb-b938-ffd85dd0e72a@linux.vnet.ibm.com> <20170803162022.GB2123@tucnak> <20170803163950.GD2123@tucnak> Date: Thu, 3 Aug 2017 14:34:22 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: X-TM-AS-GCONF: 00 x-cbid: 17080319-0044-0000-0000-00000377D981 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007478; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000216; SDB=6.00897095; UDB=6.00448839; IPR=6.00677278; BA=6.00005509; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016519; XFM=3.00000015; UTC=2017-08-03 19:34:24 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17080319-0045-0000-0000-000007A5EC59 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-08-03_09:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1708030300 X-IsSubscribed: yes Hi, Here's v2 of the patch with Jakub's suggestions incorporated. Bootstrapped and tested on powerpc64le-linux-gnu with no regressions. Is this ok for trunk? Eventually this should be backported to all active releases as well. Ok for that after a week or so of burn-in? (And after 7.2, I imagine.) Thanks, Bill [gcc] 2017-08-03 Bill Schmidt Jakub Jelinek PR tree-optimization/81503 * gimple-ssa-strength-reduction.c (replace_mult_candidate): Ensure folded constant fits in the target type. [gcc/testsuite] 2017-08-03 Bill Schmidt Jakub Jelinek PR tree-optimization/81503 * gcc.c-torture/execute/pr81503.c: New file. On 8/3/17 1:02 PM, Bill Schmidt wrote: >> On Aug 3, 2017, at 11:39 AM, Jakub Jelinek wrote: >> >> On Thu, Aug 03, 2017 at 11:29:44AM -0500, Bill Schmidt wrote: >>>> And, wouldn't it be more readable to use: >>>> && (TYPE_UNSIGNED (target_type) >>>> ? (wi::eq_p (bump, wi::zext (bump, prec)) >>>> || wi::eq_p (bump + wi::to_widest (maxval) + 1, >>>> wi::zext (bump, prec))) >>>> : wi::eq_p (bump, wi::sext (bump, prec))) >>>> ? >>> Probably. As noted, it's all becoming a bit unreadable with too >>> much negative logic in a long conditional, so I want to clean that >>> up in a follow-up. >>> >>>> For TYPE_UNSIGNED, do you actually need any restriction? >>>> What kind of bump values are wrong for unsigned types and why? >>> If the value of the bump is actually larger than the precision of the >>> type (not likely except for quite small types), say 2 * (maxval + 1) >>> which is congruent to 0, the replacement is wrong. >> Ah, ok. Anyway, for unsigned type, perhaps it could be written as: >> && (TYPE_UNSIGNED (target_type) >> ? wi::eq_p (wi::neg_p (bump) ? bump + wi::to_widest (maxval) + 1 >> : bump, wi::zext (bump, prec)) >> : wi::eq_p (bump, wi::sext (bump, prec))) >> I mean, if bump >= 0, then the bump + wi::to_widest (maxval) + 1 >> value has no chance to be equal to zero extended bump, and >> for bump < 0 only that one has a chance. > Yeah, that's true. And arguably my case for the really large bump > causing problems is kind of thin, because the program is probably > already broken in that case anyway. But I think I will sleep better > having the check in there, as somebody other than SLSR will catch > the bug then. ;-) > > Thanks for all the help with this one. These corner cases are > always tricky, and I appreciate the extra eyeballs. > > Bill > >> Jakub >> Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 250791) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2074,6 +2074,10 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ { tree target_type = TREE_TYPE (gimple_assign_lhs (c->cand_stmt)); enum tree_code cand_code = gimple_assign_rhs_code (c->cand_stmt); + unsigned int prec = TYPE_PRECISION (target_type); + tree maxval = (POINTER_TYPE_P (target_type) + ? TYPE_MAX_VALUE (sizetype) + : TYPE_MAX_VALUE (target_type)); /* It is highly unlikely, but possible, that the resulting bump doesn't fit in a HWI. Abandon the replacement @@ -2082,6 +2086,17 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ types but allows for safe negation without twisted logic. */ if (wi::fits_shwi_p (bump) && bump.to_shwi () != HOST_WIDE_INT_MIN + /* It is more likely that the bump doesn't fit in the target + type, so check whether constraining it to that type changes + the value. For a signed type, the value mustn't change. + For an unsigned type, the value may only change to a + congruent value (for negative bumps). */ + && (TYPE_UNSIGNED (target_type) + ? wi::eq_p (wi::neg_p (bump) + ? bump + wi::to_widest (maxval) + 1 + : bump, + wi::zext (bump, prec)) + : wi::eq_p (bump, wi::sext (bump, prec))) /* It is not useful to replace casts, copies, negates, or adds of an SSA name and a constant. */ && cand_code != SSA_NAME Index: gcc/testsuite/gcc.c-torture/execute/pr81503.c =================================================================== --- gcc/testsuite/gcc.c-torture/execute/pr81503.c (nonexistent) +++ gcc/testsuite/gcc.c-torture/execute/pr81503.c (working copy) @@ -0,0 +1,15 @@ +unsigned short a = 41461; +unsigned short b = 3419; +int c = 0; + +void foo() { + if (a + b * ~(0 != 5)) + c = -~(b * ~(0 != 5)) + 2147483647; +} + +int main() { + foo(); + if (c != 2147476810) + return -1; + return 0; +}