#3658 [rk3a] Floating-point math is broken

[D-mo]

This simple program makes the board (RCM3010) hang:

#include <r3ka.h>

volatile float x, y, z;

void main()
{
    PBDDR  = 0xff;
    PBDR = 0x00;

    x = 42;
    y = 69;
    z = x + y;

    PBDR = 0xff;
}

NOTE: I am setting PB low and high just as an indicator (I have LED attached to one of the pins).

The line causing the problem is z = x + y, which calls ___fsadd internal function. Using other floating-point functions such as ___fsmul or ___fsdivhave the same effect.

😕😕😕

[D-mo]

Here is .lst file for the above.

[Philipp Klaus Krause]

That's odd, since
1) The Rabbit ports pass the regression tests (on the simulator)
2) I ran Whetstone on a Rabbit a while ago (AFAIR using SDCC 4.1.0 r3ka port for RCM3319)

[D-mo]

@spth This has something to do with using custom crt0. I suspect I screwed something up. Investigating now

[D-mo]

OK, so the problem is with calling conventions. Here is the final test case that fails:

__sfr __banked __at 0x0040 PBDR;
__sfr __banked __at 0x0047 PBDDR;

int fn(int baud) { return baud + 0.5; }

void main()
{
    PBDDR = 0xff;
    PBDR = 0x00;

    int x = fn(38400);

    PBDR = 0xff;
}

I compile it as follows:

sdcc -mr3ka --sdcccall 1 main.c
sdobjcopy -I ihex -O binary main.ihx main.bin

This makes the board hang. If I remove the --sdcccall 1 option, it works. Attached are .lst files for both cases (with and without the option).

[D-mo]

Not sure why my original example didn't work. I may have uploaded the wrong binary while trying to figure wtf was going on...

[D-mo]

If I just mark the function fn as __sdcccall(1) everything still works fine. Maybe with the --sdcccall 1 option the compiler assumes that the internal functions (__fsadd, etc.) are also __sdcccall(1)?

[Philipp Klaus Krause]

Yes, it does. For --sdcccall 1, you will need a standard library compiled with --sdcccall 1 (both for the internal functions and for the standard library functions).

[D-mo]

Hmm... So this switch is not very useful if you rely on any of the internal sdcc functions. Is there an option to have it on by default? Maybe just for the Rabbit targets?

During compilation of sdcc that is. In other words I'd like to compile the Rabbit library with sdcccall set to 1 and have it on by default unless disabled with 0.

[D-mo]

@spth I see there is a commit from 2021:

[r12692] __sdcccall(1) convention as default for z80, z80n and z180 picked from breaktheworld branch.

I guess this could also be expanded to the Rabbit targets?

[Philipp Klaus Krause]

Yes. Back then I did some research into what would be an efficient calling convention for the z80-related ports and for stm8: https://arxiv.org/abs/2112.01397
The default was never changed for the Rabbits, though.
1) The community had mixed reactions to the change for z80 (it improved the generated code, but broke everyone's handwritten asm). Many still use --stdcccall 0.
2) There was only limited interest in the Rabbit ports (few users compared to other ports).
3) With relatively little interest from users in the Rabbit ports, it seemed unclear if it would be worth the effort to have a calling convention different from z80. After all, an alternative could be to use the new z80 calling convention for the Rabbits too. It would not be as good as the new Rabbit one, but still better than the old one.
In the end, I shifted my focus to other projects (improving standard compliance, and generalized constant propagation).

However, it would still possible to pick up the work, and continue. After all, by far most of the effort has been done already. If there is sufficient user interest, I might look into it; if the switch to the new convention is done next months, that would be in time for the 4.4.0 release (a big breaking change like this IMO needs a few months of testing in before a release).

[D-mo]

1) The community had mixed reactions to the change for z80 (it improved the generated code, but broke everyone's handwritten asm). Many still use --stdcccall 0.

They should just stay on the older version or label their functions with __sdcccall(0). :)

2) There was only limited interest in the Rabbit ports (few users compared to other ports).
3) With relatively little interest from users in the Rabbit ports, it seemed unclear if it would be worth the effort to have a calling convention different from z80. ...

Well, I am just one user, but I do represent about 500 RCM3010 boards... Not sure if that counts. ;)

In the end, I shifted my focus to other projects (improving standard compliance, and generalized constant propagation).

That's understandable and is appreciated. If it wasn't for this project, I'd be sitting in Windows VM right now and using Dynamic C.

However, it would still possible to pick up the work, and continue. ...

I wouldn't mind picking it up, but unfortunately, I'am already a week behind my deadline. And today I've run into another bug after upgrading to 4.3.0. (Opening separate bug report.)

[D-mo]

Hmm... So this switch is not very useful if you rely on any of the internal sdcc functions.

