JohnEarnest / Octo

A Chip8 IDE
MIT License
682 stars 55 forks source link

skip instruction does not skip long form of index register assignment #67

Closed whoozle closed 7 years ago

whoozle commented 7 years ago

Consider the following example:

: main
   vc := 10
   vd := 10
   v0 := 0

   :breakpoint before
   if v0 == 0 then i := long data
   if v0 == 1 then i := long data
   :breakpoint after #CORRUPTED V0 HERE

   loop again

:org 0x7000
 0 1 2 3 4 5 6 7 8 
: data
 0 1 2 3 4 5 6 7 8 
 0 1 2 3 4 5 6 7 8 
jdeeny commented 7 years ago

I think this is basically working 'as designed' since the 4XNN, 5XNN opcodes just increment the PC by 2 and i := long NNNN is 4-bytes.

JohnEarnest commented 7 years ago

Correct. It didn't seem wise to modify the semantics of existing opcodes to accommodate new (XO-Chip specific) instructions, even if it might be more convenient in some cases.

whoozle commented 7 years ago

those instructions are called SKIP, and semantically it's a conditional predicate. Since long form of assignment was added in XO chip, I'd expect it to skip it properly.

whoozle commented 7 years ago

It didn't seem wise to modify the semantics of existing opcodes

Was adding 0 height mode for sprite instruction such modification too? :)

It does not seem rational to me, since the only old code it could possibly break — someone deliberately put always true in skip condition and skip f0 00 invalid opcode in some old code just for fun. I doubt it has been used anywhere.

I think this is basically working 'as designed' since the 4XNN, 5XNN opcodes just increment the PC by 2 and i := long NNNN is 4-bytes.

no instruction can possibly get PC value, so it does not matter if it skips 2 or 4 bytes, you can't get any side effect of it in chip8 execution environment

Lack of consistency here leads to hard-to-catch errors, if addr is also valid instruction. :(

Could you add error message in compiler then?

whoozle commented 7 years ago

added error message for this https://github.com/JohnEarnest/Octo/pull/69

JohnEarnest commented 7 years ago

After sleeping on this, I've changed my mind. I will modify the interpreter to make all six conditional skip instructions skip over a double-wide i := long NNNN. I can't think of any existing, valid programs it will break, and it makes the feature easier to use.

whoozle commented 7 years ago

Thanks for bringing consistency here!