This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64
- From: Joseph Myers <joseph at codesourcery dot com>
- To: <vladimir dot mezentsev at oracle dot com>
- Cc: <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 6 Feb 2018 17:13:35 +0000
- Subject: Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64
- Authentication-results: sourceware.org; auth=none
- References: <1516912467-13364-1-git-send-email-vladimir.mezentsev@oracle.com> <alpine.DEB.2.20.1801260005550.12943@digraph.polyomino.org.uk> <c1a5bfb5-8346-70e8-e1e7-13f9c05df852@oracle.com> <alpine.DEB.2.20.1801292046190.28694@digraph.polyomino.org.uk> <03c87b52-efb5-3d55-64f3-1d8cad8cb96d@oracle.com>
On Tue, 6 Feb 2018, vladimir.mezentsev@oracle.com wrote:
> I compared with __builtin_ilogb. There is a same performance degradation.
I'm not clear on exactly what you compared with.
Do you mean the *existing* __builtin_ilogb? That's only supported for
constant arguments that are not zero, infinity or NaN, and with an i386
optab for -funsafe-math-optimizations. So it's not a useful comparison at
all, because it would call a libm function.
Rather, if built-in functions were used for this, they'd be *new* built-in
functions. It would be entirely possible to implement __builtin_ilogb (at
least for -fno-math-errno) that actually expands inline, using target
hooks to provide the values of FP_ILOGB0 / FP_ILOGBNAN, though the
expansion might be too big to be generally useful.
But a built-in function specifically to extract the exponent field of a
floating-point number (without doing any biasing or adjusting for special
cases) would be a more plausible comparison. The existing description of
floating-point number formats in real.h includes specification of
signbit_ro and signbit_rw. It would be possible to have such a
specification of the location of an exponent field, where there's one that
can be located simply within the floating-point number, and then
machine-independent code to expand such a built-in function.
I'm not asserting that you should add such a function. I *am* asserting
that the patch proposal needs to include serious consideration of various
options such as:
* Adding such a built-in function.
* Adding predefined macros (conditional on -fbuilding-libgcc) that assert
that a particular floating-point mode has a particular IEEE format.
Such predefined macros have a clear precedent - we already have
__LIBGCC_%s_MANT_DIG__, __LIBGCC_HAS_%s_MODE__, __LIBGCC_%s_FUNC_EXT__,
__LIBGCC_%s_EXCESS_PRECISION__. So why not something to describe IEEE
properties of a machine mode?
The use of sfp-machine.h is problematic because, while having
sfp-machine.h implies use of at least some IEEE formats, there is no
implication the other way round: sfp-machine.h is only present if at least
one format uses software floating point, not if all use hardware floating
point (or use software floating point in libc not libgcc, or use software
floating point from fp-bit instead of soft-fp). That's not a particularly
sensible criterion for whether to use the new logic.
Furthermore, it's entirely possible that only some formats are IEEE -
thus, on powerpc64le, __LIBGCC_HAS_TF_MODE__ is true, and soft-fp.h is
present, but TFmode is actually IBM long double not IEEE binary128, so
your logic would handle that case incorrectly. Thus you need "mode X has
IEEE format Y" logic that avoids the case where mode X exists, and IEEE
format Y is supported, and sfp-machine.h exists, but mode X has another
format, because that's actually the case right now for TFmode on
powerpc64le.
Again, I'm not saying you should add such predefined macros - but at a
first look I think they are a better approach than the use of presence of
sfp-machine.h to indicate which modes have IEEE formats, and I think they
are among the options that any patch submission needs to discuss carefully
if rejecting.
--
Joseph S. Myers
joseph@codesourcery.com