From: Rich Felker <dalias@libc.org>
To: Marius Hillenbrand <mhillen@linux.ibm.com>
Cc: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] s390x: derive float_t from compiler or default to float
Date: Wed, 2 Dec 2020 11:01:40 -0500 [thread overview]
Message-ID: <20201202160140.GW534@brightrain.aerifal.cx> (raw)
In-Reply-To: <20201202142504.GV534@brightrain.aerifal.cx>
On Wed, Dec 02, 2020 at 09:25:04AM -0500, Rich Felker wrote:
> On Wed, Dec 02, 2020 at 11:44:59AM +0100, Marius Hillenbrand wrote:
> > On 12/1/20 9:50 PM, Rich Felker wrote:
> > > On Tue, Dec 01, 2020 at 03:36:34PM +0100, Marius Hillenbrand wrote:
> > >> 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. To improve standards compliance, this patch changes the
> > >> definition of float_t to be derived from the compiler's
> > >> __FLT_EVAL_METHOD__.
> > >>
> > >> 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
> > >> musl apparently adopted the definition from glibc.
> > >>
> > >> Given the performance overhead and reduced standards compliance, I have
> > >> reevaluated cleaning up the special behavior on s390x. I found only 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).
> > >>
> > >> gcc-11 will drop the special case to retrofit double
> > >> precision behavior for -fexcess-precision=standard so that
> > >> __FLT_EVAL_METHOD__ will be 0 on s390x in any scenario.
> > >> https://gcc.gnu.org/pipermail/gcc-patches/2020-November/560224.html
> > >> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=a5dd6b69fcbe74c02d4821ac2daf2b8c9f819f6e
> > >>
> > >> glibc 2.33 will most likely adopt the same behavior as in this patch, so
> > >> that float_t will eventually be float on s390x in any scenario.
> > >> https://sourceware.org/pipermail/libc-alpha/2020-November/120212.html
> > >>
> > >> Testing with libc-test showed no regressions. Failing testcases
> > >> src/math/lgammaf[_r].exe succeed with the patch.
> > >>
> > >> Please review and consider merging this patch.
> > >
> > > Thanks for the detailed report. To be clear, all models/ISA-levels
> > > support the single-precision ops and future GCC will always use them
> > > even with -fexcess-precision=standard, but old ones switch to using
> > > double precision ops with -fexcess-precision=standard to meet the
> > > contract of evaluating in (old definition of) float_t. Is this
> > > correct?
> >
> > Yes, your summary is correct -- with one exception that I omitted in my
> > original post: future GCC compiled against current libc will still
> > switch to using double precision ops with -fexcess-precision=standard to
> > match the old definition of float_t. When future GCC detects a future
> > libc at compile-time, it will always use single-precision ops. Without
> > that switch, updating GCC while keeping your current libc would have
> > worsened the situation wrt the C standard.
>
> How does this "detecting an updated libc" take place? That sounds like
> it could be really problematic...
I'm looking at
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/560225.html
which seems to be what you're talking about, and don't understand how
it's intended to work. It looks like it's running a test for target
behavior on the host compiler (there is no target compiler at the
point this test is run). Looking again, I guess that's why it's under
a condition for build==host==target. What happens when cross
compiling? Do you get the old behavior unless manually setting
--disable-s390-excess-float-precision?
Also I guess this mildly breaks use of a libc older than the one the
compiler was built for, but that's probably the case in general with
GCC for various other reasons too.
Rich
next prev parent reply other threads:[~2020-12-02 16:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-01 14:36 Marius Hillenbrand
2020-12-01 20:50 ` Rich Felker
2020-12-02 10:44 ` Marius Hillenbrand
2020-12-02 14:25 ` Rich Felker
2020-12-02 16:01 ` Rich Felker [this message]
2020-12-02 17:09 ` Marius Hillenbrand
2020-12-02 19:13 ` Rich Felker
2020-12-03 16:53 ` Marius Hillenbrand
2020-12-03 19:06 ` Rich Felker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201202160140.GW534@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=mhillen@linux.ibm.com \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).