public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix up foperator_abs::op1_range [PR107569]
Date: Wed, 9 Nov 2022 12:32:18 +0100	[thread overview]
Message-ID: <CAGm3qMVrCKYAbBevEyma2rOr94Kdd2DpVcpb_oxbx3BV=VyOvA@mail.gmail.com> (raw)
In-Reply-To: <Y2uHDeXiivo401ni@tucnak>

On Wed, Nov 9, 2022 at 11:55 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> foperator_abs::op1_range works except for the NaN handling,
> from:
> [frange] double [-Inf, 1.79769313486231570814527423731704356798070567525844996599e+308 (0x0.fffffffffffff8p+1024)]
> lhs it computes r
> [frange] double [-1.79769313486231570814527423731704356798070567525844996599e+308 (-0x0.fffffffffffff8p+1024), 1.79769313486231570814527423731704356798070567525844996599e+308 (0x0.fffffffffffff8p+1024)] +-NAN
> which is correct except for the +-NAN part.
> For r before the final step it makes sure to add -NAN if there is +NAN
> in the lhs range, but the final r.union_ makes it unconditional +-NAN,
> because the frange ctor sets +-NAN.
> So, I think we need to clear it (or have some set variant which
> says not to set NAN).
>
> This patch fixes that, but isn't enough to fix the PR, something in
> the assumptions handling is still broken (and the PR has other parts).

LGTM.

Sorry I haven't gotten to the PR yet.  I'll poke at it later today.

Thanks.
Aldy

>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2022-11-09  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/107569
>         * range-op-float.cc (foperator_abs::op1_range): Clear NaNs
>         from the negatives frange before unioning it into r.
>
> --- gcc/range-op-float.cc.jj    2022-11-06 11:56:27.138137781 +0100
> +++ gcc/range-op-float.cc       2022-11-08 18:13:18.026974667 +0100
> @@ -1280,9 +1280,10 @@ foperator_abs::op1_range (frange &r, tre
>      return true;
>    // Then add the negative of each pair:
>    // ABS(op1) = [5,20] would yield op1 => [-20,-5][5,20].
> -  r.union_ (frange (type,
> -                   real_value_negate (&positives.upper_bound ()),
> -                   real_value_negate (&positives.lower_bound ())));
> +  frange negatives (type, real_value_negate (&positives.upper_bound ()),
> +                   real_value_negate (&positives.lower_bound ()));
> +  negatives.clear_nan ();
> +  r.union_ (negatives);
>    return true;
>  }
>
>
>         Jakub
>


      reply	other threads:[~2022-11-09 11:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 10:55 Jakub Jelinek
2022-11-09 11:32 ` Aldy Hernandez [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGm3qMVrCKYAbBevEyma2rOr94Kdd2DpVcpb_oxbx3BV=VyOvA@mail.gmail.com' \
    --to=aldyh@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).