Subject: Re: Using iff example - very bad coding style!
From: Clifford E. Cummings (cliffc@sunburst-design.com)
Date: Tue Dec 11 2001 - 14:39:52 PST
At 01:29 PM 12/11/01 -0800, you wrote:
>Cliff,
>
>First of all I want to say that you are right in that the original
>examples are not working. Your first remark: "On a posedge clk with no
>load signal, nothing happens, but both should count" was correct, as
>well as your second remark made on the bug inherited by me from the
>original description (which had the same problem):"if load == 0, the
>second always block will not permit synchronous resets to occur".
>
>So, here is my corrected version for synchronous reset:
>
>always @(posedge clock iff (load == 0) and (reset == 1))
> count <= count +1;
>
>always @(posedge clock iff (load == 1) or (reset == 0)) //synchronous reset
> if (!reset) count <= 0;
> else if (load) count <= d; //gated input
>
>Now, let us talk about race conditions. In case a register is
>written from two different processes, there could be a race condition
>only in case the two processes attempt to write into the register
>simultaneously, and the register could have either value as a result,
>depending on which process writes last. However, in this case the
>conditions for writing to count are mutual exclusive and therefore no
>race condition can occur.
True. As with most race conditions in Verilog, if the designer is careful
and runs an analysis of all possible cases, racey coding styles can be
coded without races. The same is true with using blocking assignments to
model sequential logic. If the designer is careful, the simulation will run
correctly and match the synthesized output. It is also possible to do most
surgeries without anesthesia; however, all of these examples can inflict a
great deal of pain.
As I stated before, I believe that it is never necessary to make
assignments to the same variable from more than one procedural block. Your
merged example below makes assignments to the variable from just one always
block using nonblocking assignments. Analysis complete! I don't have to
search my code to find all of the procedural blocks that make assignments
to this variable and insure that the assignments are guaranteed to be
mutually exclusive. I don't have to worry about blocking assignments
possibly introducing other race conditions to this model. Analysis complete!
The old Cadence synthesis tool, Synergy, required clocking and resetting to
be done from different always blocks using assign and deassign. I recently
had to help a customer convert Synergy code into synthesizable code. There
were assign and deassign assignments to the same variable from multiple
procedural blocks spread out over a very large RTL file. Figuring out which
had priority was a nightmare.
If the proposed enhancement is going to encourage the above coding style,
we should remove the enhancement.
If anybody working for me does RTL modeling using the above coding style,
they're fired!
I would suggest using the above example as an interview question for anyone
claiming to be a skilled Verilog behavioral or RTL coder: "What, if
anything, is wrong with the above model?" If the candidate takes time to
analyze all possible inputs combinations to determine that in this case,
the model will work, -10 points. If the first thing the candidate says is,
making assignments to the same variable from more than one procedural block
is a bad idea, +10 points.
One thing for sure, we should not include this type of example in the
SystemVerilog LRM.
>Of course, your training classes address the usage of existing tools
>and there you are the expert (I have no idea what Synopsis generates
>when it sees multiple processes writing to the same register :-)
>
>So, we may mearge the two processes, as follows:
>
>always @(posedge clock) //synchronous reset
> if (!reset) count <= 0;
> else if (load) count <= d; //gated input
> else count <= count + 1;
More concise. Easy to understand. A much better coding style, but it does
not use the new capabilities.
>Similarly, for the asynchronous reset:
>
>always @(posedge clock iff (load == 0) and (reset == 1))
> count <= count +1;
>
>always @(negedge reset) //asynchronous reset
> if (!reset) count <= 0;
>
>always @(posedge clock iff (load == 1) and (reset == 1)//gated input
> count <= d;
Another job interview question!
>or the mearged version:
>
>always @(posedge clock or negedge reset) //asynchronous reset
> if (!reset) count <= 0;
> else if (load) count <= d; //gated input
> else count <= count + 1;
Again, much more concise. Easy to understand. A better coding style. No
race analysis required.
>Alec
Regards - Cliff
//*****************************************************************//
// Cliff Cummings Phone: 503-641-8446 //
// Sunburst Design, Inc. FAX: 503-641-8486 //
// 14314 SW Allen Blvd. E-mail: cliffc@sunburst-design.com //
// PMB 501 Web: www.sunburst-design.com //
// Beaverton, OR 97005 //
// //
// Expert Verilog, Synthesis and Verification Training //
//*****************************************************************//
This archive was generated by hypermail 2b28 : Tue Dec 11 2001 - 14:40:22 PST