One way to mitigate this issue is to have a macro that resolves to either __sdcccall(0) or __sdcccall(1) (or something else) depending on which target the code is being compiled for, and then explicitly mark all internal functions with it. I believe several other things are already handled that way in the library. Eg:

#if defined(__SDCC_z80) || defined(__SDCC_z180) ...
#  define __SDCCCALL __sdcccall(1)
#elif defined(__SDCC_r2k) || defined(__SDCC_r3ka) ...
#  define __SDCCCALL __sdcccall(0)
#elif defined(__SDCC_mcs51) ...
#  define __SDCCCALL ...
#endif

...

float __fsadd (float, float) __SDCCCALL;

[Maarten Brock]

Is this something we could handle with a pushable pragma, to set the default calling convention? I'm not sure if it would be better than modifying all prototypes with this macro, though. The library header files could all look something like this:

#pragma save
#pragma sdcccall z80,z180 1
#pragma sdcccall r2k,r3ka 0

<prototypes>
...
#pragma restore

[Philipp Klaus Krause]

It seems to me, that most users either want to use the default for everything (and maybe want the default to be --sdcccall 1) or are using their own custom stdlib (and just use --sdcccall 0).

[Maarten Brock]

If users want to use a different calling convention, how easy is it for them to get a corresponding stdlib? I don't think this is easy on e.g. Windows.

It is simple to start sdcc with --sdcccall 1, but when the stdlib that comes with sdcc is compiled with --sdcccall 0 the generated binary will fail without build warning.

Maybe --sdcccall should imply --nostdlib ?
Maybe sdcc should install libs compiled with both calling conventions and automatically pick the correct one ?
Maybe the linker should complain when linking an incompatible calling convention ?
Or maybe it is a good idea to let the stdlib headers enforce the calling convention.

[Philipp Klaus Krause]

Having standard library functions use a different convention from the --sdcccall argument is not a standard compliant C implementation: Unqualified function pointers to standard libary functions wouldn't work.
I guess, if I look into this again, and make --sdcccall 1 the default for 4.4.0, the problem here is solved?

[Benedikt Freisen]

Would it make sense to put the calling convention in the library path?
That way people could use different compiled versions of the library for different projects and would get a linker error if no library for the selected calling convention is present, unless they also use --nostdlib.

[Philipp Klaus Krause]

Yes, but might not be worth the effort, as having just the new one works well enough for most users. Those using --sdcccall 0 mostly use their own standard library anyway, and the few exceptions tend to just grab the precompiled library from the last release where --sdcccall 0 was the default.
Having to maintain two versions does come with extra effort: any asm-implemented functions need to be maintained in two versions.

[D-mo]

It is simple to start sdcc with --sdcccall 1, but when the stdlib that comes with sdcc is compiled with --sdcccall 0 the generated binary will fail without build warning.

This is exactly what I ran into.

I guess, if I look into this again, and make --sdcccall 1 the default for 4.4.0, the problem here is solved?

In my biased opinion I would say yes. :)

[Maarten Brock]

I guess, if I look into this again, and make --sdcccall 1 the default for 4.4.0, the problem here is solved?

In my opinion: no. It just moves the problem around. Then the unsuspecting user giving --sdcccall 0 will be bitten.

Maybe we could modify the generated asm labels to signify the calling convention so the linker can complain. E.g.

void foo(void) __sdcccall(0) {}
void bar(void) __sdcccall(1) {}

_0_foo:
    ret

_1_bar:
    ret

[Philipp Klaus Krause]

Changing the asm labels seems a bit heavy-handed (and is a breaking change, as users will have to change names in all their hand-written asm functions).
Of course being able to communicate the calling convention used to the linker would be great, and also catch other bugs (we don't just have sdccccall, some ports also support compatibility calling conventions to call code using another Compiler's convention). But that is probably a somewhat bigger task.

For now maybe emit a warning when --sdcccall with non-default argument is used without --nostdlib or without --no-std-crt0?

[D-mo]

Would it make sense to put the calling convention in the library path?

Yes, but might not be worth the effort...

IMHO it would be worthwhile. Users (ie, devs who are using your product) expect things to work out of the box. I almost gave up on SDCC because I kept running into one bug after another. And it was only my repulsion towards Windows that made be persist. Other users would probably just move on.

So again, IMHO it would be worthwhile in the long run.

Or at the least, it should be very clearly documented in the manual that using this switch will have these consequences.

[Philipp Klaus Krause]

In [r14372] , I implemented a warning when a non-default sdcccall option is used with default stdlib or crt0. I consider that as a fix for this bug. Further improvements might make sense, but would IMO be feature requests.

[Philipp Klaus Krause]

Also, as of [r14373], --sdcccall 1 is now the default for all z80-related ports.