Re: [Qemu-devel] [PATCH v2 2/8] softfloat: use bits32 instead of uint32

[Aurelien Jarno]

On Tue, Jan 04, 2011 at 03:51:37PM +0000, Peter Maydell wrote:
> On 4 January 2011 15:15, Aurelien Jarno <address@hidden> 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
address@hidden                 http://www.aurel32.net