From patchwork Mon Nov 30 14:53:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marius Hillenbrand X-Patchwork-Id: 41234 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 23DF23851C09; Mon, 30 Nov 2020 14:54:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 23DF23851C09 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1606748051; bh=mlhANWM2+tMPeMsXHf3QjomVuOP0mBTlBTZB1xP3bAU=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=xmQ6X8fpKTl89dgP5FWrER9ypBoOc+AnJsO2BnL8ZQ/8G/Eni+cTkjU82WH11BkVH 7OoQfCxU5XlygOjy6n4ClbD7o2Vj8htu2O7nrmwLmTDb6Yr7BAHM3xkXCZvcGBEsWr 5IXWsoxHZnywoMsRLIhXCGvnnoJ0XbdJshq20RmY= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from oc3353602072.ibm.com (114-221-018-212.ip-addr.vsenet.de [212.18.221.114]) by sourceware.org (Postfix) with ESMTP id 1B86E3851C09 for ; Mon, 30 Nov 2020 14:54:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1B86E3851C09 Received: by oc3353602072.ibm.com (Postfix, from userid 1000) id E75BB1760252; Mon, 30 Nov 2020 15:54:06 +0100 (CET) To: libc-alpha@sourceware.org, marius.hillenbrand@ibm.com Subject: [PATCH] S390: Derive float_t from FLT_EVAL_METHOD Date: Mon, 30 Nov 2020 15:53:59 +0100 Message-Id: <20201130145359.2831-1-mhillen@linux.ibm.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, KHOP_HELO_FCRDNS, PDS_RDNS_DYNAMIC_FP, RDNS_DYNAMIC, SPF_HELO_NONE, SPF_NONE, SUBJ_OBFU_PUNCT_FEW, SUBJ_OBFU_PUNCT_MANY, 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-Patchwork-Original-From: Marius Hillenbrand via Libc-alpha From: Marius Hillenbrand Reply-To: Marius Hillenbrand Cc: Stefan Liebler Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Hi, float_t should represent the type that is used to evaluate float expressions internally. On s390(x), float_t is currently set to double. In contrast, the isa supports single-precision float operations and compilers by default evaluate float in single precision, which violates the C standard (sections 5.2.4.2.2 and 7.12 in C11/C17). With -fexcess-precision=standard, gcc evaluates float in double precision, which aligns with the standard yet at the cost of added conversion instructions. This patch changes the definition of float_t to be derived from the compiler's __FLT_EVAL_METHOD__ and thereby align with the C standard -- that is, drop the s390-specific definition and apply default behavior. For background: The port of glibc to s390 incorrectly deferred to the generic definitions which, back then, tied float_t to double. Since then, this definition has been kept to avoid ABI changes, most recently in the refactoring of float_t into bits/flt-eval-method.h https://sourceware.org/legacy-ml/libc-alpha/2016-11/msg00903.html and the discussion around https://gcc.gnu.org/legacy-ml/gcc-patches/2016-09/msg02392.html Given the performance overhead, I have reevaluated the impact of turning float_t into float and only found two packages, ImageMagick and clucene, that use float_t in their API, out of >130k Debian source packages scanned. To avoid breaking ABI changes, I patched these packages to avoid their reliance on float_t (in ImageMagick since 7.0.10-39, patch in https://github.com/ImageMagick/ImageMagick/pull/2832 - patch for clucene in https://sourceforge.net/p/clucene/bugs/233). With these measures in place, we should get rid of the inconsistent definition of float_t for s390. Marius ------------>8------------>8------------>8------------>8------------ float_t supposedly represents the type that is used to evaluate float expressions internally. While the isa supports single-precision float operations, the port of glibc to s390 incorrectly deferred to the generic definitions which, back then, tied float_t to double. gcc by default evaluates float in single precision, so that scenario violates the C standard (sections 5.2.4.2.2 and 7.12 in C11/C17). With -fexcess-precision=standard, gcc evaluates float in double precision, which aligns with the standard yet at the cost of added conversion instructions. With this patch, we drop the s390-specific definition of float_t and defer to the default behavior, which aligns float_t with the compiler-defined FLT_EVAL_METHOD in a standard-compliant way. Checked on s390x-linux-gnu with 31-bit and 64-bit builds. --- NEWS | 7 +++++++ sysdeps/s390/bits/flt-eval-method.h | 24 ------------------------ 2 files changed, 7 insertions(+), 24 deletions(-) delete mode 100644 sysdeps/s390/bits/flt-eval-method.h diff --git a/NEWS b/NEWS index 685b93c8e9..6bec44d807 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,13 @@ Deprecated and removed features, and other changes affecting compatibility: program is now installed in the /usr/bin subdirectory. Previously, the /usr/sbin subdirectory was used. +* On s390(x), the type float_t is now derived from the macro + __FLT_EVAL_METHOD__ that is defined by the compiler, instead of being + hardcoded to double. This does not affect the ABI of any libraries + that are part of the GNU C Library, but may affect the ABI of other + libraries that use this type in their interfaces. The new definition + improves consistency with compiler behavior in many scenarios. + Changes to build and runtime requirements: * On Linux, the system administrator needs to configure /dev/pts with diff --git a/sysdeps/s390/bits/flt-eval-method.h b/sysdeps/s390/bits/flt-eval-method.h deleted file mode 100644 index 826bbfec2f..0000000000 --- a/sysdeps/s390/bits/flt-eval-method.h +++ /dev/null @@ -1,24 +0,0 @@ -/* Define __GLIBC_FLT_EVAL_METHOD. S/390 version. - Copyright (C) 2016-2020 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#ifndef _MATH_H -# error "Never use directly; include instead." -#endif - -/* This value is used because of a historical mistake. */ -#define __GLIBC_FLT_EVAL_METHOD 1