| Message ID | 87ehhekyru.fsf@talisman.default |
|---|---|
| State | New |
| Headers | show |
BTW, I'm not sure it's right to be using *_muladd for MIPS. MADD.fmt & co. were fused operations in the early MIPS IV processors, but they've had an intermediate rounding step since then (i.e. they're equivalent to a separate multiplication and addition). I'm not feeling brave enough to tackle that though. Richard
On 21 January 2013 21:32, Richard Sandiford <rdsandiford@googlemail.com> wrote: > Honour float_muladd_negate_c in the case where the product is zero and > c is nonzero. Previously we would fail to negate c. > > Seen in (and tested against) the gfortran testsuite on MIPS. > > Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com> > --- > fpu/softfloat.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index ac3d150..0028415 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -2234,6 +2234,9 @@ float32 float32_muladd(float32 a, float32 b, float32 c, int flags STATUS_PARAM) > } > } > /* Zero plus something non-zero : just return the something */ > + if (flags & float_muladd_negate_c) { > + signflip ^= 1; > + } > return make_float32(float32_val(c) ^ (signflip << 31)); This is a correct change in that it fixes a definite bug and gives the right results, but I wonder if it might be clearer to instead change the return to read: return packFloat32(cSign ^ signflip, cExp, cSig); ? That would mean we consistently handle the negate_c flag by: * flip cSign as soon as we split c into its component fields * never refer to c again (the source of the bug here is me trying to be clever and avoid reassembling the float, but forgetting that cSign might have changed.) -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 21 January 2013 21:32, Richard Sandiford <rdsandiford@googlemail.com> wrote: >> Honour float_muladd_negate_c in the case where the product is zero and >> c is nonzero. Previously we would fail to negate c. >> >> Seen in (and tested against) the gfortran testsuite on MIPS. >> >> Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com> >> --- >> fpu/softfloat.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/fpu/softfloat.c b/fpu/softfloat.c >> index ac3d150..0028415 100644 >> --- a/fpu/softfloat.c >> +++ b/fpu/softfloat.c >> @@ -2234,6 +2234,9 @@ float32 float32_muladd(float32 a, float32 b, float32 c, int flags STATUS_PARAM) >> } >> } >> /* Zero plus something non-zero : just return the something */ >> + if (flags & float_muladd_negate_c) { >> + signflip ^= 1; >> + } >> return make_float32(float32_val(c) ^ (signflip << 31)); > > This is a correct change in that it fixes a definite bug and gives > the right results, but I wonder if it might be clearer to instead > change the return to read: > > return packFloat32(cSign ^ signflip, cExp, cSig); > > ? > > That would mean we consistently handle the negate_c flag by: > * flip cSign as soon as we split c into its component fields > * never refer to c again > > (the source of the bug here is me trying to be clever and avoid > reassembling the float, but forgetting that cSign might have > changed.) Heh, wondered about that, but not knowing the code, I went for the "safe" option. I'll do it as you suggest, thanks. Richard
Richard Sandiford <rdsandiford@googlemail.com> writes: > BTW, I'm not sure it's right to be using *_muladd for MIPS. MADD.fmt & > co. were fused operations in the early MIPS IV processors, but they've > had an intermediate rounding step since then (i.e. they're equivalent > to a separate multiplication and addition). I'm not feeling brave > enough to tackle that though. Actually, this turned out to be the cause of the remaining gfortran testsuite failures. Patch coming up... Richard
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index ac3d150..0028415 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -2234,6 +2234,9 @@ float32 float32_muladd(float32 a, float32 b, float32 c, int flags STATUS_PARAM) } } /* Zero plus something non-zero : just return the something */ + if (flags & float_muladd_negate_c) { + signflip ^= 1; + } return make_float32(float32_val(c) ^ (signflip << 31)); } @@ -3787,6 +3790,9 @@ float64 float64_muladd(float64 a, float64 b, float64 c, int flags STATUS_PARAM) } } /* Zero plus something non-zero : just return the something */ + if (flags & float_muladd_negate_c) { + signflip ^= 1; + } return make_float64(float64_val(c) ^ ((uint64_t)signflip << 63)); }
Honour float_muladd_negate_c in the case where the product is zero and c is nonzero. Previously we would fail to negate c. Seen in (and tested against) the gfortran testsuite on MIPS. Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com> --- fpu/softfloat.c | 6 ++++++ 1 file changed, 6 insertions(+)