Hi!
On Fri, 31 May 2013 13:34:12 +0100, Peter Maydell <peter.mayd...@linaro.org>
wrote:
> On 31 May 2013 13:07, Michael Tokarev <m...@tls.msk.ru> wrote:
> > Hmm. And where's the simplification? Here's context diff for the same:
> >
> > *** fpu/softfloat-specialize.h.orig 2013-05-31 16:02:51.614710351 +0400
> > --- fpu/softfloat-specialize.h 2013-05-31 16:02:59.838820308 +0400
> > ***************
> > *** 936,946 ****
> > if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid
> > STATUS_VAR);
> > ! if ( a.low >> 63 ) {
> > ! z.sign = a.high >> 15;
> > ! z.low = 0;
> > ! z.high = a.low << 1;
> > ! } else {
> > ! z.sign = floatx80_default_nan_high >> 15;
> > ! z.low = 0;
> > ! z.high = floatx80_default_nan_low << 1;
> > }
> > return z;
> > --- 936,945 ----
> > if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid
> > STATUS_VAR);
> > ! /* Replace a Pseudo NaN with a default NaN. */
> > ! if (!(a.low >> 63)) {
> > ! a.low = floatx80_default_nan_low;
> > ! a.high = floatx80_default_nan_high;
> > }
> > + z.sign = a.high >> 15;
> > + z.low = 0;
> > + z.high = a.low << 1;
> > return z;
> >
> >
> > Yes, your version is 3 lines shorter, because it
> > does not have extra else{} (2 lines) and the
> > remaining if() construct is one line shorter too,
> > due to moving z.low=0 construct into common place.
> >
> > But I don't think your version is more readable, --
> > before it was easy to understand what is going on,
> > we had two easy case with all right stuff done for
> > each case. Now we do some preparation before, so
> > the common case works.
>
> I think you could make a reasonable argument for this
> change being an improvement, because it makes it clear
> what we're doing: if what we have is an x86 pseudo-NaN,
> we replace it with the 80-bit default NaN, and then
> proceed to do 80-bit-to-canonical conversion in the
> usual way. The comment also explains why this if()
> exists for the 80 bit case when it doesn't for the
> equivalent 32 bit and 64 bit functions. As a code
> change I actually quite like it.Yes, this exactly is my reasoning: first, convert a x86 Pseudo NaN into a generic NaN, then do the floating-point type conversion itself). I thought this would be obvious (and hence the patch trivial) -- hey, I even added a comment! -- but apparently what is obvious/trivial to one isn't to the other. :-) > That said, I think any new patches to fpu/ need to > come with an explicit statement that they can be > licensed under the softfloat-2a license or GPLv2 > or BSD (etc etc) so they aren't an obstacle to > the softfloat-2a-to-2b conversion that is in the works. > [cc'd Anthony so he can correct me if I'm wrong.] I hereby place this one contribution (which I think wouldn't constitute a copyrightable change anyway) into the Public Domain, allowing any kind of usage. Grüße, Thomas
pgptNwmltXUR2.pgp
Description: PGP signature
