CPU.hdl: Comparison failure at line 29

[Chen Li]

Here is my CPU.hdl:

//deleted




the /jmp/ logic is if left and right are both 1(/and/) in one case(/or/), then it should jump.  But it fails in line 29. I cannot figure out where goese wrong. Thanks in advance!

[cadet1620]

Look at the the lines that are miscomparing:

     |time| inM  |  instruction   |reset| outM  |writeM |addre| pc  |DRegiste|
cmp: |14  | 11111|1110110111100000|  0  |*******|   0   | 1000|   16|     -1 |
out: |14  | 11111|1110110111100000|  0  |  -4639|   0   |28128|   16|     -1 |

The instruction that is failing is 1110110111100000. Look in CPU.tst to decode the instruction.

set instruction %B1110110111100000, // A=A+1

The previous instruction, @999, set A=999, so this instruction should set it to 1000.

The A register is being loaded with a value that appears to be unrelated to the ALU output.

Fix this one problem, and your CPU will pass.

Hint:

    // if this instruction is a-instruction
    Not(in=instruction[15], out=aInst);
    //if this instruction is c-instruction
    Not(in=aInst, out=cInst);

Making aInst and cInst is a very good idea. You should move this code to the beginning of PARTS: because it is very important.  Then you should use aInst and cInst throughout the remaining code instead if instruction[15]. This will make the code more understandable and help prevent errors like this one.

--Mark

Please edit your post to remove the HDL.

[Chen Li]

Thanks! I think this typo is made by emacs's dabbrev-expand.

[Moshe]

I have exactly the same problem and still don't understand what am I doing wrong. It passes all the other tests. Is it my first Mux sel? or the ARegister load? I would appreciate a hint

[cadet1620]

The instruction where you are failing, A=A+1, is the first C-command with the A Register destination bit set.

Check the logic for A Register's load pin and the select input on the Mux16 that connects to the A Register input.

One thing that you can do is single step the simulator and stop one line before the failure so that you are looking at the erroneous inputs to the A register just before the erroneous instruction executes. Trace the logic back to figure out where the error is.

If you don't see the problem, post the .out and .cmp files where the failure is. The specific wrong values will give a clue as to what is wrong.

[Moshe]

Thank you very much for your quick response. Here is the .out file

|time| inM  |  instruction   |reset| outM  |writeM |addre| pc  |DRegiste|
|0+  |     0|0011000000111001|  0  |      0|   0   |    0|    0|      0 |
|1   |     0|0011000000111001|  0  |      0|   0   |12345|    1|      0 |
|1+  |     0|1110110000010000|  0  |  12345|   0   |12345|    1|  12345 |
|2   |     0|1110110000010000|  0  |  12345|   0   |12345|    2|  12345 |
|2+  |     0|0101101110100000|  0  |     -1|   0   |12345|    2|  12345 |
|3   |     0|0101101110100000|  0  |     -1|   0   |23456|    3|  12345 |
|3+  |     0|1110000111010000|  0  |  11111|   0   |23456|    3|  11111 |
|4   |     0|1110000111010000|  0  |  12345|   0   |23456|    4|  11111 |
|4+  |     0|0000001111101000|  0  | -11111|   0   |23456|    4|  11111 |
|5   |     0|0000001111101000|  0  | -11111|   0   | 1000|    5|  11111 |
|5+  |     0|1110001100001000|  0  |  11111|   1   | 1000|    5|  11111 |
|6   |     0|1110001100001000|  0  |  11111|   1   | 1000|    6|  11111 |
|6+  |     0|0000001111101001|  0  | -11111|   0   | 1000|    6|  11111 |
|7   |     0|0000001111101001|  0  | -11111|   0   | 1001|    7|  11111 |
|7+  |     0|1110001110011000|  0  |  11110|   1   | 1001|    7|  11110 |
|8   |     0|1110001110011000|  0  |  11109|   1   | 1001|    8|  11110 |
|8+  |     0|0000001111101000|  0  | -11110|   0   | 1001|    8|  11110 |
|9   |     0|0000001111101000|  0  | -11110|   0   | 1000|    9|  11110 |
|9+  | 11111|1111010011010000|  0  |     -1|   0   | 1000|    9|     -1 |
|10  | 11111|1111010011010000|  0  | -11112|   0   | 1000|   10|     -1 |
|10+ | 11111|0000000000001110|  0  |   1000|   0   | 1000|   10|     -1 |
|11  | 11111|0000000000001110|  0  |     14|   0   |   14|   11|     -1 |
|11+ | 11111|1110001100000100|  0  |     -1|   0   |   14|   11|     -1 |
|12  | 11111|1110001100000100|  0  |     -1|   0   |   14|   14|     -1 |
|12+ | 11111|0000001111100111|  0  |      1|   0   |   14|   14|     -1 |
|13  | 11111|0000001111100111|  0  |      1|   0   |  999|   15|     -1 |
|13+ | 11111|1110110111100000|  0  |   1000|   0   |  999|   15|     -1 |
|14  | 11111|1110110111100000|  0  |  -4639|   0   |28128|   16|     -1 |
|14+ | 11111|1110001100001000|  0  |     -1|   1   |28128|   16|     -1 |
|15  | 11111|1110001100001000|  0  |     -1|   1   |28128|   17|     -1 |
|15+ | 11111|0000000000010101|  0  |  -4640|   0   |28128|   17|     -1 |

