Re: [hatari-devel] Bug in FPU code

[Andreas Grabher <andreas.grabher@xxxxxxxxxxxx>]

I'm happy my patch is useful!
About the endianness and long double issue:
I think this code will work on all little endian systems as long as the size of long double is >= 10 bytes. Bigger size should not be a problem (for example on my system long double is defined 16 bytes long as reported by printf("Size: %lu\n", sizeof(long double);

For big endian systems we would need a slightly different function and also take into account the actual size of long double. Please correct me if i'm wrong.

I think a bigger problem than endianness will be systems that define long double as < 10 bytes.


Am 05.03.2012 um 22:44 schrieb Thomas Huth:

> Am Mon, 05 Mar 2012 22:05:34 +0100
> schrieb Laurent Sallafranque <laurent.sallafranque@xxxxxxx>:
> 
>> Hi,
>> 
>> Eero, Thomas, Nicolas, I've tested the patch with the esquimau demo,
>> but I haven't seen any change. (Eskimau does a lot of FPU).
>> 
>> As I'm not a specialist of #if USE_LONG_DOUBLE defines and the code 
>> after, I prefer asking you your advice on this patch before uploading 
>> anything.
>> 
>> Is the fix OK for any case of #if USE_LONG_DOUBLE ? (on every
>> computer, ... little / big endian, ...) ?
>> 
>> At least, the patch doesn't seem to break anything, but I've done
>> only one test.
> 
> Well, by closely looking at the source code, you can see that the
> original code is obviously wrong:
> 
>   ... long double src ...
>   register uae_u16 *finalword = (uae_u16 *)(&src + 8);
> 
> Here they "try" to access the memory at location &src + 8 ... which
> should obviously be 8 _bytes_ after &src, but since &src is a "long
> double" pointer, they rather access the memory at 8 * 8 = 64 bytes
> after &src.
> 
> So yes, please apply the patch, IMHO Andreas did a very good job here
> by spotting this bug. As an alternative, you could also cast &src to
> a "uint8_t *" before adding the "8", that should fix this issue, too.
> 
> And don't worry about breaking the endianess, the original code here was
> not endianess safe, either, so the endianess issues should maybe be
> fixed with another patch later.
> 
> Thomas
> 
> 
> 
>