minimig-mist icon indicating copy to clipboard operation
minimig-mist copied to clipboard

Race condition in Copper code?

Open dirkwhoffmann opened this issue 6 years ago • 8 comments

While reviewing the Copper code, I came across the following two code blocks:

always @(posedge clk)
    ...
    	copjmp1 = 1;
...
always @(posedge clk)
   ...
  	if (copjmp1 && clk_ena)

Isn’t there a race condition here, because copjmp1 is written via a blocking assignment and read in the if statement at the same clock event?

dirkwhoffmann avatar Feb 21 '19 17:02 dirkwhoffmann

I didn't analyzed the copper code, but this style is valid. With blocking assignment copjmp1 will be valid in the same clock cycle (not in the next in case of non-blocking). Synthesizer is aware of such assignment and will synthesize it correct, although synthesized logic will be more complex. Blocking assignment for non-local signal is generally bad idea, but if you will change it to non-blocking assignment it may brake something.

sorgelig avatar Feb 21 '19 18:02 sorgelig

Thanks a lot for your replay, Sorgelig. I agree that timing will break if we changed it to non-blocking statements. But can we really assume that copjmp1 is assigned 1 before the if statement is evaluated. As far as I understand the Verilog spec, there is no guaranteed evaluation order between the two always blocks. If the if statement is evaluated first, it would be evaluated with an old value of copjmp1.

dirkwhoffmann avatar Feb 22 '19 09:02 dirkwhoffmann

Hi,

thanks for pointing this out.

In implemented design, there should be no issue.

The synthesis tool should do 'the right thing', that is, ignore blocking assignment and treat it as a non-blocking.

Simulation tools, on the other hand, won't, so this is probably a simulation - synthesis mismatch, and needs to be fixed.

(Pull requests are welcome ;) )

rkrajnc avatar Feb 22 '19 10:02 rkrajnc

I'm still a little bit confused about the intended behaviour:

always @(posedge clk)
    ...
    	copjmp1 = 1;
...
always @(posedge clk)
   ...
  	if (copjmp1 && clk_ena)
        strobe = 1; 

If the synthesis tool ignores the blocking assignments and treats the assignments as non-blocking, this means that:

  • On the first positive clock edge, copjmp1 is set to 1
  • On the second positive clock edge, strobe is set to 1 (if clk_ena is 1)

Is this correct?

dirkwhoffmann avatar Feb 22 '19 10:02 dirkwhoffmann

Yes, that looks correct. I would test if first though, just to be sure.

While I wrote that the synthesis tool will do the right thing, you shouldn't assume that will always be the case, just that in this specific case it should be* - that is why it is very unwise to use blocking assignments in an edge-sensitive block.

*In this specific case, the copjmp1 reg is written in a separate block than the block where it is read, so this should synthesize as non-blocking.

rkrajnc avatar Feb 22 '19 11:02 rkrajnc

BTW, if you look at the code, you can see that if copjmp1 was updated immediately, then the if (copjmp1 && clk_ena) would never be true, so this can't be how the synthesized design works.

rkrajnc avatar Feb 22 '19 11:02 rkrajnc

Yes, you're right, I didn't look at the else part testing clk_ena. So everything must be treated as non-blocking as it is typical for sequential logic. Thanks a lot for your help!

I did ask, because I am currently trying to translate the Copper code into a graphical representation that exhibits the state model together with the exact timing, simply to learn the details about Copper. I started out with looking at UAE, but I gave up quickly, because the UAE code has gotten so complex over the years. While looking for alternatives, I stumbled across the Minimig project which I wasn't aware of before. What I've seen so far is very pleasing, and the code is well documented. Great project!

dirkwhoffmann avatar Feb 22 '19 11:02 dirkwhoffmann

Looking forward to your work on the vAmiga!

rkrajnc avatar Feb 22 '19 12:02 rkrajnc