English Amiga Board

English Amiga Board (http://eab.abime.net/index.php)
-   Coders. System (http://eab.abime.net/forumdisplay.php?f=113)
-   -   V34 Bugs in IEEEDPFieee and RawDoFmt (http://eab.abime.net/showthread.php?t=97563)

phx 31 May 2019 18:03

V34 Bugs in IEEEDPFieee and RawDoFmt
 
There is no note about it in the BUGS section of the V40 mathieeedoubtrans.doc and I don't remember that I heard about it before, but can anybody confirm that IEEEDPFieee from Kickstart 1.x is bugged?

I did the following short test program to convert an IEEE single precision number into double precision:
Code:

        move.l  4.w,a6
        lea    mathdbtransname(pc),a1
        moveq  #0,d0
        jsr    -552(a6)                ; OpenLibrary
        tst.l  d0
        beq    exit

        ; convert single precision 0.267261 to double precision
        move.l  d0,a6
        move.l  #$3e88d677,d0
        jsr    -108(a6)                ; IEEEDPFieee
        movem.l d0-d1,-(sp)            ; save double precision conversion

        move.l  a6,a1
        move.l  4.w,a6
        jsr    -414(a6)                ; CloseLibrary mieeedoubtrans

        lea    dosname(pc),a1
        moveq  #0,d0
        jsr    -552(a6)
        move.l  d0,-(sp)                ; save DOSBase
        move.l  d0,a6
        jsr    -60(a6)                ; Output
        move.l  d0,-(sp)                ; save stdout

        ; print double precision result in hex
        move.l  4.w,a6
        lea    fmtstr(pc),a0
        lea    8(sp),a1                ; double precision value on stack
        lea    write(pc),a2
        move.l  sp,a3
        jsr    -522(a6)                ; RawDoFmt and Write to stdout

        movem.l (sp)+,d0/a1
        jsr    -414(a6)                ; CloseLibrary dos
        addq.l  #8,sp
exit:
        moveq  #0,d0
        rts

write:
        movem.l d0/a6,-(sp)
        movem.l (a3),d1/a6
        move.b  d0,(sp)
        move.l  sp,d2
        moveq  #1,d3
        jsr    -48(a6)                ; Write next character to stdout
        movem.l (sp)+,d0/a6
        rts

mathdbtransname:
        dc.b    "mathieeedoubtrans.library",0
dosname:
        dc.b    "dos.library",0
fmtstr:
        dc.b    "%lx%lx",10,0
        even

The result printed on Workbench 3.1 is
3FD11ACEE0000000

which is correct (0.267261). Workbench 1.3, on the other hand, prints:
3FD1DDFF D0000000

This is 0.279175 and a considerable difference. Is that a known bug? Perhaps Kickstart 1.x mathieeedoubtrans was not much tested or used with IEEE single precision, as it also had no mathieeesingbas/trans libraries? Or does it even expect FFP instead of IEEESP?

Bonus bug:
I was suprised that 1.3 first prints 3FD1DDFF with a blank, then waits for 30 seconds(!), prints D0000000 and waits for another 30 seconds! I didn't remember that RawDoFmt() was also that bugged... :confused

Bruce Abbott 01 June 2019 06:00

Quote:

Originally Posted by phx (Post 1324705)
Bonus bug:
I was suprised that 1.3 first prints 3FD1DDFF with a blank, then waits for 30 seconds(!), prints D0000000 and waits for another 30 seconds! I didn't remember that RawDoFmt() was also that bugged... :confused

There's a bug, but it's in your code. In the 'write' callback you should save and restore D2 and D3. Also copying the byte in D0 directly to the top of stack isn't a good idea because it splats over the current contents. In your code this is the (long) saved contents of D0, which luckily doesn't matter because D0 doesn't need to be preserved.

I changed your code to this:-

Code:

write:
        movem.l d2-d3/a6,-(sp)
        movem.l (a3),d1/a6
        move.b  d0,-(sp)
        move.l  sp,d2
        moveq  #1,d3
        jsr    -48(a6)                ; Write next character to stdout
        move.b  (sp)+,d0
        movem.l (sp)+,d2-d3/a6
        rts

and it printed properly in WB1.3 on my A500 (previously it would print a 'random' character several thousand times before doing the second number).

Still got the wrong hex result though... must investigate further!

phx 01 June 2019 13:44

Quote:

Originally Posted by Bruce Abbott (Post 1324791)
There's a bug, but it's in your code. In the 'write' callback you should save and restore D2 and D3.

Indeed! That's my fault. Thanks. So RawDoFmt() is fine. :)

Quote:

Also copying the byte in D0 directly to the top of stack isn't a good idea because it splats over the current contents. In your code this is the (long) saved contents of D0
That was intended. I was pushing D0 onto the stack not to save it, but to make room for the character on the stack. Writing it with "move.b d0,-(sp)" is also a nice solution, though.

Quote:

Still got the wrong hex result though... must investigate further!
Yes. It has definitely nothing to do with RawDoFmt. I spotted the problem in a different context and wrote this code for better reproduction.

Bruce Abbott 01 June 2019 17:55

Quote:

Originally Posted by phx (Post 1324819)
That was intended. I was pushing D0 onto the stack not to save it, but to make room for the character on the stack. Writing it with "move.b d0,-(sp)" is also a nice solution, though.

So long as you know what you are doing it's OK, but not good programming style IMO. It looks like you were intending to save and restore D0, when in fact you were just pushing it onto the stack to get some space (and the popped value is corrupted). In my experience it's usually better to write conventional code that is easy to maintain, rather than doing tricky stuff in an attempt to save a few bytes or cycles.

Anyway it seems you were right, there is a bug in the WB1.3 mathieeedoubtrans.library!

Here's a partial disassembly of V34.8 (from WB1.3.3)

Code:

r003a1c:
 clr.l  D1
 rts

r003a74:              ; IEEEDPFieee
 movea.l D0,A0
 swap    D0
 beq.s  r003a1c
 move.w  D0,D1
 andi.w  #$7f80,D0
 asr.w  #3,D0
 addi.w  #$3800,D0
 andi.w  #$8000,D1
 or.w    D1,D0
 swap    D0
 move.l  A0,D1
 ror.l  #3,D1
 movea.l D1,A0
 andi.l  #$000fffff,D1
 or.l    D1,D0
 move.l  A0,D1
 andi.l  #$e0000000,D1
 rts

And here's the V37.1 version (supplied with WB2.1 and 3.0):-

Code:

r003d78:
 clr.l  D1
 rts

r003dd0:              ; IEEEDPFieee
 movea.l D0,A0
 swap    D0
 beq.s  r003d78
 move.w  D0,D1
 andi.w  #$7f80,D0
 asr.w  #3,D0
 addi.w  #$3800,D0
 andi.w  #$8000,D1
 or.w    D1,D0
 swap    D0
 move.l  A0,D1
 ror.l  #3,D1
 movea.l D1,A0
 andi.l  #$000fffff,D1
 clr.w  D0        <--- extra instruction
 or.l    D1,D0
 move.l  A0,D1
 andi.l  #$e0000000,D1
 rts

The extra 'clr.w D0' instruction is the only difference in the IEEEDPFieee function, and with it the correct result is returned.

phx 01 June 2019 20:57

Thanks for the confirmation. That's a serious problem, as single- to double precision conversions happen frequently when using floating point. Using printf with floats always converts to double precision, for example.

I wonder how Kickstart 1.x C compilers handled this problem. Perhaps they had their own conversion code in libm instead of using AmigaOS shared libraries.

Bruce Abbott 02 June 2019 07:39

Quote:

Originally Posted by phx (Post 1324879)
I wonder how Kickstart 1.x C compilers handled this problem. Perhaps they had their own conversion code in libm instead of using AmigaOS shared libraries.

SASC appears to have internal code for converting float to double.

You could work around the bug by using the code I posted from V37.1 instead of calling the library function. If for some reason you don't want to do that then does V37.1 work in WB1.x? Or perhaps we should patch V34.8!

BastyCDGS 02 June 2019 16:34

Quote:

Originally Posted by Bruce Abbott (Post 1324918)
SASC appears to have internal code for converting float to double.

You could work around the bug by using the code I posted from V37.1 instead of calling the library function. If for some reason you don't want to do that then does V37.1 work in WB1.x? Or perhaps we should patch V34.8!

I really don't remember any program in the OS1.x era which used the math libraries.

The math libraries never had been copied to any custom made WB/Utility/whatever disc by me and nothing complained missing it. Even stuff for OS2.x+ seems using it rarely.

mark_k 02 June 2019 21:08

This newsgroup posting from 1989 may be someone reporting the same problem.

Thomas Richter 02 June 2019 21:24

Quote:

Originally Posted by phx (Post 1324879)
I wonder how Kickstart 1.x C compilers handled this problem. Perhaps they had their own conversion code in libm instead of using AmigaOS shared libraries.

The math models work quite different. IEEE single precision did not gain much attention. In fact, the SAS/C does not even support it (and the way how we compiled mathieeesingtrans for 3.1.4 is just another story).

The typical math models are either: 1) FLOAT=DOUBLE=mathffp. This is the mathffp.library and mathtrans.library. This is *not* recommended for new code as FFP has a couple of serious problems (no denormalized numbers, no NANs, no INFs).

2) FLOAT=DOUBLE=mathieeedoubbas. In this math model, singbas/trans are neither used.

3) FLOAT=DOUBLE=internal. This uses the internal math code of SAS/C.