And this is the .cmp file

|time| inM  |  instruction   |reset| outM  |writeM |addre| pc  |DRegiste|
|0+  |     0|0011000000111001|  0  |*******|   0   |    0|    0|      0 |
|1   |     0|0011000000111001|  0  |*******|   0   |12345|    1|      0 |
|1+  |     0|1110110000010000|  0  |*******|   0   |12345|    1|  12345 |
|2   |     0|1110110000010000|  0  |*******|   0   |12345|    2|  12345 |
|2+  |     0|0101101110100000|  0  |*******|   0   |12345|    2|  12345 |
|3   |     0|0101101110100000|  0  |*******|   0   |23456|    3|  12345 |
|3+  |     0|1110000111010000|  0  |*******|   0   |23456|    3|  11111 |
|4   |     0|1110000111010000|  0  |*******|   0   |23456|    4|  11111 |
|4+  |     0|0000001111101000|  0  |*******|   0   |23456|    4|  11111 |
|5   |     0|0000001111101000|  0  |*******|   0   | 1000|    5|  11111 |
|5+  |     0|1110001100001000|  0  |  11111|   1   | 1000|    5|  11111 |
|6   |     0|1110001100001000|  0  |  11111|   1   | 1000|    6|  11111 |
|6+  |     0|0000001111101001|  0  |*******|   0   | 1000|    6|  11111 |
|7   |     0|0000001111101001|  0  |*******|   0   | 1001|    7|  11111 |
|7+  |     0|1110001110011000|  0  |  11110|   1   | 1001|    7|  11110 |
|8   |     0|1110001110011000|  0  |  11109|   1   | 1001|    8|  11110 |
|8+  |     0|0000001111101000|  0  |*******|   0   | 1001|    8|  11110 |
|9   |     0|0000001111101000|  0  |*******|   0   | 1000|    9|  11110 |
|9+  | 11111|1111010011010000|  0  |*******|   0   | 1000|    9|     -1 |
|10  | 11111|1111010011010000|  0  |*******|   0   | 1000|   10|     -1 |
|10+ | 11111|0000000000001110|  0  |*******|   0   | 1000|   10|     -1 |
|11  | 11111|0000000000001110|  0  |*******|   0   |   14|   11|     -1 |
|11+ | 11111|1110001100000100|  0  |*******|   0   |   14|   11|     -1 |
|12  | 11111|1110001100000100|  0  |*******|   0   |   14|   14|     -1 |
|12+ | 11111|0000001111100111|  0  |*******|   0   |   14|   14|     -1 |
|13  | 11111|0000001111100111|  0  |*******|   0   |  999|   15|     -1 |
|13+ | 11111|1110110111100000|  0  |*******|   0   |  999|   15|     -1 |
|14  | 11111|1110110111100000|  0  |*******|   0   | 1000|   16|     -1 |
|14+ | 11111|1110001100001000|  0  |     -1|   1   | 1000|   16|     -1 |
|15  | 11111|1110001100001000|  0  |     -1|   1   | 1000|   17|     -1 |
|15+ | 11111|0000000000010101|  0  |*******|   0   | 1000|   17|     -1 |

The error is on line 14
May I email you my CPU.hdl?
Thank you

[cadet1620]

Moshe wrote

Thank you very much for your quick response. Here is the .out file

|time| inM  |  instruction   |reset| outM  |writeM |addre| pc  |DRegiste|
|13+ | 11111|1110110111100000|  0  |   1000|   0   |  999|   15|     -1 |
|14  | 11111|1110110111100000|  0  |  -4639|   0   |28128|   16|     -1 |

And this is the .cmp file

|time| inM  |  instruction   |reset| outM  |writeM |addre| pc  |DRegiste|
|13+ | 11111|1110110111100000|  0  |*******|   0   |  999|   15|     -1 |
|14  | 11111|1110110111100000|  0  |*******|   0   | 1000|   16|     -1 |

This is an A=A+1 instruction.  The A register should have been loaded with 1000, but instead it was loaded with 28128.

This is an interesting number: 28128 = 110110111100000 binary, which is the low 15 bits of the instruction (addre[ssM] is the low 15 bits of the A Register).  This means that the "sel" input on the A Register input Mux16 is probably connected to the wrong thing.

Since this Mux16 controls whether the ALU output (for C instructions) or CPU instruction input (for A instructions) goes to A, it needs to be controlled by the "instruction" bit that differentiates the two types of instructions.

[Moshe]

Thank you
If I understand correctly, it means that the 'sel' logic of the first Mux is wrong.
But all the 'd' s are 0 so there is no jump and the Mux should choose the instruction and not the feed from the ALU. What is wrong is this logic?

[cadet1620]

There is a 'd':
    1110110111100000
I do not understand what you mean about jump. This mux has nothing to do with jumps.

Using the 'd' bits to control this mux will be a problem because those bits can be set in a-instructions, too.  You should only select the instruction to go to the A register for a-instructions. In all other cases (c-instructions since there are only two types of instructions 8-), select the ALU output.