OpenCores

Instruction JP (HL) stalls the CPU

Back to bugtracker overview.

Information:
Type :: BUG
Status :: CLOSED
Assigned to :: nobody

Description:
When executin the JP (HL) instruction during a simulation, the CPU reads the opcode and the PC is not updated, so it stucks reading again the JP (HL) opcode from the same address, stalling the CPU if no interrupts are triggered.

While tracking down this bug, I noticed that although register mux_rdor had the right value (the current value from HL) for some reason, that value wasn't propaged to register ALU160 even when signal ALU160_sel had also the right value (0). Line #127 of NextZ80Reg.v should assign the current value of mux_rdor to ALU160 according to the current value of ALU160_sel, and therefore, input D0 to ALU16 should be the right value, making ADDR to be also the right value. None of this happened.

Then I noticed the combinatorial always block located from line #123 in NextZ80Reg.v that assigns values to several of such said signals. Comb always use blocking assignments so the order of such do matter.

The first sentences of this always block are:
always @* begin
DIN = DINW_SEL ? {DI, DI} : ALU8OUT;
ALU80 = REG_WSEL[0] ? rdow[7:0] : rdow[15:8];
ALU81 = REG_RSEL[0] ? mux_rdor[7:0] : mux_rdor[15:8];
ALU160 = ALU160_sel ? pc : mux_rdor;

The last one is that causes the issue. ALU160 is assigned with the current value of pc or mux_rdor depending on the value of ALU160_sel. The problem is that mux_rdor is not yet assigned a value. This is done later inside this always block. As order matter, the value of mux_rdor assigned to ALU160 is the one before the new assignment. In synthesis this bug get unnoticed because synthesizers can infer the right order based upon real dependencies between RHS and LHS parts of assignments.

In order for simulation to work correctly, the assignments inside the always block must be reordered so the ones that rely on variables which also get a value in a different assignment in the same always block, should be written after them. Another option is to break the always block into various always blocks so each one of them models a simpler combinational circuit with no interdependent assignments inside it. This is the approach I've taken. So, the always block starting from line #123 at NextZ80Reg.v ...

always @* begin
DIN = DINW_SEL ? {DI, DI} : ALU8OUT;
ALU80 = REG_WSEL[0] ? rdow[7:0] : rdow[15:8];
ALU81 = REG_RSEL[0] ? mux_rdor[7:0] : mux_rdor[15:8];
ALU160 = ALU160_sel ? pc : mux_rdor;

case({REG_WSEL[3], DO_SEL})
0: DO = ALU80;
1: DO = th;
2: DO = FLAGS;
3: DO = ALU8OUT[7:0];
4: DO = pc[15:8];
5: DO = pc[7:0];
6: DO = sp[15:8];
7: DO = sp[7:0];
endcase
case({ALU16OP == 4, REG_RSEL[3:0]})
5'b01001, 5'b11001: mux_rdor = {rdor[15:8], r};
5'b01010, 5'b01011: mux_rdor = sp;
5'b01100, 5'b01101, 5'b11100, 5'b11101: mux_rdor = {8'b0, CONST};
default: mux_rdor = rdor;
endcase
end

Becomes a series of always, like this:

always @* begin
DIN = DINW_SEL ? {DI, DI} : ALU8OUT;
ALU80 = REG_WSEL[0] ? rdow[7:0] : rdow[15:8];
ALU81 = REG_RSEL[0] ? mux_rdor[7:0] : mux_rdor[15:8];
ALU160 = ALU160_sel ? pc : mux_rdor;
end

always @* begin
case({REG_WSEL[3], DO_SEL})
0: DO = ALU80;
1: DO = th;
2: DO = FLAGS;
3: DO = ALU8OUT[7:0];
4: DO = pc[15:8];
5: DO = pc[7:0];
6: DO = sp[15:8];
7: DO = sp[7:0];
endcase
end

always @* begin
case({ALU16OP == 4, REG_RSEL[3:0]})
5'b01001, 5'b11001: mux_rdor = {rdor[15:8], r};
5'b01010, 5'b01011: mux_rdor = sp;
5'b01100, 5'b01101, 5'b11100, 5'b11101: mux_rdor = {8'b0, CONST};
default: mux_rdor = rdor;
endcase
end

which fixed my issue.

Comments:

Rodriguez Jodar, Miguel Angel May 17, 2018
The bug manifested after I changed all "x" by "0" in the code.
Dumitrache, Nicolae May 14, 2018
You may replace the 'x' with '0' or '1',or use the "Post-Translate" simulation.

Post a comment:
Login to post comments!

Back to bugtracker overview.

© copyright 1999-2019 OpenCores.org, equivalent to Oliscience, all rights reserved. OpenCores®, registered trademark.