4) native 68881/882 math.

The Manx compiler handled it similarly. It could either use mathffp math, or mathieeedoubbas, or 68881 math. single precision IEEE does not play any role.

phx 03 June 2019 12:01

Quote:

Originally Posted by Bruce Abbott (Post 1324918)
You could work around the bug by using the code I posted from V37.1 instead of calling the library function.

Yes. That's the best solution for me. Also the conversion routine is not as complicated as I feared, so I can easily put it into a linker library.
I hope there are not many more issues in V34 math libraries...

Quote:

Originally Posted by Thomas Richter (Post 1325019)
The typical math models are either: 1) FLOAT=DOUBLE=mathffp.

Didn't ANSI-C define that the IEEE Std 754 has to be used for floating point data representation?

Quote:

2) FLOAT=DOUBLE=mathieeedoubbas. In this math model, singbas/trans are neither used.
I would guess that storing float variables as 8 bytes double precision causes a lot of problems here.

Quote:

The Manx compiler handled it similarly. It could either use mathffp math, or mathieeedoubbas, or 68881 math. single precision IEEE does not play any role.
Right. I remember the various options. I used Aztec-C V3.4 in the 80s.

My current math model for vbcc's m68k-kick13 target is to use shared libraries for everything (as I did for the OS2+ target), but due to the lack of IEEE single precision support all "float" artihmetics are done by mathffp/mathtrans (converting IEEEsingle to FFP back and forth). Maybe I should better convert everything to IEEEdouble - but that might be slower.

