From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 625DF3858D37 for ; Mon, 6 Jul 2020 16:01:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 625DF3858D37 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-7d9Qs5xfMvyog-Ojdp0OyQ-1; Mon, 06 Jul 2020 12:01:05 -0400 X-MC-Unique: 7d9Qs5xfMvyog-Ojdp0OyQ-1 Received: by mail-qk1-f200.google.com with SMTP id q6so18025637qke.21 for ; Mon, 06 Jul 2020 09:01:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=s0djG1qX75KATbe+5KniZzkhvUhnoFLy1QI9VXMolj0=; b=pYvpjgF1lb4S1mKSkRwfvrpVoe0tVw80XjRTN9reEQfRN0jnuhN64Wbrf9MVPvSAzo Uh6m34Er/HTNh/Aw26Dv0K2XvKO+eMpNofnjL211n/uJ6j60fwbaaxb/4wj8hzitD46o 4GFCPok8gj06UtFHzEFsBzCekU8U+FoQn/z6Xr6YbUVynlvNKDAz3EIj5s/VSDJ/1yR1 POliKwFh3Pjr2xMl/kp/mFgmnRUvXVSph9rqxkRmmXervxgP8W6q4sH7z9qrnJ9P5xDY rpqqk2gzsEYrSIk/nrXMTEIVMg8xaByCDEougHLxI7oK+dlKmu8Wjt34/jGiNl4YZRyM UpGw== X-Gm-Message-State: AOAM531hBpdpn6xCQCg7osEXwLR0DNFaGYWSwgMyTqAkLWzRzbSF1Qdv wk4n6ywDGhzIlacH0CeDxx3tD+I4o8G471plRYciz8CUQigdVaYoXuSJMCHH6rkgiQgJ/8HATd/ 0TNFoEu+z4Fby0Nsw/Rjw X-Received: by 2002:a05:6214:d4d:: with SMTP id 13mr14426942qvr.22.1594051264355; Mon, 06 Jul 2020 09:01:04 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwGudGvYj6ud/vt0vGvDOVEqJ67Sg/9BA32OiOI8hDAf0TDgRVVuYJqlT+Pd6SF82U7AmfJiw== X-Received: by 2002:a05:6214:d4d:: with SMTP id 13mr14426893qvr.22.1594051263961; Mon, 06 Jul 2020 09:01:03 -0700 (PDT) Received: from [192.168.1.4] (198-84-170-103.cpe.teksavvy.com. [198.84.170.103]) by smtp.gmail.com with ESMTPSA id j16sm21007800qtp.92.2020.07.06.09.01.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Jul 2020 09:01:03 -0700 (PDT) Subject: Re: Use C2x return value from getpayload of non-NaN (bug 26073) To: Joseph Myers , libc-alpha@sourceware.org References: From: Carlos O'Donell Organization: Red Hat Message-ID: Date: Mon, 6 Jul 2020 12:01:02 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2020 16:01:13 -0000 On 7/1/20 6:05 PM, Joseph Myers wrote: > In TS 18661-1, getpayload had an unspecified return value for a > non-NaN argument, while C2x requires the return value -1 in that case. Reviewed TS 18661-1 F.10.13.1 getpayload and confirmed the result is unspecified for non-NaN argument case. Confirmed in N2454 that if *x is not a NaN that the result shall be -1 (not unspecified as in TS 18661-1). > This patch implements the return value of -1. I don't think this is > worth having a new symbol version that's an alias of the old one, > although occasionally we do that in such cases where the new function > semantics are a refinement of the old ones (to avoid programs relying > on the new semantics running on older glibc versions but not behaving > as intended). > > Tested for x86_64 and x86; also ran math/ tests for aarch64 and > powerpc. OK for mater. Reviewed-by: Carlos O'Donell > --- > > Carlos, is this OK at the current slush development stage? Yes it is. I consider this a "forward looking" change that while making a semantic change is within the scope of a bug fix. > diff --git a/manual/arith.texi b/manual/arith.texi > index 89c2c064f1..75eaf67fe7 100644 > --- a/manual/arith.texi > +++ b/manual/arith.texi > @@ -1895,9 +1895,10 @@ propagated from NaN inputs to the result of a floating-point > operation. These functions, defined by TS 18661-1:2014 and TS > 18661-3:2015, return the payload of the NaN pointed to by @var{x} > (returned as a positive integer, or positive zero, represented as a > -floating-point number); if @var{x} is not a NaN, they return an > -unspecified value. They raise no floating-point exceptions even for > -signaling NaNs. > +floating-point number); if @var{x} is not a NaN, they return > +@minus{}1. They raise no floating-point exceptions even for signaling > +NaNs. (The return value of @minus{}1 for an argument that is not a > +NaN is specified in C2x; the value was unspecified in TS 18661.) OK. > @end deftypefun > > @deftypefun int setpayload (double *@var{x}, double @var{payload}) > diff --git a/math/libm-test-getpayload.inc b/math/libm-test-getpayload.inc > index bd660471aa..72e3c19d1e 100644 > --- a/math/libm-test-getpayload.inc > +++ b/math/libm-test-getpayload.inc > @@ -20,17 +20,17 @@ > > static const struct test_f_f_data getpayload_test_data[] = > { > - TEST_fp_f (getpayload, plus_infty, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, minus_infty, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, plus_zero, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, minus_zero, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, 1000, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, max_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, -max_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, min_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, -min_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, min_subnorm_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > - TEST_fp_f (getpayload, -min_subnorm_value, IGNORE, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, plus_infty, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, minus_infty, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, plus_zero, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, minus_zero, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, 1000, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, max_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, -max_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, min_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, -min_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, min_subnorm_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > + TEST_fp_f (getpayload, -min_subnorm_value, -1.0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), OK. Add test for variants. > #if HIGH_ORDER_BIT_IS_SET_FOR_SNAN > TEST_fp_f (getpayload, snan_value_pl ("0x0"), plus_zero, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > TEST_fp_f (getpayload, -snan_value_pl ("0x0"), plus_zero, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED), > diff --git a/sysdeps/ieee754/dbl-64/s_getpayload.c b/sysdeps/ieee754/dbl-64/s_getpayload.c > index 3ab89ddd66..5a055be35a 100644 > --- a/sysdeps/ieee754/dbl-64/s_getpayload.c > +++ b/sysdeps/ieee754/dbl-64/s_getpayload.c > @@ -27,6 +27,9 @@ __getpayload (const double *x) > { > uint32_t hx, lx; > EXTRACT_WORDS (hx, lx, *x); > + if ((hx & 0x7ff00000) != 0x7ff00000 > + || ((hx & 0xfffff) | lx) == 0) > + return -1; OK. > hx &= 0x7ffff; > uint64_t ix = ((uint64_t) hx << 32) | lx; > if (FIX_INT_FP_CONVERT_ZERO && ix == 0) > diff --git a/sysdeps/ieee754/dbl-64/wordsize-64/s_getpayload.c b/sysdeps/ieee754/dbl-64/wordsize-64/s_getpayload.c > index 2c887b93b7..eba96d0c77 100644 > --- a/sysdeps/ieee754/dbl-64/wordsize-64/s_getpayload.c > +++ b/sysdeps/ieee754/dbl-64/wordsize-64/s_getpayload.c > @@ -26,6 +26,9 @@ __getpayload (const double *x) > { > uint64_t ix; > EXTRACT_WORDS64 (ix, *x); > + if ((ix & 0x7ff0000000000000ULL) != 0x7ff0000000000000ULL > + || (ix & 0xfffffffffffffULL) == 0) > + return -1; OK. > ix &= 0x7ffffffffffffULL; > return (double) ix; > } > diff --git a/sysdeps/ieee754/flt-32/s_getpayloadf.c b/sysdeps/ieee754/flt-32/s_getpayloadf.c > index e1d22473ff..1baad4847e 100644 > --- a/sysdeps/ieee754/flt-32/s_getpayloadf.c > +++ b/sysdeps/ieee754/flt-32/s_getpayloadf.c > @@ -27,6 +27,9 @@ __getpayloadf (const float *x) > { > uint32_t ix; > GET_FLOAT_WORD (ix, *x); > + if ((ix & 0x7f800000) != 0x7f800000 > + || (ix & 0x7fffff) == 0) > + return -1; OK. > ix &= 0x3fffff; > if (FIX_INT_FP_CONVERT_ZERO && ix == 0) > return 0.0f; > diff --git a/sysdeps/ieee754/ldbl-128/s_getpayloadl.c b/sysdeps/ieee754/ldbl-128/s_getpayloadl.c > index db55a00f66..1fbe704608 100644 > --- a/sysdeps/ieee754/ldbl-128/s_getpayloadl.c > +++ b/sysdeps/ieee754/ldbl-128/s_getpayloadl.c > @@ -26,6 +26,9 @@ __getpayloadl (const _Float128 *x) > { > uint64_t hx, lx; > GET_LDOUBLE_WORDS64 (hx, lx, *x); > + if ((hx & 0x7fff000000000000ULL) != 0x7fff000000000000ULL > + || ((hx & 0xffffffffffffULL) | lx) == 0) > + return -1; OK. > hx &= 0x7fffffffffffULL; > /* Construct the representation of the return value directly, since > 128-bit integers may not be available. */ > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_getpayloadl.c b/sysdeps/ieee754/ldbl-128ibm/s_getpayloadl.c > index 3b5a1a8414..abbd694eea 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_getpayloadl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_getpayloadl.c > @@ -27,6 +27,9 @@ __getpayloadl (const long double *x) > double xhi = ldbl_high (*x); > uint64_t ix; > EXTRACT_WORDS64 (ix, xhi); > + if ((ix & 0x7ff0000000000000ULL) != 0x7ff0000000000000ULL > + || (ix & 0xfffffffffffffULL) == 0) > + return -1; OK. > ix &= 0x7ffffffffffffULL; > if (FIX_INT_FP_CONVERT_ZERO && ix == 0) > return 0.0L; > diff --git a/sysdeps/ieee754/ldbl-96/s_getpayloadl.c b/sysdeps/ieee754/ldbl-96/s_getpayloadl.c > index 8f09cb74c8..761bd69b58 100644 > --- a/sysdeps/ieee754/ldbl-96/s_getpayloadl.c > +++ b/sysdeps/ieee754/ldbl-96/s_getpayloadl.c > @@ -27,6 +27,9 @@ __getpayloadl (const long double *x) > uint16_t se __attribute__ ((unused)); > uint32_t hx, lx; > GET_LDOUBLE_WORDS (se, hx, lx, *x); > + if ((se & 0x7fff) != 0x7fff > + || ((hx & 0x7fffffff) | lx) == 0) > + return -1; OK. > hx &= 0x3fffffff; > uint64_t ix = ((uint64_t) hx << 32) | lx; > return (long double) ix; > -- Cheers, Carlos.