nand2tetris / web-ide

A web-based IDE for https://nand2tetris.org
https://nand2tetris.github.io/web-ide
Other
38 stars 11 forks source link

Capture CPUOutput in tock before updating CPUState from tock #340

Closed DavidSouther closed 3 weeks ago

DavidSouther commented 3 weeks ago

Closes #337

netalondon commented 3 weeks ago

Doesn't this conflict with CPU.tst which expects the current behavior? Is the test wrong then?

axelkern commented 3 weeks ago

If I understand the design of the CPU simulator correctly, the idea is that after the "tock" the CPU shows some intermediate state where no new instruction has been fetched, but the state of the CPU is otherwise updated and therefore the combinatorial outputs of the CPU show then a mix of old instruction with new register states. That is the reason of outM being wrong in this case.

This is somehow deviating from the hardware design of the Hack computer in my opinion, where the memory is implemented with a pure combinatorial Mux/Demux system without registering addresses. Consequently a new instruction will be immediately "fetched" when PC changes since it is just combinatorial. In a real world implementation, the best approach is probably to fetch the new instruction on the inverted clock signal, which is implicitly what the simulator does. In this case the outputs of the CPU would indeed show the above described intermediate state.

Proposal: If this design should be kept as is, then the simplest way of implementing it from my point of view is to move the memory write into tick() into cpuTock() and do the write before A is updated. I think in this case the rest of the code can remain as it was and the tests should work.

// move this into cpuTock() before if (!bits.c) {...
if (writeM) {
      this.RAM.set(addressM, outM);
}
DavidSouther commented 3 weeks ago

Closed in favor of #344