search

PleasingFungus / Silicon-Zeroes

Issue repository for Silicon Zeroes. (Contains no actual code.)
12 stars 0 forks source link

Level "Delay Branch" allows this incorrect solution (tests don't catch it) #132

Open mikeweilgart opened 4 years ago

mikeweilgart commented 4 years ago

puzz-Delay Branch.mftsv.txt

When a BMEM instruction is encountered, this solution is designed to delay going on to the next instruction (ANY next instruction) until the BMEM instruction has made it past the caching register, at which point the memory values are read and compared and the "branch/don't branch" decision is conveyed back (by means of an input selector) to the latch which contains the "next memory slot to read from (for instructions)".

However, there is a bug in this design: the "branch/don't branch" information is kept too long. If the VERY NEXT instruction after a BMEM instruction (whether branched or not) is another BMEM instruction, the decision from the previous BMEM instruction will be applied to the new BMEM instruction as well. If the decision was "branch" this won't matter because the machine will branch to the same instruction as before, so it will just wait an extra clock cycle (as it should anyway). However, if the decision is "not branch" this will be incorrectly applied to the next BMEM instruction.

(The "Goal" for this level is 22 modules; I did it in 16 but the solution is buggy.)

The tests don't catch this.

Later I may try to write a failing test on this machine, or someone else could. The test would have to have something like:

5: BMEM 51 52 12 # where this should not branch 6: BMEM 61 62 9 # and this SHOULD branch 7: MSET -8 - 8 # so this shouldn't be run, but on the machine shared above it will be run.

PleasingFungus commented 4 years ago

Ahh, some of the later levels aren't as well tested as I'd like... thanks for catching this one! I've made a note.

amonakov commented 4 years ago

@mikeweilgart see also issue #118 — from your description I think it's the same

mikeweilgart commented 4 years ago

@amonakov yes, I think so. Ironically, I haven't yet worked out how to fix that bug in my mock CPU—even though I can proudly say I spotted the bug without even having a test to help me find it!