From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53982 invoked by alias); 4 Oct 2019 20:57:46 -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 53973 invoked by uid 89); 4 Oct 2019 20:57:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= 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; Fri, 04 Oct 2019 20:57:44 +0000 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 mx1.redhat.com (Postfix) with ESMTPS id 876393067294; Fri, 4 Oct 2019 20:57:42 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-68.rdu2.redhat.com [10.10.112.68]) by smtp.corp.redhat.com (Postfix) with ESMTP id E4D4A5C1B5; Fri, 4 Oct 2019 20:57:40 +0000 (UTC) Subject: Re: [PATCH] Fix up sqrt(x) < c and sqrt(x) >= c match.pd folding (PR tree-optimization/91734, take 2) To: Jakub Jelinek , Richard Biener Cc: "Joseph S. Myers" , gcc-patches@gcc.gnu.org References: <20190914004014.GE25273@laptop.zalov.cz> <20190921061413.GC15914@tucnak> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <1a0d81b8-008a-1b0f-622d-fec68c4cb12b@redhat.com> Date: Fri, 04 Oct 2019 20:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190921061413.GC15914@tucnak> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00386.txt.bz2 On 9/21/19 12:14 AM, Jakub Jelinek wrote: > On Mon, Sep 16, 2019 at 08:56:58AM +0200, Richard Biener wrote: >>> As mentioned in the PR, the sqrt (x) < c optimization into x < c*c >>> sometimes breaks the boundary case, if c2=c*c is inexact then in some cases >>> we need to optimize it into x <= c*c rather than x < c*c. The original >>> bugreport is when c is small and c2 is 0.0, then obviously we need <= 0.0 >>> rather than < 0.0, but the testcase includes another example where it makes >>> a difference, plus has a >= testcase too. >>> >>> Bootstrapped/regtested on powerpc64le-linux, ok for trunk? >> >> I was hoping Joseph might chime in here... anyway, does this assume >> round-to-nearest or does it work with round to +-Inf as well? I >> realize this all is under flag_unsafe_math_optimizations, but >> this flag is notoriously underspecified... So the question is >> whether we should disable the transform if c*c isn't exact and >> flag_rounding_math? The transform also doesn't seem to guard >> against isnan (c) (-funsafe-math-optimizations sets >> -fno-trapping-math and -fno-signed-zeros but not -ffinite-math-only >> or disables itself on -frounding-math) > > Here is an updated patch, which on top of the previous patch: > 1) punts for -frounding-math > 2) punts for sqrt comparisons against NaN constant > 3) for the c*c inexact also handles the other two comparisons that > apparently need to be handled too > 4) for all 4 comparisons also checks nexttoward (c2, 0.0) or nexttoward (c2, > inf) depending on the comparison kind, because as Joseph correctly noted, > with rounding to nearest up to 3 different floating point values can have > the same sqrt result, and if c2 is the middle one from them, we need to use > the 1 ulp smaller or larger one in the comparison > 5) had to adjust the testcase, because while it worked fine on powerpc64le, > on x86_64 if the test is linked with -ffast-math/-Ofast etc., crtfastmath.o > is linked in and subnormals are flushed to zero, which is not what we want > for the testcase (at least for a subset of the tests). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > BTW, I've used attached programs to look for the problematic cases on random > float/doubles and the cases the patch handles seem to be the only > problematic ones, there is never need to go further than one nexttoward to 0 > or inf. > > 2019-09-21 Jakub Jelinek > > PR tree-optimization/91734 > * generic-match-head.c: Include fold-const-call.h. > * match.pd (sqrt(x) cmp c): Check the boundary value and > in case inexact computation of c*c affects comparison of the boundary, > turn LT_EXPR into LE_EXPR, GE_EXPR into GT_EXPR, LE_EXPR into LT_EXPR > or GT_EXPR into GE_EXPR. Punt for sqrt comparisons against NaN and > for -frounding-math. For c2, try the next smaller or larger floating > point constant depending on comparison code and if it has the same > sqrt as c2, use it instead of c2. > > * gcc.dg/pr91734.c: New test. OK. One might argue that some of this code needs to be refactored into functions to make visual parsing simpler. But I'm not going to insist on it right now. jeff