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 01BC53834C2A for ; Wed, 7 Dec 2022 15:26:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 01BC53834C2A 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=1670426779; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JcdrDVX87Pgwv267bX3J81y99EBt+hzRvKgeztUXA+0=; b=T3BiWAci0z3YqikhpyJS7ACtDzPnF1Z53hrPn9h3r27hmEwzsKWSTxPd71oJKxbl7lH+aI DAV6HaQRplRIQCvnomFY16upnHY5p2mdeMZPEUF3e5Ik1QIZJ+zOVijXJL6L8jA+YXUz5n U9Ta+k9eJn71hKrPCTq5jcU2I20z6so= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-622-W6ARfyHROtysHYTr5Z7ASQ-1; Wed, 07 Dec 2022 10:26:18 -0500 X-MC-Unique: W6ARfyHROtysHYTr5Z7ASQ-1 Received: by mail-wr1-f71.google.com with SMTP id e19-20020adfa453000000b0024209415034so4308188wra.18 for ; Wed, 07 Dec 2022 07:26:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=JcdrDVX87Pgwv267bX3J81y99EBt+hzRvKgeztUXA+0=; b=NLFR5RQiRCPwPydxqRdH19XqoJjNiUXpE821zWW+41haiMv8FDB6PP3PVT2tAceWyg jkubKfmMFkIq07ZRiMSRPuWkIY+bUPUU5r7df9ys6PuhLbh+fHSl/CjhVTnhEUlKVjDx 7/ama04ltwPJENywGqELKJDQBXvVTXxccN3D3DhkVxMqMgg+5n1dOi2NxTp55CNb2MrG j+aV7uSWEEnvv4IvgHGMuB7k1QLAuTAEm+pZgEKA9tGb/ZiQQRW9U5w+ZNDCC/BWt7Ss xEY+3QA/0A/g6k0nGfYlBMHW7oOFiKfYiQHFs08f2010SFd0oGgBbf06XRMgU6lafAXH NGlg== X-Gm-Message-State: ANoB5pnYHvBfWqVS7qlRefhNCPRZORCaAzOfaf7tJTWpIdgqg1k+USUG CwbmN25IVHnXXz4DEoCfK0Bdw+MgABOnJQf/UqI6pvZC88eM43axoWJQDy+tfu9xcSnDO/XcWoq iT5AwmfnWtsYkT11SlA== X-Received: by 2002:a05:600c:3c9a:b0:3c6:c6c9:d75e with SMTP id bg26-20020a05600c3c9a00b003c6c6c9d75emr816665wmb.0.1670426777361; Wed, 07 Dec 2022 07:26:17 -0800 (PST) X-Google-Smtp-Source: AA0mqf5KUhqgc6QVVDw7IGX20WZL4JE22kEr4gqM+0rsw1i5+M5PWGOMaVeen8eal2XFgSNg2XRFtg== X-Received: by 2002:a05:600c:3c9a:b0:3c6:c6c9:d75e with SMTP id bg26-20020a05600c3c9a00b003c6c6c9d75emr816650wmb.0.1670426776927; Wed, 07 Dec 2022 07:26:16 -0800 (PST) Received: from [192.168.86.37] (128.red-81-33-16.staticip.rima-tde.net. [81.33.16.128]) by smtp.gmail.com with ESMTPSA id o16-20020a05600c511000b003b492753826sm2319388wms.43.2022.12.07.07.26.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 07 Dec 2022 07:26:16 -0800 (PST) Message-ID: Date: Wed, 7 Dec 2022 16:26:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [PATCH] range-op-float: Fix up frange_arithmetic [PR107967] To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org, andrew MacLeod References: From: Aldy Hernandez In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,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: On 12/7/22 09:29, Jakub Jelinek wrote: > Hi! > > The addition of PLUS/MINUS/MULT/RDIV_EXPR frange handlers causes > miscompilation of some of the libm routines, resulting in lots of > glibc test failures. A part of them is purely PR107608 fold-overflow-1.c > etc. issues, say when the code does > return -0.5 / 0.0; > and expects division by zero to be emitted, but we propagate -Inf > and avoid the operation. > But there are also various tests where we end up with different computed > value from the expected ones. All those cases are like: > is: inf inf > should be: 1.18973149535723176502e+4932 0xf.fffffffffffffff0p+16380 > is: inf inf > should be: 1.18973149535723176508575932662800701e+4932 0x1.ffffffffffffffffffffffffffffp+16383 > is: inf inf > should be: 1.7976931348623157e+308 0x1.fffffffffffffp+1023 > is: inf inf > should be: 3.40282346e+38 0x1.fffffep+127 > and the corresponding source looks like: > static const double huge = 1.0e+300; > double whatever (...) { > ... > return huge * huge; > ... > } > which for rounding to nearest or +inf should and does return +inf, but > for rounding to -inf or 0 should instead return nextafter (inf, -inf); > The rules IEEE754 has are that operations on +-Inf operands are exact > and produce +-Inf (except for the invalid ones that produce NaN) regardless > of rounding mode, while overflows: > "a) roundTiesToEven and roundTiesToAway carry all overflows to ∞ with the > sign of the intermediate result. > b) roundTowardZero carries all overflows to the format’s largest finite > number with the sign of the intermediate result. > c) roundTowardNegative carries positive overflows to the format’s largest > finite number, and carries negative overflows to −∞. > d) roundTowardPositive carries negative overflows to the format’s most > negative finite number, and carries positive overflows to +∞." > > The behavior around overflows to -Inf or nextafter (-inf, inf) was actually > handled correctly, we'd construct [-INF, -MAX] ranges in those cases > because !real_less (&value, &result) in that case - value is finite > but larger in magnitude than what the format can represent (but GCC > internal's format can), while result is -INF in that case. > But for the overflows to +Inf or nextafter (inf, -inf) was handled > incorrectly, it tested real_less (&result, &value) rather than > !real_less (&result, &value), the former test is true when already the > rounding value -> result rounded down and in that case we shouldn't > round again, we should round down when it didn't. > > So, in theory this could be fixed just by adding one ! character, > - if ((mode_composite || (real_isneg (&inf) ? real_less (&result, &value) > + if ((mode_composite || (real_isneg (&inf) ? !real_less (&result, &value) > : !real_less (&value, &result))) > but the following patch goes further. The distance between > nextafter (inf, -inf) and inf is large (infinite) and expressions like > 1.0e+300 * 1.0e+300 always produce +inf in round to nearest mode by far, > so I think having low bound of nextafter (inf, -inf) in that case is > unnecessary. But if it isn't multiplication but say addition and we are > inexact and very close to the boundary between rounding to nearest > maximum representable vs. rounding to nearest +inf, still using [MAX, +INF] > etc. ranges seems safer because we don't know exactly what we lost in the > inexact computation. > > The following patch implements that. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2022-12-07 Jakub Jelinek > > PR tree-optimization/107967 > * range-op-float.cc (frange_arithmetic): Fix a thinko - if > inf is negative, use nextafter if !real_less (&result, &value) > rather than if real_less (&result, &value). If result is +-INF > while value is finite and -fno-rounding-math, don't do rounding > if !inexact or if result is significantly above max representable > value or below min representable value. > > * gcc.dg/pr107967-1.c: New test. > * gcc.dg/pr107967-2.c: New test. > * gcc.dg/pr107967-3.c: New test. > > --- gcc/range-op-float.cc.jj 2022-12-06 10:25:16.594848892 +0100 > +++ gcc/range-op-float.cc 2022-12-06 20:53:47.751295689 +0100 > @@ -287,9 +287,64 @@ frange_arithmetic (enum tree_code code, > > // Be extra careful if there may be discrepancies between the > // compile and runtime results. > - if ((mode_composite || (real_isneg (&inf) ? real_less (&result, &value) > - : !real_less (&value, &result))) > - && (inexact || !real_identical (&result, &value))) > + bool round = false; > + if (mode_composite) > + round = true; > + else if (real_isneg (&inf)) > + { > + round = !real_less (&result, &value); > + if (real_isinf (&result, false) > + && !real_isinf (&value) > + && !flag_rounding_math) > + { > + // Use just [+INF, +INF] rather than [MAX, +INF] > + // even if value is larger than MAX and rounds to > + // nearest to +INF. Unless INEXACT is true, in > + // that case we need some extra buffer. > + if (!inexact) > + round = false; > + else > + { > + REAL_VALUE_TYPE tmp = result, tmp2; > + frange_nextafter (mode, tmp, inf); > + // TMP is at this point the maximum representable > + // number. > + real_arithmetic (&tmp2, MINUS_EXPR, &value, &tmp); > + if (!real_isneg (&tmp2) > + && (REAL_EXP (&tmp2) - REAL_EXP (&tmp) > + >= 2 - REAL_MODE_FORMAT (mode)->p)) > + round = false; > + } > + } > + } This chunk... > + else > + { > + round = !real_less (&value, &result); > + if (real_isinf (&result, true) > + && !real_isinf (&value) > + && !flag_rounding_math) > + { > + // Use just [-INF, -INF] rather than [-INF, +MAX] > + // even if value is smaller than -MAX and rounds to > + // nearest to -INF. Unless INEXACT is true, in > + // that case we need some extra buffer. > + if (!inexact) > + round = false; > + else > + { > + REAL_VALUE_TYPE tmp = result, tmp2; > + frange_nextafter (mode, tmp, inf); > + // TMP is at this point the minimum representable > + // number. > + real_arithmetic (&tmp2, MINUS_EXPR, &value, &tmp); > + if (real_isneg (&tmp2) > + && (REAL_EXP (&tmp2) - REAL_EXP (&tmp) > + >= 2 - REAL_MODE_FORMAT (mode)->p)) > + round = false; > + } > + } > + } ...is quite similar to this one. Could you abstract this? Also: > + if (real_isinf (&result, true) > + && !real_isinf (&value) > + && !flag_rounding_math) Either abstract this out into a predicate since it's also repeated, or comment it. Aldy > + if (round && (inexact || !real_identical (&result, &value))) > { > if (mode_composite) > { > --- gcc/testsuite/gcc.dg/pr107967-1.c.jj 2022-12-06 20:02:22.844086729 +0100 > +++ gcc/testsuite/gcc.dg/pr107967-1.c 2022-12-06 20:03:59.444683025 +0100 > @@ -0,0 +1,35 @@ > +/* PR tree-optimization/107967 */ > +/* { dg-do compile { target float64 } } */ > +/* { dg-options "-O2 -frounding-math -fno-trapping-math -fdump-tree-optimized" } */ > +/* { dg-add-options float64 } */ > +/* { dg-final { scan-tree-dump-not "return\[ \t]\*-?Inf;" "optimized" } } */ > + > +_Float64 > +foo (void) > +{ > + const _Float64 huge = 1.0e+300f64; > + return huge * huge; > +} > + > +_Float64 > +bar (void) > +{ > + const _Float64 huge = 1.0e+300f64; > + return huge * -huge; > +} > + > +_Float64 > +baz (void) > +{ > + const _Float64 a = 0x1.fffffffffffffp+1023f64; > + const _Float64 b = 0x1.fffffffffffffp+970f64; > + return a + b; > +} > + > +_Float64 > +qux (void) > +{ > + const _Float64 a = 0x1.fffffffffffffp+1023f64; > + const _Float64 b = 0x1.fffffffffffffp+969f64; > + return a + b; > +} > --- gcc/testsuite/gcc.dg/pr107967-2.c.jj 2022-12-06 20:02:29.683987331 +0100 > +++ gcc/testsuite/gcc.dg/pr107967-2.c 2022-12-06 20:03:48.685839355 +0100 > @@ -0,0 +1,35 @@ > +/* PR tree-optimization/107967 */ > +/* { dg-do compile { target float64 } } */ > +/* { dg-options "-O2 -fno-rounding-math -fno-trapping-math -fdump-tree-optimized" } */ > +/* { dg-add-options float64 } */ > +/* { dg-final { scan-tree-dump-times "return\[ \t]\*-?Inf;" 3 "optimized" } } */ > + > +_Float64 > +foo (void) > +{ > + const _Float64 huge = 1.0e+300f64; > + return huge * huge; > +} > + > +_Float64 > +bar (void) > +{ > + const _Float64 huge = 1.0e+300f64; > + return huge * -huge; > +} > + > +_Float64 > +baz (void) > +{ > + const _Float64 a = 0x1.fffffffffffffp+1023f64; > + const _Float64 b = 0x1.fffffffffffffp+970f64; > + return a + b; > +} > + > +_Float64 > +qux (void) > +{ > + const _Float64 a = 0x1.fffffffffffffp+1023f64; > + const _Float64 b = 0x1.fffffffffffffp+969f64; > + return a + b; > +} > --- gcc/testsuite/gcc.dg/pr107967-3.c.jj 2022-12-06 20:29:35.243370388 +0100 > +++ gcc/testsuite/gcc.dg/pr107967-3.c 2022-12-06 20:53:16.553748313 +0100 > @@ -0,0 +1,53 @@ > +/* PR tree-optimization/107967 */ > +/* { dg-do compile { target float64 } } */ > +/* { dg-options "-O2 -fno-rounding-math -fno-trapping-math -fdump-tree-optimized" } */ > +/* { dg-add-options float64 } */ > +/* { dg-final { scan-tree-dump-times "return\[ \t]\*-?Inf;" 3 "optimized" } } */ > + > +_Float64 > +foo (_Float64 x) > +{ > + if (x >= 1.0e+300f64) > + ; > + else > + __builtin_unreachable (); > + return x * x; > +} > + > +_Float64 > +bar (_Float64 x) > +{ > + if (x >= 1.0e+300f64) > + ; > + else > + __builtin_unreachable (); > + return x * -x; > +} > + > +_Float64 > +baz (_Float64 a, _Float64 b) > +{ > + if (a >= 0x1.fffffffffffffp+1023f64) > + ; > + else > + __builtin_unreachable (); > + if (b >= 0x1.p+972f64) > + ; > + else > + __builtin_unreachable (); > + return a + b; > +} > + > +_Float64 > +qux (_Float64 a, _Float64 b) > +{ > + if (a >= 0x1.fffffffffffffp+1023f64) > + ; > + else > + __builtin_unreachable (); > + if (b >= 0x1.fffffffffffffp+969f64) > + ; > + else > + __builtin_unreachable (); > + return a + b; > +} > > Jakub >