Thomas Richter 03 June 2019 13:06

Quote:

Originally Posted by phx (Post 1325116)
Didn't ANSI-C define that the IEEE Std 754 has to be used for floating point data representation?

Nope. ANSI-C doesn't have much to say about the nature of the floating point. There might be a couple of minimum requirements concerning the precision, but that is about it.
Quote:

Originally Posted by phx (Post 1325116)
I would guess that storing float variables as 8 bytes double precision causes a lot of problems here.

Who says that sizeof(float)=4 and sizeof(double)=8? ANSI-C only requires that double is at least as precise as float, hence, sizeof(float) == sizeof(double) is perfectly fine. In this particular case, on the Amiga, both are 8 bytes long. But both = 4 bytes long also works, as far as ANSI C is concerned.
Quote:

Originally Posted by phx (Post 1325116)
My current math model for vbcc's m68k-kick13 target is to use shared libraries for everything (as I did for the OS2+ target), but due to the lack of IEEE single precision support all &quot;float&quot; artihmetics are done by mathffp/mathtrans (converting IEEEsingle to FFP back and forth). Maybe I should better convert everything to IEEEdouble - but that might be slower.

Stay away from mathffp and mathtrans. They implement a quite lousy math model. mathieeesingbas isn't much slower, but sane. Ok, except for the strange choice of the rounding model, but that will be fixed.


All times are GMT +2. The time now is 22:08.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2020, vBulletin Solutions Inc.

Page generated in 0.04408 seconds with 11 queries