OpenCores

CALL instruction doesn´t seem to properly store PC

Back to bugtracker overview.

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

Description:
Good day!

I´m experiencing issues with CALL instruction. SP register (latch register as defined in line 71 of NextZ80Reg.v) contains 0xFF56. The CALL instruction is located at 0x128B. Return address is hence 0x128E. After reading the destination call address, I can see two write cycles, but the address to write to is 0xFFFF (for both writes). After that, SP register has the value 0xFFFF (instead of 0xFF54 as it should be).

I've tracked the offending behaviour down to a specific case statement in NextZ80Reg.v which assigns a value to mux_rdor. This case statement needs proper values, but you supply values with "x" bits, which is not acceptable. I´ve temporarily fixed it by changing the case into a casex statement, but I'm well aware that casex should be avoided by all means.

I guess all your "x" bits should be "?" bits instead, and use casez instead of casex when applicable.

I'm using iSIM with ISE 14.7 . The design uses the same WAIT pattern that you use for your example app.

Comments:

Dumitrache, Nicolae May 14, 2018
Regarding the number of LUTs, the situation may be different on other synthesis tools (Vivado, Quartus, Lattice Diamond).
Dumitrache, Nicolae May 14, 2018
Yes, you may replace the 'x' with '0' or '1',or use the "Post-Translate" simulation.
Rodriguez Jodar, Miguel Angel May 13, 2018
At the end, I've decided to get rid of all "x" values and put "0" instead. A quick check synthesizing the CPU yields very similar numbers for LUT occupancy
Rodriguez Jodar, Miguel Angel May 10, 2018
According to this post:
https://electronics.stackexchange.com/questions/225104/assigning-x-in-verilog

"x" should be used only to propagate "unkown" conditions along the design, and only for simulation purposes, but not to denote "don't cares", although some tools may use it that way.

In this other post:
https://embdev.net/topic/276558

there seems to be some confusion about using "x", "z" or "?". At the end, the Verilog standard is what dictates the right form:

"You are right, see IEEE Std 1364-2001 Chapter 9.5.1
To make the intent more vissible I suggest using casez with '?'."

Verilog standard (2002):
http://www.iuma.ulpgc.es/~nunez/clases-FdC/verilog/Verilog-IEEE-1364.pdf

On section 5.5 this can be read:

"5.5 Support for values x and z

The value x may be used as a primary on the RHS of an assignment to indicate a don’t care value for
synthesis.
The value x may be used in case item expressions (may be mixed with other expressions, such as 4’b01x0)
in a casex statement to imply a don’t care value for synthesis.
The value x shall not be used with any operators or mixed with other expressions.
The value z may be used as a primary on the RHS of an assignment to infer a three-state driver as described
in 5.4.
The value z (or ?) may be used in case item expressions (may be mixed with other expressions, such as 4’bz1z0) for casex and casez statements to imply a don’t care value for synthesis.
The value z shall not be used with any operators or mixed with other expressions."

So, yes, "x" can be used no denote "don't care" values for synthesis, but then, you have to use "casex" for correct behavioural simulation, not "case" as NextZ80Reg.v shows.

"?" is a synonim of "z", but only for casez/casex statements. If you want to state that a signal is a "don't care", and not "unkown" or "high-impedance", "?" is the way to go.
Dumitrache, Nicolae May 10, 2018
I assign 'x' as don't care in order to help for a tighter synthesis. I'm not sure what '?' is doing, so I don' use it (maybe it is more like high impedance, but I would not use high impedance for internal signals in FPGA). If you don't like 'x', the simplest solution is to replace all assigned 'x' values with '0' or '1'. But then, you may expect an increased number of used LUTs. As I don't use 'x' in the case constants, I don't think "casex" is required anywhere in the design. Indeed, as you noticed, ISIM simulator may have problems with these 'x' values, but I consider it is a simulator issue and not a design one. You can avoid it by using "Post-Translate" simulation instead of "Behavioral" one. (I repeated the answer as I noticed some of the words were cut because they were inside angle brackets).
Dumitrache, Nicolae May 10, 2018
I assign 'x' as don't care in order to help for a tighter synthesis. I'm not sure what '?' is doing, so I don' use it (maybe it is more like high impedance, but I would not use high impedance for internal signals in FPGA). If you don't like 'x', the simplest solution is to replace all assigned 'x' values with '0' or '1'. But then, you may expect an increased number of used LUTs. As I don't use 'x' in the case constants, I don't think is required anywhere in the design. Indeed, as you noticed, ISIM simulator may have problems with these 'x' values, but I consider it is a simulator issue and not a design one. You can avoid it by using "Post-Translate" simulation instead of "Behavioral" one. (I repeated the answer as I noticed some of the words were cut because they were inside angle brackets).
Dumitrache, Nicolae May 10, 2018
I assign 'x' as don't care in order to help for a tighter synthesis. I'm not sure what '?' is doing, so I don' use it (maybe it is more like high impedance, but I would not use high impedance for internal signals in FPGA). If you don't like 'x', the simplest solution is to replace all assigned 'x' values with '0' or '1'. But then, you may expect an increased number of used LUTs. As I don't use 'x' in the case constants, I don't think casex is required anywhere in the design. Indeed, as you noticed, ISIM simulator may have problems with these 'x' values, but I consider it is a simulator issue and not a design one. You can avoid it by using simulation instead of one.
Dumitrache, Nicolae May 10, 2018
I assign 'x' as don't care in order to help for a tighter synthesis. I'm not sure what '?' is doing, so I don' use it (maybe it is more like high impedance, but I would not use high impedance for internal signals in FPGA). If you don't like 'x', the simplest solution is to replace all assigned 'x' values with '0' or '1'. But then, you may expect an increased number of used LUTs. As I don't use 'x' in the case constants, I don't think is required anywhere in the design. Indeed, as you noticed, ISIM simulator may have problems with these 'x' values, but I consider it is a simulator issue and not a design one. You can avoid it by using simulation instead of one.
Rodriguez Jodar, Miguel Angel May 10, 2018
Of course:

NextZ80CPU.v : line 789, there is this:
REG_RSEL = 4'b101x;

Using "x" as "don't care" value is not the recommended way. It's better to use "?".

Because of this, when REG_RSEL is going to be evaluated here:

NextZ80Reg.v : line 139 and following.
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

It would be expected that the case with
5'b01010,5'b01011 : mux_rdor = sp;
should be chosen, but actually, it's the "default" one which is chosen because "case" doesn't work with "x", "z" or "?". It works in synthesis because there is no "x" in synthesis, but there is on simulation, and because of that, iSIM cannot show me a correct behaviour of the design.

A temporary fix is to replace "case" with "casex", which does take "x" into account, but it's advisable not to use this type of "casex" for synthesizable designs.

IMHO, the right thing to do would be to use "?" instead of "x" to denote "don't cares" and use "casez" instead of "case" or "casex" to evaluate expressions containing these "don't care" bits.
Dumitrache, Nicolae May 10, 2018
Can you give me the exact line in NextZ80Reg.v where you found the issue?

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.