On Mon, 5 Feb 2018, vladimir.mezent...@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
jos...@codesourcery.com
