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 7EBBF3858D37 for ; Thu, 20 Apr 2023 12:59:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7EBBF3858D37 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=1681995581; h=from:from:reply-to: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=TpxvCtJHhx8lj7r2u2+3kIlW7ENVPGcWEceGcAyx3R0=; b=iKRBThx/5w1lTq6L95GhxkblyvONcdB/Rxw7IrYV8pbTpNdSGB0ZTaAtpD7jqHu1U38N+z 0n9GoUaLgE9YjOnmgM+lEGTfp9E3FODMD1Y1a/W2jtpUYzqRLqmviuVjt4f+xor6/bRKE5 BjNIr2GTmqkOwEDiLB09WDBxLxQwwhU= 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-610-r3a75ZyRNICuP4-rZcTv1A-1; Thu, 20 Apr 2023 08:59:39 -0400 X-MC-Unique: r3a75ZyRNICuP4-rZcTv1A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A47BA811E7C; Thu, 20 Apr 2023 12:59:39 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.194.25]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4FDC840BC798; Thu, 20 Apr 2023 12:59:39 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 33KCxaM31981218 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 20 Apr 2023 14:59:37 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 33KCxZGk1981217; Thu, 20 Apr 2023 14:59:35 +0200 Date: Thu, 20 Apr 2023 14:59:35 +0200 From: Jakub Jelinek To: Aldy Hernandez , "Joseph S. Myers" Cc: GCC patches , Andrew MacLeod Subject: Re: [PATCH] Implement range-op entry for sin/cos. Message-ID: Reply-To: Jakub Jelinek References: <20230418131250.310916-1-aldyh@redhat.com> MIME-Version: 1.0 In-Reply-To: <20230418131250.310916-1-aldyh@redhat.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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 Tue, Apr 18, 2023 at 03:12:50PM +0200, Aldy Hernandez wrote: > [I don't know why I keep poking at floats. I must really like the pain. > Jakub, are you OK with this patch for trunk?] Thanks for working on this. Though expectedly here we are running again into the discussions we had in November about math properties of the functions vs. numeric properties in their implementations, how big maximum error shall we expect for the functions (and whether we should hardcode it for all implementations, or have some more fine-grained list of expected ulp errors for each implementation), whether the implementations at least guarantee the basic mathematical properties of the functions even if they have some errors (say for sin/cos, if they really never return > 1.0 or < -1.0) and the same questions repeated for -frounding-math, what kind of extra errors to expect when using non-default rounding and whether say sin could ever return nextafter (1.0, 2.0) or even larger value say when using non-default rounding mode. https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606466.html was my attempt to get at least some numbers on some targets, I'm afraid for most implementations we aren't going to get any numerical proofs of maximum errors and the like. For sin/cos to check whether the implementation really never returns > 1.0 or < -1.0 perhaps instead of using randomized testing we could exhaustively check some range around 0, M_PI, 3*M_PI, -M_PI, -3*M_PI, and say some much larger multiples of M_PI, say 50 ulps in each direction about those points, and similarly for sin around M_PI/2 etc., in all arounding modes. > This is the range-op entry for sin/cos. It is meant to serve as an > example of what we can do for glibc math functions. It is by no means > exhaustive, just a stub to restrict the return range from sin/cos to > [-1.0, 1.0] with appropriate smarts of NANs. > > As can be seen in the testcase, we see sin() as well as > __builtin_sin() in the IL, and can resolve the resulting range > accordingly. > > gcc/ChangeLog: > > * gimple-range-op.cc (class cfn_sincos): New. > (gimple_range_op_handler::maybe_builtin_call): Add case for sin/cos. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/range-sincos.c: New test. > --- > gcc/gimple-range-op.cc | 63 ++++++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/range-sincos.c | 40 +++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/range-sincos.c > > diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc > index 4ca32a7b5d5..36390f2645e 100644 > --- a/gcc/gimple-range-op.cc > +++ b/gcc/gimple-range-op.cc > @@ -402,6 +402,60 @@ public: > } > } op_cfn_copysign; > > +class cfn_sincos : public range_operator_float > +{ > +public: > + using range_operator_float::fold_range; > + using range_operator_float::op1_range; > + virtual bool fold_range (frange &r, tree type, > + const frange &lh, const frange &, > + relation_trio) const final override > + { > + if (lh.known_isnan () || lh.known_isinf ()) > + { > + r.set_nan (type); > + return true; > + } > + r.set (type, dconstm1, dconst1); See above, are we sure we can use [-1., 1.] range safely, or should that be [-1.-Nulps, 1.+Nulps] for some kind of expected worse error margin of the implementation? And ditto for -frounding-math, shall we increase that interval in that case, or is [-1., 1.] going to be ok? > + if (!lh.maybe_isnan ()) This condition isn't sufficient, even if lh can't be NAN, but just may be +Inf or -Inf, the result needs to include maybe NAN. > + r.clear_nan (); > + return true; Incrementally, if we decide what to do with the maximum allowed errors in ulps, if lh's range is smaller than 2*M_PI (upper_bound () - lower_bound ()), we could narrow it down further by computing the exact values for the bounds and any local maximums or minimums in between if any and creating a range out of that. > + } > + virtual bool op1_range (frange &r, tree type, > + const frange &lhs, const frange &, > + relation_trio) const final override > + { > + if (!lhs.maybe_isnan ()) > + { > + // If NAN is not valid result, the input cannot include either > + // a NAN nor a +-INF. > + REAL_VALUE_TYPE lb = real_min_representable (type); > + REAL_VALUE_TYPE ub = real_max_representable (type); > + r.set (type, lb, ub, nan_state (false, false)); > + return true; > + } > + // A known NAN means the input is [-INF,-INF][+INF,+INF] U +-NAN, > + // which we can't currently represent. > + if (lhs.known_isnan ()) > + { > + r.set_varying (type); > + return true; > + } > + // Results outside of [-1.0, +1.0] are impossible. > + REAL_VALUE_TYPE lb = lhs.lower_bound (); > + REAL_VALUE_TYPE ub = lhs.upper_bound (); > + if (real_less (&lb, &dconstm1) > + || real_less (&dconst1, &ub)) This condition could fit on one line. > + { > + r.set_undefined (); > + return true; > + } > + > + r.set_varying (type); I'm afraid we can't do really much better for the reverse case, even for a singleton range between -1.0 and +1.0 we'd have infinite number of results. In theory we could perhaps narrow it down from the minimum/maximum representable bounds, but even 1ulp there is much larger than 2*M_PI, so not sure it is worth bothering with it. Jakub