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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 5F4663952000 for ; Wed, 24 Feb 2021 17:04:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5F4663952000 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-529-ZXCkJug0M0Si5XQgPqFSOg-1; Wed, 24 Feb 2021 12:04:16 -0500 X-MC-Unique: ZXCkJug0M0Si5XQgPqFSOg-1 Received: by mail-qt1-f200.google.com with SMTP id n4so2103066qte.11 for ; Wed, 24 Feb 2021 09:04:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=ZJhIzCRxJ3G+YFcWAOLBvkVnGf6qRmqcAFehH+pVkKo=; b=eyiqSXdHq8y0WUY2S3FwgWi6V2NvEIULAfzF+FzzBIXmpVPTFSzwwAAU4QsarmMUnS FVlS7NCxOErmx9JMBbNpv9o9P8wOW87buHa6D2WnF5nau80qjoAzsGwIa3AEK2SCYdxA eCNQuaNnrZvSsHBukchL7F8lGHN6j6qL1IzZ7YTwCr1d3olbskcbChr4la1VMLcWAb9K +gKDvWWeBfjl0rh8WbylBiyzlo8InTcNOCnXkgswmGzyd/cJo+n/vB83getiUr2AjfD0 7bPF551twjk8wgjNteEDxdpEZGAPfmTelKd8Ws5inem/G7QUgXZVJEBCxizgf2qMuZx4 ZRxg== X-Gm-Message-State: AOAM533hKg6Rc0LBxQSsN/vnUd6aWCCyfmaWSKffb7BfA+SFEdDBtVhG 7lhJMZdrn+ILhkBzj86N/0/9QqwEWkyTHeGC2VK++rPsGybqho3/y9do8eFzTorgW4yFtDDIIuO sUDK7TOWEQq+JMYk= X-Received: by 2002:a37:8b46:: with SMTP id n67mr31919744qkd.167.1614186256391; Wed, 24 Feb 2021 09:04:16 -0800 (PST) X-Google-Smtp-Source: ABdhPJwHQz45XuOC3zQH6LwUh10C8J3KkuPAgOAlaTPC2e7tv3XdXRrxn2lgfa4n5KJippfGweXdZw== X-Received: by 2002:a37:8b46:: with SMTP id n67mr31919713qkd.167.1614186256127; Wed, 24 Feb 2021 09:04:16 -0800 (PST) Received: from [192.168.1.130] (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id w53sm1745834qtb.54.2021.02.24.09.04.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Feb 2021 09:04:15 -0800 (PST) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Wed, 24 Feb 2021 12:04:14 -0500 (EST) To: Jonathan Wakely cc: Patrick Palka , libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [PATCH 1/2] libstdc++: Robustify long double std::to_chars testcase [PR98384] In-Reply-To: <20210224164404.GW3008@redhat.com> Message-ID: <278c11aa-5d75-bdd2-bb9-4bd5d6c12f75@idea> References: <20210222215559.1834935-1-ppalka@redhat.com> <20210224164404.GW3008@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Feb 2021 17:04:24 -0000 On Wed, 24 Feb 2021, Jonathan Wakely wrote: > On 23/02/21 11:30 -0500, Patrick Palka via Libstdc++ wrote: > > On Mon, 22 Feb 2021, Patrick Palka wrote: > > > > > This makes the hexadecimal section of the long double std::to_chars > > > testcase more robust by avoiding false-negative FAILs due to printf > > > using a different leading hex digit than us, and by additionally > > > verifying the correctness of the hexadecimal form via round-tripping > > > through std::from_chars. > > > > > > Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64. Does this > > > look OK for trunk? > > > > The commit message could explain the issue better, so here's v2 with a > > more detailed commit message. > > > > -- >8 -- > > > > Subject: [PATCH] libstdc++: Robustify long double std::to_chars testcase > > [PR98384] > > > > The long double std::to_chars testcase currently verifies the > > correctness of its output by comparing it to that of printf, so if > > there's a mismatch between to_chars and printf, the test FAILs. This > > works well for the scientific, fixed and general formatting modes, > > because the corresponding printf conversion specifiers (%e, %f and %g) > > are rigidly specified. > > > > But this doesn't work so well for the hex formatting mode because the > > corresponding printf conversion specifier %a is more flexibly specified. > > For instance, the hexadecimal forms 0x1p+0, 0x2p-1, 0x4p-2 and 0x8p-3 > > are all equivalent and valid outputs of the %a specifier for the number > > 1. The apparent freedom here is the choice of leading hex digit -- the > > standard just requires that the leading hex digit is nonzero for > > normalized numbers. > > > > Currently, our hexadecimal formatting implementation uses 0/1/2 as the > > leading hex digit for floating point types that have an implicit leading > > mantissa bit which in practice means all supported floating point types > > except x86 long double. The latter type has a 64 bit mantissa with an > > explicit leading mantissa bit, and for this type our implementation uses > > the most significant four bits of the mantissa as leading hex digit. > > This seems to be consistent with most printf implementations, but not > > all, as PR98384 illustrates. > > > > In order to avoid false-positive FAILs due to arbitrary disagreement > > between to_chars and printf about the choice of leading hex digit, this > > patch makes the testcase's verification via printf conditional on the > > leading hex digits first agreeing. An additional verification step is > > also added: round-tripping the output of to_chars through from_chars > > should yield the original value. > > > > Tested on x86, x86_64, powerpc64be, powerpc64le and aarch64. Does this > > look OK for trunk? > > > @@ -50,6 +51,38 @@ namespace detail > > void > > test01() > > { > > + // Verifies correctness of the hexadecimal form [BEGIN,END) for VALUE by > > + // round-tripping it through from_chars (if available). > > + auto verify_via_from_chars = [] (char *begin, char *end, long double > > value) { > > +#if __cpp_lib_to_chars >= 201611L || _GLIBCXX_HAVE_USELOCALE > > This is currently going to fail, because we don't actually define > __cpp_lib_to_chars yet (we should fix that!) > > Is checking the feature test macro here useful? We know that > floating-point from_chars was committed before to_chars, so if this > test is running, we should have from_chars (modulo uselocale being > available, so that check is right). Is this to make the test usable > for other C++ std::lib implementations? This preprocessor check is copied from from_chars/{5,6}.cc, which I figured should be appropriate to use here as well. I figured we'd want to adjust each of these checks after we define __cpp_lib_to_chars appropriately anyway (e.g. if __cpp_lib_to_chars is conditioned on uselocale being available, then the three tests should be changed just look at __cpp_lib_to_chars, IIUC). > > > + long double roundtrip; > > + auto result = from_chars(begin, end, roundtrip, chars_format::hex); > > + VERIFY( result.ec == errc{} ); > > + VERIFY( result.ptr == end ); > > + VERIFY( roundtrip == value ); > > +#endif > >