Message ID | 20220917082403.1573721-1-aldyh@redhat.com |
---|---|
State | New, archived |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5044:0:0:0:0:0 with SMTP id h4csp112634wrt; Sat, 17 Sep 2022 01:25:29 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6JIxNYOVzj7hnf15Ib5dv3ukv7SrDiQ06g9m1cijdgvJIwkj8+FqFNBBoJ/uR+T9jOeS3h X-Received: by 2002:aa7:cb83:0:b0:443:3f15:84fe with SMTP id r3-20020aa7cb83000000b004433f1584femr7215925edt.17.1663403128737; Sat, 17 Sep 2022 01:25:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1663403128; cv=none; d=google.com; s=arc-20160816; b=NL/VR4Nxo3pw2X+0wMrgzMhxVCDGFIA1LuAhzHoOe0p6lt4aoYB3zXNw0dEA6YKY5g tOcYEz1GuxEuhTmuOgXXSUjd9uL5RQbrocqQ8XtkfqPp5vv19QoA+vP2TercqUUp44XX uZjhvJb4Ju/+a7u/IRIXsuCgJn7LteQYn90pVAMfMQyiN7RmhYfY7Jqng52GT1nq2E1H 9D84MnFCzgLvlYaINd8tIjlUoxFg5+o2IosJ4k6VHPDi/KLxanOgTTgoNbEK3eQLzQ0J 9KkIebEWX1SRoykYB4zHxa5TT5j1yFY5TjL/1nxjDm9YkugIURv4NAIJdv4ZVCsi6BPv J+Bg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=p/YxWrLRTx7kRMCbeW+npAOoYKbpcJ7eGs9cFktgEYo=; b=Jtv+yDe0XGbNr+GFNso2HPdJ4bkuQL8C5ofQIvQ3bm+coRSLqdYfKrMs78Pkcr2KOI vBCZjhc2ulKhdGnhpbkHtiJ+HxJ+0FEJvaM1FP1ny+6vUDLyzUfaF11Whsbj+z1ZS6yW h1o95f+SPl9qs4kl1O8RUkpVZ2SAIDuchm9lyzy6ol117n+9pZmWTfgyB6yUlGvSqklO 5n7GIY4dEQY4ZJEWp8K6YoNbf1eOClYyGAX3ui9uWn7hpNdvpbbB/XoiN/xcYRwQk6zX mOTmsYRLJ7nWUQTirGRzwk5aNzeDkHGGjbTFPe8mVQlwl9RyweUlZ7fd2uLO/XVMxLHN 758w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=DUrMMvqW; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id d9-20020a170906304900b007417c184ea5si17691528ejd.628.2022.09.17.01.25.26 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Sep 2022 01:25:28 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=DUrMMvqW; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C80AF38328EB for <ouuuleilei@gmail.com>; Sat, 17 Sep 2022 08:25:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C80AF38328EB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1663403107; bh=p/YxWrLRTx7kRMCbeW+npAOoYKbpcJ7eGs9cFktgEYo=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=DUrMMvqWolECiUtapfNNAGulJMEfDy6Vt7Mp9BZ/0UDKCBMXvLs5VQH9QRVWXSLK1 MWWCZ63Eg6ZbbP1MRfdHj50T9oJVAq/b7LpZCug9FXgWmq6LpUusx+kE4SbzIJNNGB meBWtTujmgixCj0PFysFbZKzKfSQOLpVzt0iHXMA= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 5D901383177E for <gcc-patches@gcc.gnu.org>; Sat, 17 Sep 2022 08:24:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5D901383177E Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-135-lEKGaKPBOvuL1R81MSGOPQ-1; Sat, 17 Sep 2022 04:24:18 -0400 X-MC-Unique: lEKGaKPBOvuL1R81MSGOPQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 37DCA185A78F; Sat, 17 Sep 2022 08:24:18 +0000 (UTC) Received: from abulafia.quesejoda.com (unknown [10.39.192.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C557C492CA2; Sat, 17 Sep 2022 08:24:17 +0000 (UTC) Received: from abulafia.quesejoda.com (localhost [127.0.0.1]) by abulafia.quesejoda.com (8.17.1/8.17.1) with ESMTPS id 28H8OFRA1573751 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sat, 17 Sep 2022 10:24:15 +0200 Received: (from aldyh@localhost) by abulafia.quesejoda.com (8.17.1/8.17.1/Submit) id 28H8OFdW1573750; Sat, 17 Sep 2022 10:24:15 +0200 To: Jakub Jelinek <jakub@redhat.com>, Richard Henderson <rth@gcc.gnu.org> Subject: [PATCH] frange: flush denormals to zero for -funsafe-math-optimizations. Date: Sat, 17 Sep 2022 10:24:03 +0200 Message-Id: <20220917082403.1573721-1-aldyh@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Aldy Hernandez <aldyh@redhat.com> Cc: GCC patches <gcc-patches@gcc.gnu.org> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1744204599104227253?= X-GMAIL-MSGID: =?utf-8?q?1744204599104227253?= |
Series |
frange: flush denormals to zero for -funsafe-math-optimizations.
|
|
Commit Message
Aldy Hernandez
Sept. 17, 2022, 8:24 a.m. UTC
Jakub has mentioned that for -funsafe-math-optimizations we may flush denormals to zero, in which case we need to be careful to extend the ranges to the appropriate zero. This patch does exactly that. For a range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] we flush to [+0.0, x]. It is unclear whether we should do this for Alpha, since I believe flushing to zero is the default, and the port requires -mieee for IEEE sanity. If so, perhaps we should add a target hook so backends are free to request flushing to zero. Thoughts? gcc/ChangeLog: * value-range.cc (frange::flush_denormals_to_zero): New. (frange::set): Call flush_denormals_to_zero. * value-range.h (class frange): Add flush_denormals_to_zero. --- gcc/value-range.cc | 24 ++++++++++++++++++++++++ gcc/value-range.h | 1 + 2 files changed, 25 insertions(+)
Comments
On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > denormals to zero, in which case we need to be careful to extend the > ranges to the appropriate zero. This patch does exactly that. For a > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > we flush to [+0.0, x]. > > It is unclear whether we should do this for Alpha, since I believe > flushing to zero is the default, and the port requires -mieee for IEEE > sanity. If so, perhaps we should add a target hook so backends are > free to request flushing to zero. > > Thoughts? I'm not sure what the intention of this is - it effectively results in more conservative ranges for -funsafe-math-optimizations. That is, if -funsafe-math-optimizations says that denormals don't exist and are all zero then doesn't that mean we can instead use the smalles non-denormal number less than (or greater than) zero here? That said, the flushing you do is also "valid" for -fno-unsafe-math-optimizations in case we don't want to deal with denormals in range boundaries. It might also be a correctness requirement in case we don't know how targets handle denormals (IIRC even OS defaults might matter here) so we conservatively have to treat them as being flushed to zero. So ... if we want to be on the "safe" side then please always do this. At the same point you could do if (!HONOR_SIGNED_ZEROS ()) if (real_iszero (&m_max)) { if (real_iszero (&m_min)) m.min.sign = 1; m_max.sign = 1; } else if (real_iszero (&m_min)) m_min.sign = 0; so that we canonicalize a zero bound so that the sign is known for a range. Richard. > gcc/ChangeLog: > > * value-range.cc (frange::flush_denormals_to_zero): New. > (frange::set): Call flush_denormals_to_zero. > * value-range.h (class frange): Add flush_denormals_to_zero. > --- > gcc/value-range.cc | 24 ++++++++++++++++++++++++ > gcc/value-range.h | 1 + > 2 files changed, 25 insertions(+) > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > index 67d5d7fa90f..f285734f0e0 100644 > --- a/gcc/value-range.cc > +++ b/gcc/value-range.cc > @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2) > return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2)); > } > > +// Flush denormal endpoints to the appropriate 0.0. > + > +void > +frange::flush_denormals_to_zero () > +{ > + if (undefined_p () || known_isnan ()) > + return; > + > + // Flush [x, -DENORMAL] to [x, -0.0]. > + if (real_isdenormal (&m_max) && real_isneg (&m_max)) > + { > + m_max = dconst0; > + if (HONOR_SIGNED_ZEROS (m_type)) > + m_max.sign = 1; > + } > + // Flush [+DENORMAL, x] to [+0.0, x]. > + if (real_isdenormal (&m_min) && !real_isneg (&m_min)) > + m_min = dconst0; > +} > + > // Setter for franges. > > void > @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind) > gcc_checking_assert (tree_compare (LE_EXPR, min, max)); > > normalize_kind (); > + > + if (flag_unsafe_math_optimizations) > + flush_denormals_to_zero (); > + > if (flag_checking) > verify_range (); > } > diff --git a/gcc/value-range.h b/gcc/value-range.h > index 3a401f3e4e2..795b1f00fdc 100644 > --- a/gcc/value-range.h > +++ b/gcc/value-range.h > @@ -327,6 +327,7 @@ private: > bool union_nans (const frange &); > bool intersect_nans (const frange &); > bool combine_zeros (const frange &, bool union_p); > + void flush_denormals_to_zero (); > > tree m_type; > REAL_VALUE_TYPE m_min; > -- > 2.37.1 >
On Mon, Sep 19, 2022 at 09:37:22AM +0200, Richard Biener wrote: > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > > denormals to zero, in which case we need to be careful to extend the > > ranges to the appropriate zero. This patch does exactly that. For a > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > > we flush to [+0.0, x]. > > > > It is unclear whether we should do this for Alpha, since I believe > > flushing to zero is the default, and the port requires -mieee for IEEE > > sanity. If so, perhaps we should add a target hook so backends are > > free to request flushing to zero. > > > > Thoughts? > > I'm not sure what the intention of this is - it effectively results in > more conservative ranges for -funsafe-math-optimizations. That is, > if -funsafe-math-optimizations says that denormals don't exist and > are all zero then doesn't that mean we can instead use the smalles > non-denormal number less than (or greater than) zero here? Yes, with -funsafe-math-optimizations we will sometimes have slightly larger ranges, while when we know or assume we know that denormals will be honored we can be more precise. It is similar to the -fno-signed-zeros case, when we are more relaxed and say we don't care about sign of zeros and optimizations can be performed to leave the sign of zero in whatever state they like, we can't have [+0.0, xx] or [xx, -0.0] ranges and need to extend them to [-0.0, xx] or [xx, +0.0]. The honor denormals case is similar, if there is the possibility of having denormals flushed to zero, [+denormal, xx] needs to be extended to [+0.0, xx] and [xx, -denormal] to [xx, -0.0]. Now, I think we just should add a separate option whether denormals are honored or not, because always extending the ranges with denormals on the boundaries might prevent some useful optimizations. If we add such an option, it could default to not honoring them for -funsafe-math-optimizations, and say on alpha it should default to not honoring them by default always unless -mieee option is used, but all of this if the user didn't use the option explicitly, in that case follow what the user said. That way if users know they set DTZ flag themselves or use libraries that do so, they can tell the compiler about it. > That said, the flushing you do is also "valid" for > -fno-unsafe-math-optimizations > in case we don't want to deal with denormals in range boundaries. It is always valid to extend the range more, but could be harmful to optimizations. Say if we have then x > 0.0 test... Of course, without -ffast-math we won't DCE various floating point operations because they can raise exceptions, but at least we can DCE the basic block after it that depends on x > 0.0. Jakub
On Mon, Sep 19, 2022 at 9:37 AM Richard Biener <richard.guenther@gmail.com> wrote: > > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > > denormals to zero, in which case we need to be careful to extend the > > ranges to the appropriate zero. This patch does exactly that. For a > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > > we flush to [+0.0, x]. > > > > It is unclear whether we should do this for Alpha, since I believe > > flushing to zero is the default, and the port requires -mieee for IEEE > > sanity. If so, perhaps we should add a target hook so backends are > > free to request flushing to zero. > > > > Thoughts? > > I'm not sure what the intention of this is - it effectively results in > more conservative ranges for -funsafe-math-optimizations. That is, > if -funsafe-math-optimizations says that denormals don't exist and > are all zero then doesn't that mean we can instead use the smalles > non-denormal number less than (or greater than) zero here? > > That said, the flushing you do is also "valid" for > -fno-unsafe-math-optimizations > in case we don't want to deal with denormals in range boundaries. > > It might also be a correctness requirement in case we don't know how > targets handle denormals (IIRC even OS defaults might matter here) so > we conservatively have to treat them as being flushed to zero. Actually, rth suggested we always flush to zero because we don't know what the target would do. Again, I'm happy to do whatever you agree on. I have no opinion. My main goal here is correctness. > > So ... if we want to be on the "safe" side then please always do this. > > At the same point you could do > > if (!HONOR_SIGNED_ZEROS ()) > if (real_iszero (&m_max)) > { > if (real_iszero (&m_min)) > m.min.sign = 1; > m_max.sign = 1; > } But wouldn't that set [-0.0, -0.0] when encountering [+0, +0] ?? > else if (real_iszero (&m_min)) > m_min.sign = 0; Jakub actually suggested something completely different...just set +0 always for !HONOR_SIGNED_ZEROS. Aldy > > so that we canonicalize a zero bound so that the sign is known for a range. > > Richard. > > > gcc/ChangeLog: > > > > * value-range.cc (frange::flush_denormals_to_zero): New. > > (frange::set): Call flush_denormals_to_zero. > > * value-range.h (class frange): Add flush_denormals_to_zero. > > --- > > gcc/value-range.cc | 24 ++++++++++++++++++++++++ > > gcc/value-range.h | 1 + > > 2 files changed, 25 insertions(+) > > > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > > index 67d5d7fa90f..f285734f0e0 100644 > > --- a/gcc/value-range.cc > > +++ b/gcc/value-range.cc > > @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2) > > return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2)); > > } > > > > +// Flush denormal endpoints to the appropriate 0.0. > > + > > +void > > +frange::flush_denormals_to_zero () > > +{ > > + if (undefined_p () || known_isnan ()) > > + return; > > + > > + // Flush [x, -DENORMAL] to [x, -0.0]. > > + if (real_isdenormal (&m_max) && real_isneg (&m_max)) > > + { > > + m_max = dconst0; > > + if (HONOR_SIGNED_ZEROS (m_type)) > > + m_max.sign = 1; > > + } > > + // Flush [+DENORMAL, x] to [+0.0, x]. > > + if (real_isdenormal (&m_min) && !real_isneg (&m_min)) > > + m_min = dconst0; > > +} > > + > > // Setter for franges. > > > > void > > @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind) > > gcc_checking_assert (tree_compare (LE_EXPR, min, max)); > > > > normalize_kind (); > > + > > + if (flag_unsafe_math_optimizations) > > + flush_denormals_to_zero (); > > + > > if (flag_checking) > > verify_range (); > > } > > diff --git a/gcc/value-range.h b/gcc/value-range.h > > index 3a401f3e4e2..795b1f00fdc 100644 > > --- a/gcc/value-range.h > > +++ b/gcc/value-range.h > > @@ -327,6 +327,7 @@ private: > > bool union_nans (const frange &); > > bool intersect_nans (const frange &); > > bool combine_zeros (const frange &, bool union_p); > > + void flush_denormals_to_zero (); > > > > tree m_type; > > REAL_VALUE_TYPE m_min; > > -- > > 2.37.1 > > >
On Mon, Sep 19, 2022 at 3:04 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > On Mon, Sep 19, 2022 at 9:37 AM Richard Biener > <richard.guenther@gmail.com> wrote: > > > > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > > > denormals to zero, in which case we need to be careful to extend the > > > ranges to the appropriate zero. This patch does exactly that. For a > > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > > > we flush to [+0.0, x]. > > > > > > It is unclear whether we should do this for Alpha, since I believe > > > flushing to zero is the default, and the port requires -mieee for IEEE > > > sanity. If so, perhaps we should add a target hook so backends are > > > free to request flushing to zero. > > > > > > Thoughts? > > > > I'm not sure what the intention of this is - it effectively results in > > more conservative ranges for -funsafe-math-optimizations. That is, > > if -funsafe-math-optimizations says that denormals don't exist and > > are all zero then doesn't that mean we can instead use the smalles > > non-denormal number less than (or greater than) zero here? > > > > That said, the flushing you do is also "valid" for > > -fno-unsafe-math-optimizations > > in case we don't want to deal with denormals in range boundaries. > > > > It might also be a correctness requirement in case we don't know how > > targets handle denormals (IIRC even OS defaults might matter here) so > > we conservatively have to treat them as being flushed to zero. > > Actually, rth suggested we always flush to zero because we don't know > what the target would do. Again, I'm happy to do whatever you agree More specifically, we don't know what the OS will do, so we either should have a flag of some kind set to TRUE by default when unsafe math optimizations, or always assume denormal flushing could happen. Again, no opinion. Up to y'all. Aldy > on. I have no opinion. My main goal here is correctness. > > > > > So ... if we want to be on the "safe" side then please always do this. > > > > At the same point you could do > > > > if (!HONOR_SIGNED_ZEROS ()) > > if (real_iszero (&m_max)) > > { > > if (real_iszero (&m_min)) > > m.min.sign = 1; > > m_max.sign = 1; > > } > > But wouldn't that set [-0.0, -0.0] when encountering [+0, +0] ?? > > > else if (real_iszero (&m_min)) > > m_min.sign = 0; > > Jakub actually suggested something completely different...just set +0 > always for !HONOR_SIGNED_ZEROS. > > Aldy > > > > > so that we canonicalize a zero bound so that the sign is known for a range. > > > > Richard. > > > > > gcc/ChangeLog: > > > > > > * value-range.cc (frange::flush_denormals_to_zero): New. > > > (frange::set): Call flush_denormals_to_zero. > > > * value-range.h (class frange): Add flush_denormals_to_zero. > > > --- > > > gcc/value-range.cc | 24 ++++++++++++++++++++++++ > > > gcc/value-range.h | 1 + > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > > > index 67d5d7fa90f..f285734f0e0 100644 > > > --- a/gcc/value-range.cc > > > +++ b/gcc/value-range.cc > > > @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2) > > > return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2)); > > > } > > > > > > +// Flush denormal endpoints to the appropriate 0.0. > > > + > > > +void > > > +frange::flush_denormals_to_zero () > > > +{ > > > + if (undefined_p () || known_isnan ()) > > > + return; > > > + > > > + // Flush [x, -DENORMAL] to [x, -0.0]. > > > + if (real_isdenormal (&m_max) && real_isneg (&m_max)) > > > + { > > > + m_max = dconst0; > > > + if (HONOR_SIGNED_ZEROS (m_type)) > > > + m_max.sign = 1; > > > + } > > > + // Flush [+DENORMAL, x] to [+0.0, x]. > > > + if (real_isdenormal (&m_min) && !real_isneg (&m_min)) > > > + m_min = dconst0; > > > +} > > > + > > > // Setter for franges. > > > > > > void > > > @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind) > > > gcc_checking_assert (tree_compare (LE_EXPR, min, max)); > > > > > > normalize_kind (); > > > + > > > + if (flag_unsafe_math_optimizations) > > > + flush_denormals_to_zero (); > > > + > > > if (flag_checking) > > > verify_range (); > > > } > > > diff --git a/gcc/value-range.h b/gcc/value-range.h > > > index 3a401f3e4e2..795b1f00fdc 100644 > > > --- a/gcc/value-range.h > > > +++ b/gcc/value-range.h > > > @@ -327,6 +327,7 @@ private: > > > bool union_nans (const frange &); > > > bool intersect_nans (const frange &); > > > bool combine_zeros (const frange &, bool union_p); > > > + void flush_denormals_to_zero (); > > > > > > tree m_type; > > > REAL_VALUE_TYPE m_min; > > > -- > > > 2.37.1 > > > > >
On Mon, Sep 19, 2022 at 3:04 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > On Mon, Sep 19, 2022 at 9:37 AM Richard Biener > <richard.guenther@gmail.com> wrote: > > > > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > > > denormals to zero, in which case we need to be careful to extend the > > > ranges to the appropriate zero. This patch does exactly that. For a > > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > > > we flush to [+0.0, x]. > > > > > > It is unclear whether we should do this for Alpha, since I believe > > > flushing to zero is the default, and the port requires -mieee for IEEE > > > sanity. If so, perhaps we should add a target hook so backends are > > > free to request flushing to zero. > > > > > > Thoughts? > > > > I'm not sure what the intention of this is - it effectively results in > > more conservative ranges for -funsafe-math-optimizations. That is, > > if -funsafe-math-optimizations says that denormals don't exist and > > are all zero then doesn't that mean we can instead use the smalles > > non-denormal number less than (or greater than) zero here? > > > > That said, the flushing you do is also "valid" for > > -fno-unsafe-math-optimizations > > in case we don't want to deal with denormals in range boundaries. > > > > It might also be a correctness requirement in case we don't know how > > targets handle denormals (IIRC even OS defaults might matter here) so > > we conservatively have to treat them as being flushed to zero. > > Actually, rth suggested we always flush to zero because we don't know > what the target would do. Again, I'm happy to do whatever you agree > on. I have no opinion. My main goal here is correctness. Yes, I think we should do this. > > > > So ... if we want to be on the "safe" side then please always do this. > > > > At the same point you could do > > > > if (!HONOR_SIGNED_ZEROS ()) > > if (real_iszero (&m_max)) > > { > > if (real_iszero (&m_min)) > > m.min.sign = 1; > > m_max.sign = 1; > > } > > But wouldn't that set [-0.0, -0.0] when encountering [+0, +0] ?? Yeah, that's my laziness not adding a special case for m_min == m_max. > > > else if (real_iszero (&m_min)) > > m_min.sign = 0; > > Jakub actually suggested something completely different...just set +0 > always for !HONOR_SIGNED_ZEROS. Hmm, but the [-1, -0.] with known sign becomes [-1, +0.] with unknown sign? Richard. > Aldy > > > > > so that we canonicalize a zero bound so that the sign is known for a range. > > > > Richard. > > > > > gcc/ChangeLog: > > > > > > * value-range.cc (frange::flush_denormals_to_zero): New. > > > (frange::set): Call flush_denormals_to_zero. > > > * value-range.h (class frange): Add flush_denormals_to_zero. > > > --- > > > gcc/value-range.cc | 24 ++++++++++++++++++++++++ > > > gcc/value-range.h | 1 + > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > > > index 67d5d7fa90f..f285734f0e0 100644 > > > --- a/gcc/value-range.cc > > > +++ b/gcc/value-range.cc > > > @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2) > > > return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2)); > > > } > > > > > > +// Flush denormal endpoints to the appropriate 0.0. > > > + > > > +void > > > +frange::flush_denormals_to_zero () > > > +{ > > > + if (undefined_p () || known_isnan ()) > > > + return; > > > + > > > + // Flush [x, -DENORMAL] to [x, -0.0]. > > > + if (real_isdenormal (&m_max) && real_isneg (&m_max)) > > > + { > > > + m_max = dconst0; > > > + if (HONOR_SIGNED_ZEROS (m_type)) > > > + m_max.sign = 1; > > > + } > > > + // Flush [+DENORMAL, x] to [+0.0, x]. > > > + if (real_isdenormal (&m_min) && !real_isneg (&m_min)) > > > + m_min = dconst0; > > > +} > > > + > > > // Setter for franges. > > > > > > void > > > @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind) > > > gcc_checking_assert (tree_compare (LE_EXPR, min, max)); > > > > > > normalize_kind (); > > > + > > > + if (flag_unsafe_math_optimizations) > > > + flush_denormals_to_zero (); > > > + > > > if (flag_checking) > > > verify_range (); > > > } > > > diff --git a/gcc/value-range.h b/gcc/value-range.h > > > index 3a401f3e4e2..795b1f00fdc 100644 > > > --- a/gcc/value-range.h > > > +++ b/gcc/value-range.h > > > @@ -327,6 +327,7 @@ private: > > > bool union_nans (const frange &); > > > bool intersect_nans (const frange &); > > > bool combine_zeros (const frange &, bool union_p); > > > + void flush_denormals_to_zero (); > > > > > > tree m_type; > > > REAL_VALUE_TYPE m_min; > > > -- > > > 2.37.1 > > > > > >
On Mon, Sep 19, 2022 at 3:45 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Mon, Sep 19, 2022 at 3:04 PM Aldy Hernandez <aldyh@redhat.com> wrote: > > > > On Mon, Sep 19, 2022 at 9:37 AM Richard Biener > > <richard.guenther@gmail.com> wrote: > > > > > > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > Jakub has mentioned that for -funsafe-math-optimizations we may flush > > > > denormals to zero, in which case we need to be careful to extend the > > > > ranges to the appropriate zero. This patch does exactly that. For a > > > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x] > > > > we flush to [+0.0, x]. > > > > > > > > It is unclear whether we should do this for Alpha, since I believe > > > > flushing to zero is the default, and the port requires -mieee for IEEE > > > > sanity. If so, perhaps we should add a target hook so backends are > > > > free to request flushing to zero. > > > > > > > > Thoughts? > > > > > > I'm not sure what the intention of this is - it effectively results in > > > more conservative ranges for -funsafe-math-optimizations. That is, > > > if -funsafe-math-optimizations says that denormals don't exist and > > > are all zero then doesn't that mean we can instead use the smalles > > > non-denormal number less than (or greater than) zero here? > > > > > > That said, the flushing you do is also "valid" for > > > -fno-unsafe-math-optimizations > > > in case we don't want to deal with denormals in range boundaries. > > > > > > It might also be a correctness requirement in case we don't know how > > > targets handle denormals (IIRC even OS defaults might matter here) so > > > we conservatively have to treat them as being flushed to zero. > > > > Actually, rth suggested we always flush to zero because we don't know > > what the target would do. Again, I'm happy to do whatever you agree > > on. I have no opinion. My main goal here is correctness. > > Yes, I think we should do this. Ok, I'm pushing the attached patch because it's conservatively correct everywhere. We can tweak it when y'all reach consensus. > > > > > > > So ... if we want to be on the "safe" side then please always do this. > > > > > > At the same point you could do > > > > > > if (!HONOR_SIGNED_ZEROS ()) > > > if (real_iszero (&m_max)) > > > { > > > if (real_iszero (&m_min)) > > > m.min.sign = 1; > > > m_max.sign = 1; > > > } > > > > But wouldn't that set [-0.0, -0.0] when encountering [+0, +0] ?? > > Yeah, that's my laziness not adding a special case for m_min == m_max. > > > > > > else if (real_iszero (&m_min)) > > > m_min.sign = 0; > > > > Jakub actually suggested something completely different...just set +0 > > always for !HONOR_SIGNED_ZEROS. > > Hmm, but the [-1, -0.] with known sign becomes [-1, +0.] with unknown sign? Huh. I guess that's true. Does that happen often enough in practice to matter? I suppose we're going into uncharted territory with folks asking for no signs on zeros, but them appearing in the source code? My gut feeling is that we should take whatever signs are in the source code (similar to what we're doing for NANs), but don't introduce any additional ones. Again, no strong opinions. Feel free to add whatever adjustment you think is necessary. Aldy
On Tue, Sep 20, 2022 at 07:22:03AM +0200, Aldy Hernandez wrote: > > > Jakub actually suggested something completely different...just set +0 > > > always for !HONOR_SIGNED_ZEROS. > > > > Hmm, but the [-1, -0.] with known sign becomes [-1, +0.] with unknown sign? > > Huh. I guess that's true. Does that happen often enough in practice Sure, if you -fno-signed-zeros/-ffast-math and some variable can be zero, copysign/signbit is undefined. The option basically asserts you don't care about it... Jakub
diff --git a/gcc/value-range.cc b/gcc/value-range.cc index 67d5d7fa90f..f285734f0e0 100644 --- a/gcc/value-range.cc +++ b/gcc/value-range.cc @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2) return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2)); } +// Flush denormal endpoints to the appropriate 0.0. + +void +frange::flush_denormals_to_zero () +{ + if (undefined_p () || known_isnan ()) + return; + + // Flush [x, -DENORMAL] to [x, -0.0]. + if (real_isdenormal (&m_max) && real_isneg (&m_max)) + { + m_max = dconst0; + if (HONOR_SIGNED_ZEROS (m_type)) + m_max.sign = 1; + } + // Flush [+DENORMAL, x] to [+0.0, x]. + if (real_isdenormal (&m_min) && !real_isneg (&m_min)) + m_min = dconst0; +} + // Setter for franges. void @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind) gcc_checking_assert (tree_compare (LE_EXPR, min, max)); normalize_kind (); + + if (flag_unsafe_math_optimizations) + flush_denormals_to_zero (); + if (flag_checking) verify_range (); } diff --git a/gcc/value-range.h b/gcc/value-range.h index 3a401f3e4e2..795b1f00fdc 100644 --- a/gcc/value-range.h +++ b/gcc/value-range.h @@ -327,6 +327,7 @@ private: bool union_nans (const frange &); bool intersect_nans (const frange &); bool combine_zeros (const frange &, bool union_p); + void flush_denormals_to_zero (); tree m_type; REAL_VALUE_TYPE m_min;