ISA change: New behaviour for the X flag

sy2002 / QNICE-FPGA

QNICE-FPGA is a 16-bit computer system for recreational programming built as a fully-fledged System-on-a-Chip in portable VHDL.

http://qnice-fpga.com

Other

70 stars 15 forks source link

ISA change: New behaviour for the X flag #165

Open sy2002 opened 4 years ago

sy2002 commented 4 years ago

First of all, the following decision has to be made:

Shall the X flag act as Bernd described it here: https://github.com/sy2002/QNICE-FPGA/issues/160#issuecomment-707624518
Or shall the X-flag act as Michael described it here: https://github.com/sy2002/QNICE-FPGA/issues/160#issuecomment-707896598

After that has been decided (@bernd-ulmann), the following TODOs need to be done.

[x] Change the intro document
[x] Change the programming card
[x] Change the emulator
[x] Change the CPU
[x] Change cpu_test.asm
[ ] Write a section about our recommended flag handling in our programming best practices
[ ] Ask Volker if this change leads to any change in VASM or the VBCC toolchain (did he use X?)
[ ] Do a smoke test on the VBCC apps (e.g. in qbin) in the emulator and on hardware

@MJoergen and @bernd-ulmann: Is this list complete?

MJoergen commented 4 years ago

That list seems complete.

MJoergen commented 4 years ago

As mentioned in https://github.com/sy2002/QNICE-FPGA/issues/160#issuecomment-707994872 I support @bernd-ulmann 's proposal for the X-flag.

bernd-ulmann commented 4 years ago

I have already changed the following things:

Emulator now changes the X bit only implicitly in the case of SHL/SHR.
Into and programming card have been updated.

MJoergen commented 4 years ago

I've updated the cpu_test.asm so it now passes with the new emulator.

I've done a smoke-test of our demo programs on the emulator.

MJoergen commented 4 years ago

Fixed and verified in hardware.

sy2002 commented 4 years ago

Great job, both of you gentlemen! And speedy ;-)

@bernd-ulmann Could you rename all ISA version references (is it just the headline in the intro.pdf or are there more) to V1.7? This ISA change leads to a new ISA version number...

MJoergen commented 4 years ago

I still need to update the MTH$SHR32 routine so that it actually does shift into X.

MJoergen commented 4 years ago

Actually, the routine already worked :-) I've updated the test program test_shift.asm to test this behaviour.

NOTE: In the current implementation, the routine SHR32 correctly sets the X flag following the same semantics as the SHR instruction. However, the value of the C flag after the call is undefined.

Likewise the routine SHL32 correct sets the C flag as expected, but the value of the X flag after the call is undefined.

This is done to simplify the implementation. This is a (slight) difference from the semantics of the SHL and SHR instructions. I added a short comment about this in math_library.asm.

I hope this is ok ?

sy2002 commented 4 years ago

@MJoergen For me it is absolutely OK.

Though it "feels" like a slight inconsistency. As I am currently in a rush I did not look at the code so a quick question to you: Save the X or the C flag before the routine and restoring it on exit for not having it undefined but as it was on entry: Would this break the elegance of the implementation?

MJoergen commented 4 years ago

I don't know about "elegance" :-D But it would require some more instructions, and I'm just asking whether it is worth it? I mean, would anyone use it?

The tricky part is to restore the X flag without destroying the newly generated C flag (or vice versa). So several instructions must be used to mask the particular status flag to be copied.

bernd-ulmann commented 4 years ago

Great job, both of you gentlemen! And speedy ;-)

@bernd-ulmann Could you rename all ISA version references (is it just the headline in the intro.pdf or are there more) to V1.7? This ISA change leads to a new ISA version number...

Done. :-)

sy2002 commented 4 years ago

When doing my initial grep to check the X register usage, I did only grep for the positive X, but not for the negative !X. So doing a grep -i ", \!X" *.asm (note the !X) revealed in test_programs these files:

qtransfer.asm
sdcard.asm
vga_int.asm

and in montior these files:

fat32_library.asm
qtransfer.asm

@MJoergen I will take care of my sources, i.e. all but vga_int.asm. There it looks like you are using X to check for -1? Please check (and maybe fix) this here:

https://github.com/sy2002/QNICE-FPGA/blob/develop/test_programs/vga_int.asm#L148

_ISR_1              MOVE    @R2, R3
                    MOVE    R6, @R2
                    MOVE    R3, R6
                    SUB     1, @R0
                    RBRA    _ISR_1, !X

MJoergen commented 4 years ago

Fixed and verified on hardware