From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 96F183858C54 for ; Fri, 10 Mar 2023 08:53:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 96F183858C54 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 94E5A2277D; Fri, 10 Mar 2023 08:53:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1678438417; h=from:from:reply-to: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=sEQT6jSH/kYgxW0WtGhjGeKA5Fw/QfX0KI1BYnhryIs=; b=X2gknC1pEjIlnONV9txlJqiQJZ6eucGNwsMyF/9/OFHIvuEAkkdVZGAUZKoaF28dRq0x6W 6FaaY00IDKS188wIaJ2nDfHiYiF770cUs7e9okVxJs/AIJwJ0J5+aF/8VS9EwAJPZJlU+1 0pTeVdVRtjIsC977wbs8RSmn2aSKsFg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1678438417; h=from:from:reply-to: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=sEQT6jSH/kYgxW0WtGhjGeKA5Fw/QfX0KI1BYnhryIs=; b=ZjZk8drbFwmfpbC2bEmqhq4hYZIsd5J0HnPDuYObCEZc4BU4mja9NUE6nSSMauIrMKQctT t5IDJ7h55MYfa1CQ== Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 82F502C141; Fri, 10 Mar 2023 08:53:37 +0000 (UTC) Date: Fri, 10 Mar 2023 08:53:37 +0000 (UTC) From: Richard Biener To: Jakub Jelinek cc: Aldy Hernandez , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008] In-Reply-To: Message-ID: References: User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,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 Fri, 10 Mar 2023, Jakub Jelinek wrote: > Hi! > > The following patch does two things (both related to range extension > around the boundaries). > > The first part (in the 2 real_isfinite blocks) is to make the ranges > narrower when the old boundaries are minimum and/or maximum representable > finite number. In that case frange_nextafter gives -Inf or +Inf, > but then the resulting computed reverse range is very far from the actually > needed range, usually extends up to infinity or could even result in NaNs. > While infinities are really the next representable numbers in the > corresponding mode, REAL_VALUE_TYPE is actually a type with wider range > for exponent and 160 bit precision, so the patch instead uses > nextafter number in a hypothetical floating point format with the same > mantissa precision but wider range of exponents. This significantly > improves the actual ranges of the reverse operations, while still making > them conservatively correct. > > The second part is a fix for miscompilation of the new testcase below. > For -ffinite-math-only, without this patch we extend the minimum and/or > maximum representable finite number to -Inf or +Inf, with the patch to > some number outside of the normal exponent range of the mode, but then > we use set which canonicalizes it and turns the boundaries back to > the minimum and/or maximum representable finite numbers, but because > in say [__DBL_MAX__, __DBL_MAX__] = op1 + [__DBL_MAX__, __DBL_MAX__] > op1 can be larger than 0, up to the largest number which rounds to even > down back to __DBL_MAX__ and there are still no infinities involved, > it needs to work even with -ffinite-math-only. So, we really need to > widen the lhs range a little bit even in that case. The patch does > that through temporarily clearing -ffinite-math-only, such that the > value with infinities or the outside of bounds values passes the > setting and verification (the VR_VARYING case is needed because > we get ICEs otherwise, but when lhs is VR_VARYING in -ffast-math, > i.e. minimum to maximum representable finite and both signs of NaN, > then set does all we need, we don't need to or in a NaN range). > We don't really later use the range in a way that would become a problem > that it is wider than varying, we actually just perform maths on the > two boundaries. > > As I said in the PR, this doesn't fix the !MODE_HAS_INFINITIES case, > I believe we actually need to treat the boundary values as infinities > in that case because they (probably) work like that, but it is unclear > if it is just the reverse operation lhs widening that is a problem there, > or whether it is a general problem. I have zero experience with > floating points without infinities (PDP11, some ARM half type?, > what else?). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2023-03-10 Jakub Jelinek > > PR tree-optimization/109008 > * range-op-float.cc (float_widen_lhs_range): If lb is > minimum representable finite number or ub is maximum > representable finite number, instead of widening it to > -inf or inf widen it to negative or positive 0x0.8p+(EMAX+1). > Temporarily clear flag_finite_math_only when canonicalizing > the widened range. > > * gcc.dg/pr109008.c: New test. > > --- gcc/range-op-float.cc.jj 2023-03-09 09:54:53.880453046 +0100 > +++ gcc/range-op-float.cc 2023-03-09 20:52:07.456284507 +0100 > @@ -2217,12 +2217,42 @@ float_widen_lhs_range (tree type, const > REAL_VALUE_TYPE lb = lhs.lower_bound (); > REAL_VALUE_TYPE ub = lhs.upper_bound (); > if (real_isfinite (&lb)) > - frange_nextafter (TYPE_MODE (type), lb, dconstninf); > + { > + frange_nextafter (TYPE_MODE (type), lb, dconstninf); > + if (real_isinf (&lb)) > + { > + /* For -DBL_MAX, instead of -Inf use > + nexttoward (-DBL_MAX, -LDBL_MAX) in a hypothetical > + wider type with the same mantissa precision but larger > + exponent range; it is outside of range of double values, > + but makes it clear it is just one ulp larger rather than > + infinite amount larger. */ > + lb = dconstm1; > + SET_REAL_EXP (&lb, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); > + } > + } > if (real_isfinite (&ub)) > - frange_nextafter (TYPE_MODE (type), ub, dconstinf); > + { > + frange_nextafter (TYPE_MODE (type), ub, dconstinf); > + if (real_isinf (&ub)) > + { > + /* For DBL_MAX similarly. */ > + ub = dconst1; > + SET_REAL_EXP (&ub, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1); > + } > + } > + /* Temporarily disable -ffinite-math-only, so that frange::set doesn't > + reduce the range back to real_min_representable (type) as lower bound > + or real_max_representable (type) as upper bound. */ > + bool save_flag_finite_math_only = flag_finite_math_only; > + flag_finite_math_only = false; > ret.set (type, lb, ub); > - ret.clear_nan (); > - ret.union_ (lhs); > + if (lhs.kind () != VR_VARYING) > + { > + ret.clear_nan (); > + ret.union_ (lhs); > + } > + flag_finite_math_only = save_flag_finite_math_only; Meh - I wonder if we can avoid all this by making float_widen_lhs_range friend of frange and simply access m_min/m_max directly and use the copy-CTOR to copy bounds and nan state ... after all verify_range will likely fail after you restore flag_finite_math_only ... But OK for the moment. Thanks, Richard. > return ret; > } > > --- gcc/testsuite/gcc.dg/pr109008.c.jj 2023-03-09 12:25:11.507955698 +0100 > +++ gcc/testsuite/gcc.dg/pr109008.c 2023-03-09 12:33:35.795598344 +0100 > @@ -0,0 +1,26 @@ > +/* PR tree-optimization/109008 */ > +/* { dg-do run } */ > +/* { dg-options "-O2 -ffinite-math-only -fexcess-precision=standard" } */ > + > +__attribute__((noipa)) double > +foo (double eps) > +{ > + double d = __DBL_MAX__ + eps; > + if (d == __DBL_MAX__) > + if (eps > 16.0) > + return eps; > + return 0.0; > +} > + > +int > +main () > +{ > +#if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024 && __DBL_MIN_EXP__ == -1021 \ > + && __FLT_EVAL_METHOD__ == 0 > + if (foo (0x0.8p+970) == 0.0) > + __builtin_abort (); > + if (foo (32.0) == 0.0) > + __builtin_abort (); > +#endif > + return 0; > +} > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)