Feedback on floating point Math library requested

[Michael Steil]

I just merged a change that exports a jump table for a C128-compatible floating point Math library:

• 
  change: https://github.com/commanderx16/x16-rom/issues/231
  


• 
  documentation: https://github.com/commanderx16/x16-docs/blob/master/Commander X16 Programmer's Reference Guide.md#math-library
  

There is something I would like some feedback on:

As I understand it, the calls FADDT, FMULTT, FDIVT and FPWRT of the original C128 API didn't work as expected and were useless. If you look at these subroutines in x16-rom/math/*.s, you'll see that they require the flags set up in a certain way, which a client can't reproduce without breaking the abstraction and reading some internal state of the library.

I appended replacement calls FADDT2, FMULTT2, FDIVT2 and FPWRT2, which do the necessary setup, to the end of the jump table. Now I'm wondering whether I should replace the original calls with these instead. Given that they were probably never useful (or used!), it's safe (and clean) to replace them.

What do you think? Anything I'm missing?

Edit: Here's an article that explains the problem: https://www.c64-wiki.de/wiki/FADDT; English: https://www-c64--wiki-de.translate.goog/wiki/FADDT?_x_tr_sl=auto&_x_tr_tl=en&_x_tr_hl=en-US&_x_tr_pto=wapp

See also: https://github.com/commanderx16/x16-rom/issues/244

[Scott Robison]

On 3/26/2022 at 10:36 AM, Michael Steil said:

I just merged a change that exports a jump table for a C128-compatible floating point Math library:

• 
  change: https://github.com/commanderx16/x16-rom/issues/231
  


• 
  documentation: https://github.com/commanderx16/x16-docs/blob/master/Commander X16 Programmer's Reference Guide.md#math-library
  

There is something I would like some feedback on:



As I understand it, the calls FADDT, FMULTT, FDIVT and FPWRT of the original C128 API didn't work as expected and were useless. If you look at these subroutines in x16-rom/math/*.s, you'll see that they require the flags set up in a certain way, which a client can't reproduce without breaking the abstraction and reading some internal state of the library.



I appended replacement calls FADDT2, FMULTT2, FDIVT2 and FPWRT2, which do the necessary setup, to the end of the jump table. Now I'm wondering whether I should replace the original calls with these instead. Given that they were probably never useful (or used!), it's safe (and clean) to replace them.



What do you think? Anything I'm missing?



Edit: Here's an article that explains the problem: https://www.c64-wiki.de/wiki/FADDT; English: https://www-c64--wiki-de.translate.goog/wiki/FADDT?_x_tr_sl=auto&_x_tr_tl=en&_x_tr_hl=en-US&_x_tr_pto=wapp

I agree that if the C128 version was defective, there is no reason to maintain compatibility. Just do it the "right way" for CX16.

[Michael Steil]

On 3/26/2022 at 5:46 PM, Scott Robison said:

I agree that if the C128 version was defective, [...]

The C128 is *is* defective, right? Am I missing something here?

[BruceMcF]

"I agree that if" is the same way I might put it, if I had been told it was defective but had never experienced it for myself.

(As I haven't, since I fried my C128D back in the late 80s within a month of purchase by putting a power tap for my printer interface into the datasette port upside down, shorting out the 8510.)

But, yes, replace them. As an assembly language programmer, anything that uses them correctly would be so tied into the internals of the C128 that it'd have to be well documented to make porting it manageable, and if it's documenting that it's doing something to get around the bugs in those functions, it'd be welcome to have "get rid of this stuff, the CX16 versions don't skip the set-up" be the road to porting that part.

And as a Forth programmer, it'd be in a wrapper anyway to expose an executable word in the dictionary and interface to the floating point stack, so stripping the set-up code out of the wrapper is a "fix once, use from then on" process.

[Scott Robison]

On 3/27/2022 at 3:13 AM, Michael Steil said:

The C128 is *is* defective, right? Am I missing something here?

@BruceMcFexplained what I meant. I never used the floating point library from asm on the 128, so I never experienced its shortcomings.

Perhaps a better phrasing would be "Commander X16 has the opportunity to fix anything that was done improperly or suboptimally on previous machines that it is inspired by". If it were attempting to be 100% compatible with a particular machine, then there would be a reason to keep those defects, but it is its own thing. It wants to be reminiscent of those older machines, but that shouldn't extend to keeping around a buggy API just because some machine of yesteryear did.

[Michael Steil]

Okay, I merged the fix: https://github.com/commanderx16/x16-rom/commit/ae608673f0210953172d6837acfbb231d62ddbd1