View Issue Details

IDProjectCategoryView StatusLast Update
0000496LDMud 3.5Implementationpublic2011-11-21 23:11
Reporterhkremss Assigned Tozesstra  
PrioritynormalSeverityfeatureReproducibilityN/A
Status resolvedResolutionfixed 
Target Version3.5.0Fixed in Version3.5.0 
Summary0000496: double precision floats
DescriptionAs far as I can see from the source code, the only reason for the use of the 'special 48-Bit floating point' format was support of Classic Atari/Amiga platforms from good old MC68xxx days. While Atari seems to be dead, current Amigaish systems have gcc and full 64 bit double precision float support. Anyway support for Classic Atari/Amiga was dropped in 3.3, so why keeping it? I suggest to clean up svalue.h and the READ/SPLIT/STORE_DOUBLE macro stuff and use the 2 already existing 32 bit 'exponent' and 'mantissa' members to form a 'real double'. Shouldn't be too complicated. We could achieve higher math precision in LPC and probably get cleaner code as well.
TagsNo tags attached.

Relationships

related to 0000793 resolvedzesstra New savefile format supporting double precision for floats 

Activities

zesstra

2007-01-05 07:33

administrator   ~0000525

Mhmm, I might be wrong, but the exponent is currently 16 bit, so there is no existing 32 bit 'exponent' member (which might be changed of course).

I wonder, if it would be nowadays feasible, to implement an alternative FLOAT_FORMAT and directly use 'double' as a member in the svalue (and discard u.mantissa and x.exponent in that case), to save some calculation when using floats and only split into mantissa and exponent for storage in save_svalue()/restore_svalue()? Any comments on that idea?

hkremss

2007-01-06 10:36

reporter   ~0000526

> Mhmm, I might be wrong

No, you're right. My mistake to assume, that ph_int is 32 bit. To use double as member in svalue would be the best way, but it would change the size of 'union u' and svalue_s. Don't know if this causes more trouble somewhere else...

zesstra

2008-06-30 06:09

administrator   ~0000630

This may be a candidate for 3.5.x.
It would be necessary to check if the increase in size for the svalue is a blocker. BTW: That gets (in my opinion) less important with increased use of LP64 (64 bit) platforms as p_int is then anyway as big as a double.

Gnomi

2008-06-30 06:42

manager   ~0000631

For LP64 I would suggest using double directly, but for 32 bit systems I would rather not increase svalue_t and stick to the old format or use float, because in LPC we use floats seldom, but svalues are everywhere, so that would be an enormous waste of space.

zesstra

2008-06-30 07:00

administrator   ~0000632

FTR: That sounds reasonable to me. ;-)

zesstra

2008-06-30 07:20

administrator   ~0000633

Ok, again FTR: ;-)
We have to clean up some code before we can work on this, e.g. stuff like this one from interpret.c:

    CASE(F_FLOAT); /* --- float <mant> <exp> --- */
    {
        /* Push the float build from <mant> (4 bytes) and <exp> (2 bytes)
         * onto the stack. The binary format is the one determined
         * by STORE_DOUBLE in datatypes.h
         * TODO: This code makes heavy assumptions about data sizes and
         * TODO:: layout. E.g. there need not be a 16-Bit integral type
         * TODO:: available.
         * TODO: It should be rewritten to use the LOAD_ macros (but
         * TODO:: then the compiler needs to use them, too.
         */

Additionally, it may be an idea to use small inlineable (static) functions instead of defines. Would have to be tested for any runtime impact, but I would expect no difference with modern compilers.

fufu

2009-04-17 15:59

manager   ~0001054

I see one problem with Gnomi's idea of making this change for LP64 only: It would change the rounding behaviour of LPC code between 32bit and 64bit versions of the driver, so the same code can have different results.

We'll have to decide whether that's acceptable, or perhaps we should provide a configue option for using the old 48 bit format on LP64 anyway.

zesstra

2011-11-17 10:36

administrator   ~0002078

Setting target version to 3.5.0.
Actually, since this is already another 5 years old and I expect a wide adoption of LP64 systems also fuer MUDs in the next 1-3 years, I personally don't care about the size increase. But Gnomi disagrees in this respect and wants to keep the old format for ILP32 systems.
So that would then be a third float format.

I think, I am in favor of accepting the different behaviour on the 2 architectures.

Independant of the float format used at runtime, I suggest to simplify the savefile format (version 3) and just use sprintf+sscanf for writing/parsing doubles in any case.

zesstra

2011-11-21 23:11

administrator   ~0002082

The new float format using native doubles to store LPC floats it now in the driver in the 3.5.x branch.
https://github.com/ldmud/ldmud/commit/95ad6fe1f9500d55e73db0f21d05c827a70a262d

The new format will be automatically activated on LP64 platforms, where the int already consumes 8 bytes in the svalues. It can be manually activated on other platforms as well, although the is currently no configure option for it.

Issue History

Date Modified Username Field Change
2006-12-27 07:48 hkremss New Issue
2007-01-05 07:33 zesstra Note Added: 0000525
2007-01-06 10:36 hkremss Note Added: 0000526
2008-06-30 06:09 zesstra Note Added: 0000630
2008-06-30 06:09 zesstra Status new => acknowledged
2008-06-30 06:09 zesstra Projection none => major rework
2008-06-30 06:09 zesstra ETA none => > 1 month
2008-06-30 06:42 Gnomi Note Added: 0000631
2008-06-30 07:00 zesstra Note Added: 0000632
2008-06-30 07:20 zesstra Note Added: 0000633
2009-01-08 04:11 zesstra Project LDMud 3.3 => LDMud 3.5
2009-04-17 15:59 fufu Note Added: 0001054
2011-11-17 10:36 zesstra Note Added: 0002078
2011-11-17 10:36 zesstra Status acknowledged => confirmed
2011-11-17 10:36 zesstra Product Version 3.3 =>
2011-11-17 10:36 zesstra Target Version => 3.5.0
2011-11-17 16:38 zesstra Assigned To => zesstra
2011-11-17 16:38 zesstra Status confirmed => assigned
2011-11-17 16:44 zesstra Relationship added related to 0000793
2011-11-21 23:11 zesstra Note Added: 0002082
2011-11-21 23:11 zesstra Status assigned => resolved
2011-11-21 23:11 zesstra Fixed in Version => 3.5.0
2011-11-21 23:11 zesstra Resolution open => fixed