From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 1BECD3858426 for ; Tue, 4 Apr 2023 14:08:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1BECD3858426 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680617314; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=71HZ947JWEYSAHo1wEhGCEoknWSFeNl1phsEcyOv+l4=; b=L9HeEzvV5eyyPRBK3RqWahi1emqWXzg30u8DtxIugupJETBlqtQViMXJv5R3ZdeN0rxlD8 8P4BYRlwEIdTjwRM2Bo6xHvQ2qmpYcTkawa7HAcsjHyal53f59FjJaTXJXgHPOaC7BcPZQ n3UFF7ICLpn+Li/M10Oz361NQRW7rhU= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-262-PirsCvA9N3CWpl3AyC_Yhg-1; Tue, 04 Apr 2023 10:08:31 -0400 X-MC-Unique: PirsCvA9N3CWpl3AyC_Yhg-1 Received: by mail-ed1-f72.google.com with SMTP id s30-20020a508d1e000000b005005cf48a93so45943089eds.8 for ; Tue, 04 Apr 2023 07:08:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680617309; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=71HZ947JWEYSAHo1wEhGCEoknWSFeNl1phsEcyOv+l4=; b=1oYpBhNsW0/18GRlUf7rZ4cgkGIQYtHzXqSn6K8xp4rvfkpJQjfomV/YP9uV7CRhcH YDzniviRi7NpSwM/7lnAb7FW+T+vV6mIxkHq55w+RyYa81dvFiK+tqtdifFV2nuo8vIz nyX3iQ4DTPIFOOEzYlUmg/1dmjn+S6TgWmVT6FygBgcqRfOBD+VMQ9x28bfaGn7nPFR7 QjzDM0YGzAtcjvHDpbDUNszSj4SOa2JLV8B/bWARZzfTkYMl5BG1Ix+WNx4INf2kjC2l 5lWNcT76SwonAA+X5rOH6BG3xxDbEH4Hb0CvxaeP2GrTyY7eA4KpgQaz2s1fWTAFVi89 M4UA== X-Gm-Message-State: AAQBX9cZCmPV2X+iHdX43wW8H+AunPjU7xM4SfzSCdBvh2IzhWWfmpOl eIWJ/GXS4U0S0lNTZBxfhCvBRxTeMMrOpBMB9KXX1BMzX/Ug1uX03QLuuPBxhBW+YRT1z+8ut87 H0ek8YrHAnaBDOhA5lYVxP3Hsro7tmZ/TwFdkFyOYjA== X-Received: by 2002:a17:906:99c8:b0:92f:fc27:8ea0 with SMTP id s8-20020a17090699c800b0092ffc278ea0mr1369895ejn.9.1680617309545; Tue, 04 Apr 2023 07:08:29 -0700 (PDT) X-Google-Smtp-Source: AKy350a/JpuMWnRa0PoSuIuEDb7Mk/cVqEOclr8R/17+E+IQg+bjKZDZ3fr2C4WYsbDM5kvCRgEtYSV+Tulj17BYNQQ= X-Received: by 2002:a17:906:99c8:b0:92f:fc27:8ea0 with SMTP id s8-20020a17090699c800b0092ffc278ea0mr1369881ejn.9.1680617309214; Tue, 04 Apr 2023 07:08:29 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Aldy Hernandez Date: Tue, 4 Apr 2023 16:08:17 +0200 Message-ID: Subject: Re: [PATCH] range-op-float: Fix reverse ops of comparisons [PR109386] To: Jakub Jelinek Cc: Andrew MacLeod , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="000000000000b82c1a05f88335eb" X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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 List-Id: --000000000000b82c1a05f88335eb Content-Type: text/plain; charset="UTF-8" OK. On Mon, Apr 3, 2023, 21:54 Jakub Jelinek wrote: > Hi! > > I've missed one of my recent range-op-float.cc changes (likely the > r13-6967 one) caused > FAIL: libphobos.phobos/std/math/algebraic.d execution test > FAIL: libphobos.phobos_shared/std/math/algebraic.d execution test > regressions, distilled into a C testcase below. > > In the testcase, we have > !(u >= v) > condition where both u and v are results of fabs*, which guards > t1 = u u<= __FLT_MAX__; > and > t2 = v u<= __FLT_MAX__; > comparisons. From false u >= v where u and v have [0.0, +Inf] NAN > ranges we (incorrectly deduce that one of them is [nextafterf (0.0, 1.0), > +Inf] NAN > and the other is [0.0, nextafterf (+Inf, 0.0)] NAN and from that deduce > that > one of the comparisons is always true, because UNLE_EXPR with the maximum > representable number are false only if the value is +Inf and our ranges > tell > that is not possible. > > The bug is that the u >= v comparison determines a sensible range only when > it is true - we then know neither operand can be NAN and it behaves > correctly. But when the comparison is false, our current code gives > sensible answers only if the other op can't be NAN. If it can be NAN, > whenever it is NAN, the comparison is always false regardless of the other > value, so the other value needs to be VARYING. > Now, foperator_unordered_lt::op1_range etc. had code to deal with that > for op?.known_nan (), but as the testcase shows, it is enough if it may be > a > NAN at runtime to make it VARYING. > > So, the following patch replaces for all those BRS_FALSE cases of the > normal > non-equality comparisons if (opOTHER.known_isnan ()) r.set_varying (type); > to do it also if maybe_isnan (). > > For the unordered or ... comparisons, it is similar for BRS_TRUE. Those > comparisons are true whenever either operand is NAN, or if neither is NAN, > the corresponding normal comparison. So, if those comparisons are true > and other operand might be a NAN, we can't tell (VARYING), if it is false, > currently handling is correct. > > Bootstrapped/regtested on x86_64-linux and i686-linux, fixes those 2 > D testcases and the newly added one. Ok for trunk? > > 2023-04-03 Jakub Jelinek > > PR tree-optimization/109386 > * range-op-float.cc (foperator_lt::op1_range, > foperator_lt::op2_range, > foperator_le::op1_range, foperator_le::op2_range, > foperator_gt::op1_range, foperator_gt::op2_range, > foperator_ge::op1_range, foperator_ge::op2_range): Make r varying > for > BRS_FALSE case even if the other op is maybe_isnan, not just > known_isnan. > (foperator_unordered_lt::op1_range, > foperator_unordered_lt::op2_range, > foperator_unordered_le::op1_range, > foperator_unordered_le::op2_range, > foperator_unordered_gt::op1_range, > foperator_unordered_gt::op2_range, > foperator_unordered_ge::op1_range, > foperator_unordered_ge::op2_range): > Make r varying for BRS_TRUE case even if the other op is > maybe_isnan, > not just known_isnan. > > * gcc.c-torture/execute/ieee/pr109386.c: New test. > > --- gcc/range-op-float.cc.jj 2023-04-03 10:42:54.000000000 +0200 > +++ gcc/range-op-float.cc 2023-04-03 13:31:01.163216123 +0200 > @@ -889,7 +889,7 @@ foperator_lt::op1_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of x < NAN, we know nothing about x. > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else > build_ge (r, type, op2); > @@ -926,7 +926,7 @@ foperator_lt::op2_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of NAN < x, we know nothing about x. > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else > build_le (r, type, op1); > @@ -1005,7 +1005,7 @@ foperator_le::op1_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of x <= NAN, we know nothing about x. > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else > build_gt (r, type, op2); > @@ -1038,7 +1038,7 @@ foperator_le::op2_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of NAN <= x, we know nothing about x. > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1122,7 +1122,7 @@ foperator_gt::op1_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of x > NAN, we know nothing about x. > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -1161,7 +1161,7 @@ foperator_gt::op2_range (frange &r, > > case BRS_FALSE: > // On The FALSE side of NAN > x, we know nothing about x. > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1241,7 +1241,7 @@ foperator_ge::op1_range (frange &r, > > case BRS_FALSE: > // On the FALSE side of x >= NAN, we know nothing about x. > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -1275,7 +1275,7 @@ foperator_ge::op2_range (frange &r, tree > > case BRS_FALSE: > // On the FALSE side of NAN >= x, we know nothing about x. > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1639,7 +1639,7 @@ foperator_unordered_lt::op1_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -1673,7 +1673,7 @@ foperator_unordered_lt::op2_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1755,7 +1755,7 @@ foperator_unordered_le::op1_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -1788,7 +1788,7 @@ foperator_unordered_le::op2_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1872,7 +1872,7 @@ foperator_unordered_gt::op1_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -1907,7 +1907,7 @@ foperator_unordered_gt::op2_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > @@ -1991,7 +1991,7 @@ foperator_unordered_ge::op1_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op2.known_isnan ()) > + if (op2.known_isnan () || op2.maybe_isnan ()) > r.set_varying (type); > else if (op2.undefined_p ()) > return false; > @@ -2025,7 +2025,7 @@ foperator_unordered_ge::op2_range (frang > switch (get_bool_state (r, lhs, type)) > { > case BRS_TRUE: > - if (op1.known_isnan ()) > + if (op1.known_isnan () || op1.maybe_isnan ()) > r.set_varying (type); > else if (op1.undefined_p ()) > return false; > --- gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c.jj 2023-04-03 > 13:44:59.715981917 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c 2023-04-03 > 13:44:48.480145842 +0200 > @@ -0,0 +1,21 @@ > +/* PR tree-optimization/109386 */ > + > +static inline float > +foo (float x, float y) > +{ > + float u = __builtin_fabsf (x); > + float v = __builtin_fabsf (y); > + if (!(u >= v)) > + { > + if (__builtin_isinf (v)) return v; > + if (__builtin_isinf (u)) return u; > + } > + return 42.0f; > +} > + > +int > +main () > +{ > + if (!__builtin_isinf (foo (__builtin_inff (), __builtin_nanf ("")))) > + __builtin_abort (); > +} > > Jakub > > --000000000000b82c1a05f88335eb--