This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR libgcc/59714 complex division is surprising on aarch64


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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]