bebbo / gcc

Bebbo's gcc-6-branch for m68k-amigaos
GNU General Public License v2.0
33 stars 11 forks source link

Assuming retfp0 with 68881 breaks calls to __ floatunXXX #188

Closed erique closed 1 year ago

erique commented 1 year ago

Commit 2b293d1 ("always use fp0 for float/double if 68881 or better is present") changes the ABI from "softfp" (soft-float calling convention, with hardware floating-point instructions) to strict "hard" (FPU-specific calling conventions), as soon as 68881+ is enabled. Effectively this means that -mretfp0 is always enabled whenever -m68881 (or -m68040/060) is.

When casting from a 64bit unsigned integer to float/double, the implicit call to ___floatundidf assumes soft/softfp calling convention (i.e. result passed in d0/d1), but libgcc built with the above change will return the value in fp0.

Test program : https://gist.github.com/erique/f5aa608f6e4a6d987a4e1d423f9b890f (CEX: https://franke.ms/cex/z/h1r4os ) Generated call (regardless of applying this change):

        jsr ___floatundidf
        addq.l #8,sp
        move.l d1,-(sp)
        move.l d0,-(sp)
        fdmove.d (sp)+,fp0

___floatundidf before the change (or rather, by reverting it):

00008ba0 00008ba0 ___floatundidf:
    8ba0:       2f03            move.l d3,-(sp)
    8ba2:       202f 0008       move.l 8(sp),d0
    8ba6:       222f 000c       move.l 12(sp),d1
    8baa:       2600            move.l d0,d3
    8bac:       f203 4044       fdmove.l d3,fp0
    8bb0:       4a83            tst.l d3
    8bb2:       6c08            bge.s 8bbc 8bbc ___floatundidf+0x1c
    8bb4:       f23c 4466 4f80  fdadd.s #0e4.29497e+09,fp0
    8bba:       0000 
    8bbc:       f23c 4467 4f80  fdmul.s #0e4.29497e+09,fp0
    8bc2:       0000 
    8bc4:       f201 40c4       fdmove.l d1,fp1
    8bc8:       4a81            tst.l d1
    8bca:       6c08            bge.s 8bd4 8bd4 ___floatundidf+0x34
    8bcc:       f23c 44e6 4f80  fdadd.s #0e4.29497e+09,fp1
    8bd2:       0000 
    8bd4:       f200 00e6       fdaddx fp0,fp1
    8bd8:       f227 7480       fmoved fp1,-(sp)
    8bdc:       201f            move.l (sp)+,d0
    8bde:       221f            move.l (sp)+,d1
    8be0:       261f            move.l (sp)+,d3
    8be2:       4e75            rts

___floatundidf after the change:

00008ba0 00008ba0 ___floatundidf:
    8ba0:       2f03            move.l d3,-(sp)
    8ba2:       202f 0008       move.l 8(sp),d0
    8ba6:       222f 000c       move.l 12(sp),d1
    8baa:       2600            move.l d0,d3
    8bac:       f203 4044       fdmove.l d3,fp0
    8bb0:       4a83            tst.l d3
    8bb2:       6c08            bge.s 8bbc 8bbc ___floatundidf+0x1c
    8bb4:       f23c 4466 4f80  fdadd.s #0e4.29497e+09,fp0
    8bba:       0000 
    8bbc:       f23c 4467 4f80  fdmul.s #0e4.29497e+09,fp0
    8bc2:       0000 
    8bc4:       f201 40c4       fdmove.l d1,fp1
    8bc8:       4a81            tst.l d1
    8bca:       6c08            bge.s 8bd4 8bd4 ___floatundidf+0x34
    8bcc:       f23c 44e6 4f80  fdadd.s #0e4.29497e+09,fp1
    8bd2:       0000 
    8bd4:       f200 0466       fdaddx fp1,fp0
    8bd8:       261f            move.l (sp)+,d3
    8bda:       4e75            rts

Suggest either reverting 2b293d1, or make libfuncs generation take the ABI change (-mretfp0/-m68881) into account.

bebbo commented 1 year ago

guess: the wrong libs are linked

erique commented 1 year ago

afaict we link with the correct/expected libgcc.a

$ m68k-amigaos-gcc -O0 -m68060 -mhard-float casttest.c -o casttest -Wl,--verbose | grep libgcc
attempt to open /opt/amiga/lib/libm060/libm020/libm881/libgcc.a failed
attempt to open /opt/amiga/lib/libm060/libm020/libgcc.a failed
attempt to open /opt/amiga/lib/libm060/libgcc.a failed
attempt to open /opt/amiga/lib/libgcc.a failed
attempt to open /opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libm060/libm020/libm881/libgcc.a failed
attempt to open /opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libm060/libm020/libgcc.a failed
attempt to open /opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libm060/libgcc.a failed
attempt to open /opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a succeeded
/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a
(/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a)_floatundidf.o
(/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a)_udivdi3.o
(/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a)_umoddi3.o
(/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a)_clz.o
/opt/amiga/lib/gcc/m68k-amigaos/6.5.0b/libm060/libgcc.a

You could argue that it would (probably) work if linked against the regular (non libm060) version, but that would imho also be somewhat incorrect (that lib would have no FPU instructions at all).

Imho we link with the correct lib; it's just the generated call site is not matching libm060/libgcc.a generated code.

erique commented 1 year ago

Perhaps stating the obvious / just to be clear: this code

        jsr ___floatundidf
        addq.l #8,sp
        move.l d1,-(sp)
        move.l d0,-(sp)
        fdmove.d (sp)+,fp0

is not from any lib; afaict it's generated "on the fly" by optabs-libfuncs.c (see https://franke.ms/cex/z/h1r4os )

bebbo commented 1 year ago

thanks - now I see: http://franke.ms/cex/z/r1jG4c

bebbo commented 1 year ago

should be live in 1 hour

erique commented 1 year ago

Still - assuming -mretfp0 whenever -m68060 is enabled causes a cascade of issues.

Here is one example : https://gist.github.com/erique/d6ff19dbfc7c810fd89e6070186768bb The implementation of ceil() in libnix calls mathieeedoubbas/IEEEDPCeil(), which returns the result in d0/d1. But libnix assumes the result is in fp0.

bebbo commented 1 year ago

ups - need to test the proprer archive...

erique commented 1 year ago

It's not enough to rebuild gcc, you need to rebuild all libs too.

I simply did

rm -rf projects/gcc
make all

and assumed libgcc et al would rebuild ; maybe that was a bold assumption :D It did look like it rebuilt most things tho.. 🤔

bebbo commented 1 year ago

uh - dont' kill the projects, takes even more time, since you redownload all the the stuff.

since ceil.o is in libnix and gcc was changed you only need:

make clean-gcc clean-libnix
make libnix -j30

building libnix implies building gcc. And 30 is maybe too large - use a number that fits your cpu

bebbo commented 1 year ago

my build just finished:

bebbo commented 1 year ago
stefan@ZETRA:~/amiga-gcc/tickets/gcc188$ /opt/amiga20220916/bin/m68k-amigaos-gcc -noixemul -m68060 -lm ceil_error.c -o ceil_error
stefan@ZETRA:~/amiga-gcc/tickets/gcc188$ vamos -C40 ceil_error
GCC 6.5.0b 220915211933
clamped = 00000004 ; PASS
erique commented 1 year ago

yep, for some reason libnix wasn't rebuilt; possibly missed dependency between gcc and libnix. anyway - all good now.

erique commented 1 year ago

fixed by e377bf7