|
|
|
|
My current CPU implementation is failing on the instruction in location 7 (line 35) of CPU-external.tst:
set instruction %B1110001110011000, // MD=D-1
tick, output, tock, output;
If I understand correctly, at clock 7+ the instruction has been fetched and the ALU executes it. At that time the output from my D register is 11111 and so output from the ALU is 11110 which is fed back to the D register in my implementation. At clock 8, the new value is loaded in the D register, so D is now 11110, but the instruction hasn't changed yet so the ALU output should be come 11109. From both the values of internal pins and from the visualization diagram, it does seem that the x input into the ALU chip at that point is 11110, but the ALU is still showing output 11110 and the test case expects 11109. The diagram seems to imply that the ALU output is not correct, but the ALU chip is one of the givens in project 5. I'm at a loss as to how to track down the issue. Any ideas?
|
|
|
The way the output of the Hardware Simulator has been implemented can be a bit confusing and is not consistent from my point of view. The columns in the output for A (address) and PC are updated after the "tock", while the output column for D is already updated after the "tick". The impact is that temporarily the ALU inputs will already be fed with the value of the updated D register. In that specific case:
D=11111 -> MD=D-1=11110; after D=11110, the ALU and therefore outM will temporarily show "newD"-1=11109. That is however irrelevant for testing the correctness of your implementation and has no impact on the CPU or Memory state. Again, it is just a slightly confusing way of implementing the simulation steps.
In order to help you narrowing down the problem, could you share your CPU HDL code here?
|
|
|
Thanks for the offer, but I want to respect the author's wishes and not post any code publicly on the forum. Since there seems to be an inconsistency in the output (the visualization of the ALU is showing x input as 11110, ALU function as x-1 and ALU output as 11110) I was hoping someone else might have encountered a similar issue. It has the appearance of a timing issue/error, but I thought the ALU was a purely combinational circuit.
|
|
Administrator
|
This is not a problem -- we will simply remove the code once it has served its purpose.
|
|
Administrator
|
Another avenue to check is to use the Built-in ALU and see if it passes the tests.
|
|
|
I found the issue. Turns out the the hardware simulator is sensitive to the ordering of the statements in the hdl file. By changing the order of the statements I was able to pass the tests.
|
|
Administrator
|
It shouldn't be sensitive to the ordering. If you could post the two equivalent, but differently-ordered, files I will test them and try to either identify what is going on or, if I can confirm that they behave differently when they shouldn't, I'll bring it to the authors' attention.
|
|
|
This post was updated on .
Below is my HDL file for the CPU that fails the tests. To make it pass the test, simply move the first line (ALU) to the end.
// This file is part of www.nand2tetris.org
// and the book "The Elements of Computing Systems"
// by Nisan and Schocken, MIT Press.
// File name: projects/5/CPU.hdl
/**
* The Hack Central Processing unit (CPU).
* Parses the binary code in the instruction input and executes it according to the
* Hack machine language specification. In the case of a C-instruction, computes the
* function specified by the instruction. If the instruction specifies to read a memory
* value, the inM input is expected to contain this value. If the instruction specifies
* to write a value to the memory, sets the outM output to this value, sets the addressM
* output to the target address, and asserts the writeM output (when writeM = 0, any
* value may appear in outM).
* If the reset input is 0, computes the address of the next instruction and sets the
* pc output to that value. If the reset input is 1, sets pc to 0.
* Note: The outM and writeM outputs are combinational: they are affected by the
* instruction's execution during the current cycle. The addressM and pc outputs are
* clocked: although they are affected by the instruction's execution, they commit to
* their new values only in the next cycle.
*/
CHIP CPU {
IN inM[16], // M value input (M = contents of RAM[A])
instruction[16], // Instruction for execution
reset; // Signals whether to re-start the current
// program (reset==1) or continue executing
// the current program (reset==0).
OUT outM[16], // M value output
writeM, // Write to M?
addressM[15], // Address in data memory (of M)
pc[15]; // address of next instruction
PARTS:
<snip> Code removed after issue discussed to avoid giving away answers.
}
|
|
|
I could reproduce the bug in the new online tools with my own implementation. It worked flawlessly with the existing Java-based tools, but is sensitive to source code ordering with the online tools. I presume that these new tools are still under development and not officially published yet for a good reason.
|
|
|
There may be a slight issue with this code, which the tests don't seem to detect: in the line
Mux16(a=outA , b=inM , sel=instruction[12] , out=outAorM );
Don't we also need to check first whether the instruction is an A or C-instruction before using the a-bit?
I started the book this week so I may just be misunderstanding something
|
|
|
You are referring to combinatorial logic, so there is no "before" or "after". Everything happens at the same time, and as far as it is about combinatorial elements in the context of the Hardware Simulator, you can presume that it happens immediately as soon as the inputs change (in real life you will have some propagation delays).
The mentioned statement is therefore principally fine. In case of an A-Instruction, the output pins of that mux will just not be used, i.e. the load bits of the registers will only be active if it is a C-Instruction. So in case of an A-Instruction the ALU will still calculate something undefined, but it is meaningless and will just be ignored.
|
|
|
For future readers, this CPU implementation currently passes the online test suite.
|
|
Locked
