From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1227 invoked by alias); 19 Nov 2017 20:22:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 1207 invoked by uid 89); 19 Nov 2017 20:22:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.7 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=discovering, slick X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 19 Nov 2017 20:22:28 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CEF0EC0587DF; Sun, 19 Nov 2017 20:22:26 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-12.rdu2.redhat.com [10.10.112.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9352617C25; Sun, 19 Nov 2017 20:22:25 +0000 (UTC) Subject: Re: VRP: x+1 and -x cannot be INT_MIN To: Marc Glisse , Richard Biener Cc: GCC Patches References: From: Jeff Law Message-ID: <925b5e19-c9bd-7032-d68d-1bb70376d882@redhat.com> Date: Sun, 19 Nov 2017 21:51:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg01682.txt.bz2 On 11/19/2017 03:41 AM, Marc Glisse wrote: > On Mon, 13 Nov 2017, Richard Biener wrote: > >> On Sat, Nov 11, 2017 at 11:03 PM, Marc Glisse >> wrote: >>> Hello, >>> >>> with undefined overflow, just because we know nothing about one of the >>> arguments of an addition doesn't mean we can't say something about the >>> result. We could constrain more the cases where we replace VR_VARYING >>> with a >>> full VR_RANGE, but I didn't want to duplicate too much logic. >>> >>> The 20040409 testcases were introduced to test an RTL transformation, >>> so I >>> don't feel too bad adding -fwrapv to work around the undefined overflows >>> they exhibit. >>> >>> Bootstrap+regtest on powerpc64le-unknown-linux-gnu. >> >> Index: gcc/testsuite/gcc.c-torture/execute/20040409-1.c >> =================================================================== >> --- gcc/testsuite/gcc.c-torture/execute/20040409-1.c    (revision 254629) >> +++ gcc/testsuite/gcc.c-torture/execute/20040409-1.c    (working copy) >> @@ -1,10 +1,12 @@ >> +/* { dg-options "-fwrapv" } */ >> + >> >> I think you should use dg-additional-options (if that works).  As said >> in the PR >> it would be safest to copy the tests, add -fwrapv and just remove the >> -fno-wrapv >> cases that do not work. >> >> I think a better fix would be in the caller of >> extract_range_from_binary_expr_1, >> like simply always replacing VARYING with [min,max] if either of the two >> ranges is not varying.  In vr_values::extract_range_from_binary_expr >> that is, >> and doing an early out for varying & varying in _1.  Might simplify some >> special case code for other opts as well. > > Like this? I didn't add the early out yet, among other things because I > am tempted to add that pointer_diff_expr can never be the min value. I > didn't see any obvious simplification of other special cases (I only > looked briefly), the other place where we replace VR_VARYING with a full > range is for conversions (unary). I guess I could drop the restriction > to integers with undefined overflow... > > I had to adapt one testcase where for VR_VARYING | [1, 1] we used to > produce ~[0, 0] and now produce [-INT_MAX, INT_MAX]. I am surprised at > how late the transformation now happens (only after removing > __builtin_unreachable, in forwprop3, while trunk currently has it in > evrp IIRC), but I didn't investigate, doesn't seem like the right time > with all the VRP changes going on. > > The tests that moved to -fwrapv are not undefined for all tested values, > just a few, but subdividing further really doesn't seem worth the trouble. > > Bootstrap+regtest on powerpc64le-unknown-linux-gnu. > > 2017-11-20  Marc Glisse  > > gcc/ >     * vr-values.c (extract_range_from_binary_expr): Use a full range >     for VR_VARYING. > > gcc/testsuite/ >     PR testsuite/82951 >     * gcc.c-torture/execute/20040409-1.c: Move invalid tests... >     * gcc.c-torture/execute/20040409-1w.c: ... here with -fwrapv. >     * gcc.c-torture/execute/20040409-2.c: Move invalid tests... >     * gcc.c-torture/execute/20040409-2w.c: ... here with -fwrapv. >     * gcc.c-torture/execute/20040409-3.c: Move invalid tests... >     * gcc.c-torture/execute/20040409-3w.c: ... here with -fwrapv. >     * gcc.dg/tree-ssa/cmpmul-1.c: Tweak condition. >     * gcc.dg/tree-ssa/vrp118.c: New file. > Slick. I wish I had thought of this last year -- I think it would have made a BZ I was looking at easier to tackle. Richi's already engaged on the issue, so I'll let him handle the review side, I just wanted to note that I see value in discovering these ranges. Jeff