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

[Joseph Myers <joseph at codesourcery dot com>]

On Mon, 5 Feb 2018, vladimir.mezentsev@oracle.com wrote:

> My implementation avoids these issues.
> I use soft-fp instead built-in functions because performance with built-in
> functions (like __builtin_ilogb) is in two times worse.
> Also libm is needed for built-in functions.

I think you still need to provide a more detailed explanation of the 
implementation approach (discussing the issues and choices made, how and 
why it's safe on all systems, how your code avoids any problems with 
subnormals, infinities and NaNs, etc.).

As far as I can tell, the argument for better performance compared to 
built-in functions is that a generally useful built-in function would need 
special handling of infinities, NaNs, zero and subnormals, which you 
avoid by e.g. extracting exponent fields directly.  This means that a 
detailed justification is needed of how it's safe for the exponent fields 
of such values to be passed straight through your various functions 
without special cases 0 - probably most of that should go in comments in 
the code explaining what happens for such values and why it successfully 
avoids overflow and underflow.

The files in libgcc/soft-fp must be verbatim copies of the master sources 
in glibc.  So you can't make any local changes to them, and if you think 
changes are needed you need to patch things upstream in glibc first, with 
a proper extended explanation of why the fix is needed and why it is safe 
and the appropriate design for the fix.  There's nothing at all in this 
patch submission to explain that change.

You shouldn't have any commented-out code like your "+//#include 
"soft-fp/soft-fp.h"" in the submitted patch.

All the new inline functions need more precise comments explaining what 
their semantics are in cases such as zero, subnormals, infinities, etc., 
or stating that the semantics are that such arguments must not be passed 
to those functions.  (They also need to be formatted according to the GNU 
Coding Standards - space before '(', function name starts a new line.)

> +/* Return an exponent N, such that neither (2**(ce-N))**2 nor (2**(de-N))**2
> +   cause an overflow. Also try to avoid underflow. */

So (for example), for this comment, I'd expect information about: what is 
the valid range of exponents N that this function may return (given that, 
I suppose, you want 2**N to be a representable floating-point value), and 
what are the valid ranges of CE and DE for which this function guarantees 
to achieve those semantics with an exponent within the required range?  
And are those semantics strictly for the actual numbers specified, or, 
given how a subnormal value may result in an exponent -EXPBIAS here 
despite the actual exponent being smaller, are there additional semantics 
required in the case where CE or DE is -EXPBIAS?

Once there are comments on these functions giving all the required 
semantics, we can start to reason about whether those semantics ensure the 
required higher-level properties (regarding overflow / underflow in 
__div*c3 functions) in the case where subnormals are involved.  I'd expect 
such reasoning to appear in comments in the code.

-- 
Joseph S. Myers
joseph@codesourcery.com