Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save 2 bytes each in push and pop stack #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshgoebel
Copy link

ADD supports a direct operand, so why not use it?

Also, what is the need for the 3 byte padding between stack frames?

`ADD` supports a direct operand, so why not use it?
@joshgoebel
Copy link
Author

joshgoebel commented Sep 14, 2018

Oh, I get it now. You're leveraging the font functionality to help set the stack pointer. Any reason to not just do the math yourself (add/sub 15 to VE)?

You're also heavily dependent on your own implementation of static void _FX29() { uint16_t x = (op & 0x0F00) >> 8; I = 5 * v[x]; }. Since only 0-F are valid characters you could easily imagine a different implementation preventing the overflow with a bit mask: I = 5 * (v[x] & 0x0F). (and I think I've seen that in other implementations)

I suppose this is a few bytes smaller?

@glouw
Copy link
Owner

glouw commented Sep 14, 2018

The funny thing about compilers is that one never remembers how they work (for the life of me I cannot even remember what any of these opcodes do!). I really do appreciate the pull request though. I'm going to review it in the coming weeks.

If memory serves me right, and I think you're right, pushing and popping abuses the font pointer to save V0-VE to ROM with the LD [I] instruction. I did do a writeup on it here: http://glouw.com/2018/03/05/C8C.html

Sadly it will either thrash the baked fonts, or the virtual machine itself if you were to run this on a COSMAC emulator like Emma 02 .

(As an aside, I wouldn't recommend writing a compiler by hand. This was really just an educational thing for myself. Tools like GNUBison are the way to go. I cannot recommend Bisqwit's tutorials on compilers enough: https://youtu.be/eF9qWbuQLuw).

@joshgoebel
Copy link
Author

If memory serves me right, and I think you're right, pushing and popping abuses the font pointer to save V0-VE to ROM with the LD [I] instruction. I did do a writeup on it here: http://glouw.com/2018/03/05/C8C.html

Yes, the writeup was helpful.

I was just suggesting instead:

LD I, VE
LD [I], VE
ADD VE, 15

Feels purer to me - and more "correct" since we don't have an actual hardware spec for CHIP8. If it could be universally agreed that the "font load" DID/SHOULD overflow then the way you've done it is a pretty neat hack though (and the way it should be done, since it's 1-2 byte smaller).

Anyways the patch I submitted should work in any case.

I wouldn't recommend writing a compiler by hand.

It's fun though. I'm working on a Ruby-like (but not as dynamic) language that compiles down to CHIP8 now. I didn't realize someone had already done this with a C-like language though.

@glouw
Copy link
Owner

glouw commented Sep 16, 2018

It certainly was a fun ride building this stuff. I used this for reference:

https://compilers.iecc.com/crenshaw/

It's in Pascal, but pascal translates well to anything.

@glouw
Copy link
Owner

glouw commented Sep 16, 2018

Sadly, as for the 2 byte fix, I had a look over the opcode list: http://devernay.free.fr/hacks/chip8/C8TECH10.HTM

SUB is limited to [(8xy5) SUB Vx, Vy], but ADD supports direct values. Strange that they did that.

@joshgoebel
Copy link
Author

joshgoebel commented Sep 17, 2018

Oh. Sorry. But it's all 8-bit math with wrap-around. SUB 15 just becomes ADD 240. Not that strange considering how easy it is. A nice compiler should really just support "sub Vx, #" as a virtual OP imho and then do the math for you.

SUB doesn't allow direct operands, but ADD does.
@joshgoebel
Copy link
Author

Updated the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants