From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81420 invoked by alias); 30 Oct 2019 16:10:38 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 80032 invoked by uid 89); 30 Oct 2019 16:10:38 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=H*RU:sk:smtpout, HX-Spam-Relays-External:sk:smtpout, H*F:D*br, our X-HELO: smtpout1.mo803.mail-out.ovh.net Date: Wed, 30 Oct 2019 16:10:00 -0000 From: "Gabriel F. T. Gomes" To: Paul E Murphy CC: Subject: Re: [PATCH v2 01/30] ldbl-128ibm-compat: Add regular character printing functions Message-ID: <20191030131011.41ea6092@tereshkova> In-Reply-To: <013053c7-d6ce-6e4f-4dfd-799ffe1951f3@linux.ibm.com> References: <20191025153410.15405-1-gabriel@inconstante.net.br> <20191025153410.15405-2-gabriel@inconstante.net.br> <013053c7-d6ce-6e4f-4dfd-799ffe1951f3@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Ovh-Tracer-Id: 13109415567587331640 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedufedruddtfedgkeejucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecuhedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm X-SW-Source: 2019-10/txt/msg00924.txt.bz2 Hi, Paul, On Tue, 29 Oct 2019, Paul E Murphy wrote: >On 10/25/19 10:33 AM, Gabriel F. T. Gomes wrote: >> >> diff --git a/elf/tst-addr1.c b/elf/tst-addr1.c >> index 68ff74aabd..ee81acda5b 100644 >> --- a/elf/tst-addr1.c >> +++ b/elf/tst-addr1.c >> @@ -19,7 +19,12 @@ do_test (void) >> rather than in the binary. printf and _IO_printf >> are aliased and which one comes first in the >> hash table is up to the linker. */ >> - && strcmp (i.dli_sname, "_IO_printf") != 0); >> + && strcmp (i.dli_sname, "_IO_printf") != 0 >> + /* On architectures where long double with IEEE binary128 >> + format is available as a third option (initially, true >> + for powerpc64le), printf may be redirected to >> + __printfieee128. */ >> + && strcmp (i.dli_sname, "__printfieee128") != 0); > >Should there be a guard against this test for architectures which will >never support this symbol? Yeah, sounds good so that we avoid unintended redirections. On the other hand, this is also a residue from our initial development effort, when we added -mabi=ieeelongdouble to a lot more files, than what I have today. Today, this test case is not built with -mabi=ieeelongdouble, so I think this hunk should simply go away. >> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c >> new file mode 100644 >> index 0000000000..5de4ea3e7f >> --- /dev/null >> +++ b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c >> @@ -0,0 +1 @@ >> +#include >> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c >... >> + long double ld = -1; > >Is this the best value to use for compat tests? Would a value which >produces unique output for the respective format if the wrong compiler >flags are used? Or, maybe an extra header in *-ibm128.c and *-ieee128.c >variants to sanity check __LDBL_MANT_DIG__? My rationale for using this value is that it makes it simple to debug when something goes wrong. Since the implementation for these functions reuse the code paths added for _Float128, I didn't think it would be useful to test for more values again (the _Float128 implementation already checks for many values). This test is telling me that the parameters are being passed and read correctly (on the powerpc64le case, that they are going through VSX registers, as opposed to two FP registers for IBM long double). >> + This is used by the Versions and math_ldbl_opt.h files in >> + sysdeps/ieee754/ldbl-128ibm-compat/. It gives the ABI version where >> + long double == ibm128 was replaced with long double == _Float128 >> + for libm *l functions and libc functions using long double. */ >> + >> +#define LDBL_IBM128_VERSION GLIBC_2.31 >> +#define LDBL_IBM128_COMPAT_VERSION GLIBC_2_31 > >Should this part of the change be held off until all ldbl == ieee128 >changes are in? I guess you're right. I moved it to the last commit, which actually adds sysdeps/ieee754/ldbl-128ibm-compat to the Implies file. Thanks, Gabriel