Is the following code going to make a race condition?

manili · Sep 25, 2016

I'm trying to make a pipeline processor using Verilog HDL. I realized that there are maybe some race conditions somewhere in my code. So I'm going to write a sudo code and would like to ask you about if there is a race condition within and how to avoid it :

Code:

module A(input wire reset, input wire clock, output reg a_reg_o);
   always @(posedge clock)
   begin
      if(reset == 1'h1)
      begin
         a_reg_o = 1'h0;
      end
      else
      begin
         a_reg_o = 1'h1;
      end
   end
endmodule

module B(input wire reset, input wire clock, input a_i);
   reg b;

   always @(posedge clock)
   begin
      if(reset == 1'h1)
      begin
         b = 1'h0;
      end
      else
      begin
         if(a_i == 1'h1)
         begin
            b = 1'h1;
         end
         else
         begin
            b = 1'h0;
         end
      end
   end
endmodule

module Main(input wire reset, input wire clock);
   wire a_o;
   A a(reset, clock, a_o);
   B b(reset, clock, a_o);
endmodule

So imagine I trigger reset signal. After the first positive edge of clock the register a_reg_o is going to be 0 and register b from module B also is going to be 0 (No race conditions yet). Now I release the reset button and let it to be negative. On the next positive edge of the clock the register a_reg_o is going to be 1, but what about register b from module B ? Is it going to be :
1. Zero, Because it didn't see the a_i changes yet.
2. It depends on the modules (A and B) total delay ( i.e. a race condition).

Thank you.

vGoodtimes · Sep 26, 2016

Using "=", the behavior is not defined.

The general rule is to only use "<=" assignments in clocked always blocks, and to only use "=" assignments in non-clocked always blocks. The idea is that Verilog simulations can process the logic in every triggered always block in any order (or in parallel).

manili · Sep 26, 2016

Thanks a lot for your reply.
Is the race condition only a simulation problem or it's also a physical implementation (post synthesis) issue ? I mean is it possible to have a different behavior due to delay of different paths, after synthesis ?

pbernardi · Sep 26, 2016

Just use the "<=" inside always blocks instead "=" as vGoodtimes suggested.

Additionally, I strongly avoid to use always to non-clocked signals (ex: "always @(reset)") . It generates combinational logic which can be almost always be achieved by other ways.

Just follow these two rules and you will avoid 99% of race conditions. You still may faces some race conditions when you make combinational logic, but then it is usually a fail of programming logic than a fail in the code.

Going out of help scope you have asked, I personally also always try to use the ? : operands instead if-else whenever I can. The ? : operands fits very well into hardware, its like a magic description of a flip-flop, and save lot of lines into your code.

For example:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
module A(input wire reset, input wire clock, output reg a_reg_o);
   always @(posedge clock)
   begin
      if(reset == 1'h1)
      begin
         a_reg_o = 1'h0;
      end
      else
      begin
         a_reg_o = 1'h1;
      end
   end
endmodule

could be write as:

Code Verilog - [expand]
1
2
3
4
5
6
module A(input wire reset, input wire clock, output reg a_reg_o);
   always @(posedge clock)
   begin
      a_reg_o <= (reset == 1'h1) ? 1'h0 : 1'h1;
   end
endmodule

This would led to a "a_reg_out" flip-flop that chose between 0 and 1, depending of the "reset" input .

But, inside the FPGAs, generally there is a dedicated pin to reset! This mean, you don't need to use additional logic to use the reset. For example, The module "B" as you wrote probably uses two flip-flops. Rewriting it to use ? : operands:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
module B(input wire reset, input wire clock, input a_i);
   reg b;
 
   always @(posedge clock)
   begin
      b <= reset    ? 1'h0 :
           a_i      ? 1'h1 : 1'h0;
   end
endmodule

You look at the code and you see: it uses cascated flip-flops (for reset and a_i). Your software can optmize it to use the reset or not. But if you write:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
module B(input wire reset, input wire clock, input a_i);
   reg b;
 
   always @(posedge clock)
   begin
      if (reset)
          b <= 0;
      else
          b <= a_i  ? 1'h1 : 1'h0;
   end
endmodule

You will use one flip-flop with reset.

EDIT: Note that if you want to use the flip-flop reset, the output must be always "0". For example, if you would like the reset to put "1" you your output, this would lead to use of 2 flip-flops, because flip-flop reset cannot be used in this case.

ads-ee · Sep 26, 2016

pbernardi said:
Just use the "<=" inside always blocks instead "=" as vGoodtimes suggested.

Additionally, I strongly avoid to use always to non-clocked signals (ex: "always @(reset)") . It generates combinational logic which can be almost always be achieved by other ways.

Just follow these two rules and you will avoid 99% of race conditions. You still may faces some race conditions when you make combinational logic, but then it is usually a fail of programming logic than a fail in the code.

The first rule should really be: use '<=' in edge triggered always blocks '=' should be used exclusively in combinational always blocks.
The second rule I don't consider a rule, it's more of a suggestion to not use combinational logic, which I consider more of a coding style issue.

pbernardi said:
Going out of help scope you have asked, I personally also always try to use the ? : operands instead if-else whenever I can. The ? : operands fits very well into hardware, its like a magic description of a flip-flop, and save lot of lines into your code.

Magic description of a FF!? Where did you come up with that? From the LRM....

Code:
Code:

conditional_expression ::= // from A.8.3 cond_predicate ? { attribute_instance } expression : expression cond_predicate ::= // from A.6.6 expression_or_cond_pattern { &&& expression_or_cond_pattern } expression_or_cond_pattern ::= expression | cond_pattern cond_pattern ::= expression matches pattern

Syntax 11-2—Conditional operator syntax (excerpt from Annex A)
This subclause describes the traditional notation where cond_predicate is just a single expression.
SystemVerilog also allows cond_predicate to perform pattern matching, which is described in 12.6.
If cond_predicate is true, the operator returns the value of the first expression without evaluating the second
expression; if false, it returns the value of the second expression without evaluating the first expression. If
cond_predicate evaluates to an ambiguous value (x or z), then both the first expression and the second
expression shall be evaluated, and compared for logical equivalence as described in 11.4.5. If that
comparison is true (1), the operator shall return either the first or second expression. Otherwise the operator
returns a result based on the data types of the expressions.

bunch of code removed...

pbernardi said:
This would led to a "a_reg_out" flip-flop that chose between 0 and 1, depending of the "reset" input .

This isn't a magic FF, it's a 2-to1 multiplex.

pbernardi said:
Code:

always @(posedge clock) begin if (reset) begin a_reg_o = 1'h0; end else begin a_reg_o = 1'h1; end end

But, inside the FPGAs, generally there is a dedicated pin to reset! This mean, you don't need to use additional logic to use the reset. For example, The module "B" as you wrote probably uses two flip-flops. Rewriting it to use ? : operands:

What!? What tool would use two flip-flops to implement the reformatted code shown above? It is ONE FF called a_reg_o that has a synchronous reset. What makes you think it will use two FFs!?

pbernardi said:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
module B(input wire reset, input wire clock, input a_i);
   reg b;
 
   always @(posedge clock)
   begin
      b <= reset    ? 1'h0 :
           a_i      ? 1'h1 : 1'h0;
   end
endmodule

You look at the code and you see: it uses cascated flip-flops (for reset and a_i). Your software can optmize it to use the reset or not. But if you write:

What!? I think you need to READ the LRM. There isn't a cascaded of FFs, what you've done is find an obscure way of writing my clear easy to see reformatted code above using two conditionals (i.e 2-to-1 multiplexers)

pbernardi said:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
module B(input wire reset, input wire clock, input a_i);
   reg b;
 
   always @(posedge clock)
   begin
      if (reset)
          b <= 0;
      else
          b <= a_i  ? 1'h1 : 1'h0;
   end
endmodule

You will use one flip-flop with reset.

No, this is functionally identical to all the other versions you've written...A synchronous resetable D FF.

pbernardi said:
EDIT: Note that if you want to use the flip-flop reset, the output must be always "0". For example, if you would like the reset to put "1" you your output, this would lead to use of 2 flip-flops, because flip-flop reset cannot be used in this case.

I have no clue what you are trying to imply here, though I suspect you have a misunderstanding of the fundamentals.

FYI this is how you should code a D-FF not the ugly way you've describe it.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
// for those that like really concise code:
always @ (posedge clk)
  if (reset) b <= 1'b0;
             b <= a;
 
// somewhat more verbose but preferred by me:
always @ (posedge clk) begin
  if (reset) begin
    b <= 1'b0;
  end else begin
    b <= a;
  end
end

No confusing conditional expressions assigning constants.

- - - Updated - - -

I sure hope I never have to work on your (i.e. pbernardi) code. I'd probably toss it and just rewrite it.

manili · Sep 26, 2016

Thank you all for your replies. But come on guys I'm a newbie which answer should I trust ? There are only two (somewhat) common things between answers :
1. Use <= when you wanna use edge trigger signals.
2. Use = when you wanna use combinational circuit.

@ads-ee what is LRM ?

pbernardi · Sep 26, 2016

ads-ee said:
The first rule should really be: use '<=' in edge triggered always blocks '=' should be used exclusively in combinational always blocks.
The second rule I don't consider a rule, it's more of a suggestion to not use combinational logic, which I consider more of a coding style issue.

Exactly, more of a coding style issue. I prever avoiding combinational always blocks and use assign instead. Once I never use combinational always, I only put "<=" inside always blocks.

Magic description of a FF!? Where did you come up with that? From the LRM....

Don't be too literal. Do you think that "High speed digital design: a handbook of black magic" is a book that makes a correlation between digital design and magic as well?

I only mean that ? : operands are perfectly suited for FPGA, more than if-else combination. That happens because if-else take more space, and you can use "if" without the "else". Using "if" only may lead to undetermined results, while when you use ? : operators, you always explicitly determine what is the result in both cases, positive or negative.

This isn't a magic FF, it's a 2-to1 multiplex.

OK.

What!? What tool would use two flip-flops to implement the reformatted code shown above? It is ONE FF called a_reg_o that has a synchronous reset. What makes you think it will use two FFs!?

I did not use all the available tools, but with a more complex attributions the tools often fails to identify a reset. The example above was very simple and I believe most of tools could identify the reset correctly. But when you have complexes chains of if-else, you have to be more explicit.

What!? I think you need to READ the LRM. There isn't a cascaded of FFs, what you've done is find an obscure way of writing my clear easy to see reformatted code above using two conditionals (i.e 2-to-1 multiplexers)

You think it is obscure, I respect that. Now, I do not want to write the example code below, for example, in 50 lines:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
if (IMMDP2 || MIDX)
    PMYP2 <= 1'b0;
else    
    PMYP2 <=    IDATA[27] && !MIDX  ? IDATA[21]     :
                INST == `OP5        ? IDATA[21]     :
                IDATA[30] && OP     ? IDATA[21]     :
                CFG                 ? IDATA[21]     :                           
                INST == `RTT1       ? IDATA[16]     :
                INST == `RTT3       ? IDATA[7]      : 1'b0;

No, this is functionally identical to all the other versions you've written...A synchronous resetable D FF.

Please note that I respected original OP modules A and B and kept them separated. You are mixing them, you cited module A:

Code:

always @(posedge clock) begin
  if (reset) begin
    a_reg_o = 1'h0;
  end else begin
    a_reg_o = 1'h1;
  end
end

]

I have no clue what you are trying to imply here, though I suspect you have a misunderstanding of the fundamentals.

FYI this is how you should code a D-FF not the ugly way you've describe it.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
// for those that like really concise code:
always @ (posedge clk)
  if (reset) b <= 1'b0;
             b <= a;
 
// somewhat more verbose but preferred by me:
always @ (posedge clk) begin
  if (reset) begin
    b <= 1'b0;
  end else begin
    b <= a;
  end
end

No confusing conditional expressions assigning constants.

That's almost the same thing I wrote, without the ? : operators, just because they were reduced (once you can reduce the b <= a ? 1'b1 : 1'b0; to b<= a. I did not notice this reduction could occur here, therefore I did not took this simplification into account. From here the additional flip flop came as well.

What I meant is that the ? : operators works much better when you do not have these simplifications.

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
reg [7:0] a,b=0;
 
always @ (posedge clk) 
begin
  if (reset) 
    b <= 8'h0;
  end else 
    b <= (a == 8'h55) ? 8'h11 : 8'h22;
end

looks much more clean than:

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
reg [7:0] a,b=0;
 
always @ (posedge clk) begin
  if (reset) begin
    b <= 8'h0;
  end else begin
  if (a==8'h55)
  begin
    b <= 8'h11;
  end else
  begin
    b <= 8'h22;
  end
end

And this is more and more true when you have to cascade more logic. Once OP stated that he needs to work with a pipelined processor, and I think ? : are the operators that should be used for this. They are perfect for pipelines, and you even if you do not like it, you cannot deny the are correct.

manili · Sep 26, 2016

@pbernardi
But what about some if/else with multiple lines of code ? I think you admit that using different styles of coding is not good ? However I agree that ? : operand would use much less lines of code.

pbernardi · Sep 26, 2016

Hey man, sorry for messing up your thread.

Assuming you correct the "=" assigments using "<=" instead, you design is syncronous:

Module A always give "1" out of reset, syncronous to clock;
Module B always give "a" level when out of reset, syncronous to clock.

As they are in cascade, this is the behavior:

1: reset = 0; a = 0; b = 0; (initial state)
2: reset = 1; a = 1; b = 0; (fist clock after reset)
3: reset = 1; a = 1; b = 1; (second clock+ after reset. It will keep its state if "a" does not change).

Note that you can simulate it by yourself making a simulation file.

- - - Updated - - -

Mohammad Amin Nili said:
@pbernardi
But what about some if/else with multiple lines of code ? I think you admit that using different styles of coding is not good ? However I agree that ? : operand would use much less lines of code.

Even better, you can use for example:

Code Verilog - [expand]
1
2
3
4
5
b <= condition 1 ? result 1 : 
     condition 2 ? result 2 : 
     condition 3 ? result 3 :
     ...
     condition n ? result n : result n+1;

This could be written in a single line, but then the code is too confusing.

Sometimes you can find more advanced things like this:

Code Verilog - [expand]
1
2
3
b <= condition 1 ? condition 2 ? result 2 :
                   condition 3 ? result 3 :
                   condition 4 ? result 4 : result 5 : result 6;

I the case above, condition 2, 3 and 4 are checked inside condition 1. If condition 2, 3 and 4 fails, result 5 is given. If condition 1 fails, result 6 is chosen. If you feel confused, you can always add parenthesis:

Code Verilog - [expand]
1
2
3
b <= (condition 1 ? (condition 2 ? result 2 :
                    (condition 3 ? result 3 :
                    (condition 4 ? result 4 : result 5))) : result 6);

Well, as you can see, note that some people does not like this style of coding. But I see this very well-fitted to FPGA design.

ads-ee · Sep 26, 2016

pbernardi said:
Well, as you can see, note that some people does not like this style of coding. But I see this very well-fitted to FPGA design.

I don't like it for reset, I think it obfuscates the code for no good reason.

"if-else if-else" is well supported by synthesis tools so there is no reason to code resets using ?:.

The cases you made later for using it in multi conditions is fine as long as the user understands it is still an if-else if-else structure and is therefore still priority encoded (even though it looks more like a case statement). I use ?: but not to the extent you seem to like to use it for everything.

Is the following code going to make a race condition?

manili

Member level 1

vGoodtimes

Advanced Member level 4

manili

manili

Member level 1

pbernardi

Full Member level 3

manili

ads-ee

Super Moderator

ThisIsNotSam

manili

manili

Member level 1

pbernardi

Full Member level 3

manili

Member level 1

pbernardi

Full Member level 3

manili

ads-ee

Super Moderator

manili

Similar threads

Code Verilog - [expand]
1 2 3 4 5 6	module A(input wire reset, input wire clock, output reg a_reg_o); always @(posedge clock) begin a_reg_o <= (reset == 1'h1) ? 1'h0 : 1'h1; end endmodule

Code Verilog - [expand]
1 2 3 4 5 6 7 8 9	module B(input wire reset, input wire clock, input a_i); reg b; always @(posedge clock) begin b <= reset ? 1'h0 : a_i ? 1'h1 : 1'h0; end endmodule

Code Verilog - [expand]
1 2 3 4 5 6 7 8 9 10 11	module B(input wire reset, input wire clock, input a_i); reg b; always @(posedge clock) begin if (reset) b <= 0; else b <= a_i ? 1'h1 : 1'h0; end endmodule

Code Verilog - [expand]
1 2 3 4 5 6 7 8 9	reg [7:0] a,b=0; always @ (posedge clk) begin if (reset) b <= 8'h0; end else b <= (a == 8'h55) ? 8'h11 : 8'h22; end

Code Verilog - [expand]
1 2 3 4 5 6 7 8 9 10 11 12 13 14	reg [7:0] a,b=0; always @ (posedge clk) begin if (reset) begin b <= 8'h0; end else begin if (a==8'h55) begin b <= 8'h11; end else begin b <= 8'h22; end end

Code Verilog - [expand]
1 2 3 4 5	b <= condition 1 ? result 1 : condition 2 ? result 2 : condition 3 ? result 3 : ... condition n ? result n : result n+1;

Code Verilog - [expand]
1 2 3	b <= condition 1 ? condition 2 ? result 2 : condition 3 ? result 3 : condition 4 ? result 4 : result 5 : result 6;

Code Verilog - [expand]
1 2 3	b <= (condition 1 ? (condition 2 ? result 2 : (condition 3 ? result 3 : (condition 4 ? result 4 : result 5))) : result 6);

Is the following code going to make a race condition?

Member level 1

Advanced Member level 4

Member level 1

Full Member level 3

Super Moderator

Member level 1

Full Member level 3

Member level 1

Full Member level 3

Super Moderator

Similar threads

Privacy & Transparency

Privacy & Transparency