On Tue, Jan 04, 2011 at 03:51:37PM +0000, Peter Maydell wrote:
> On 4 January 2011 15:15, Aurelien Jarno <[email protected]> wrote:
> > Use bits32 instead of uint32 when manipulating floating point values
> > directly for consistency reasons.
>
> I'm not convinced this patch is particularly worthwhile, especially since
> Andreas is working on a patchset which will convert all the bits32
> uses back into uint32_t anyway, which is the direction to go if we
> want to make the fpu/ code consistent about its type usage.
I don't know in which direction we should go (bits32 or uint32_t), but
we should go for more consistency. When everything is consistent, it's
just a regex to go switch the type.
> If you do want to do this, the commit message should be "uint32_t"
> not "uint32" (which is a different type!)
Correct.
> > int float32_is_quiet_nan( float32 a1 )
> > {
> > float32u u;
> > - uint64_t a;
> > + bits32 a;
> > u.f = a1;
> > a = u.i;
> > return ( 0xFF800000 < ( a<<1 ) );
>
> This change is actually changing the type: shouldn't it be bits64 ?
Yes, I should have mentioned it in the changelog. For me this looks like
a typo, as we are manipulating 32 bits values. Look at
float32_is_signaling_nan().
> It seems a bit inconsistent to change
> softfloat-native.c:float32_is_quiet_nan()
> but not softfloat-native.c:float64_is_quiet_nan() (which uses uint64_t).
I guess you meant softfloat-native.c:float32_is_quiet_nan(). It looks
like I missed this one, and that it should be changed too.
> Personally I'd just drop this patch.
I'll drop it for now and wait to see what Andreas offers.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
[email protected] http://www.aurel32.net
