SpinalHDL
SpinalHDL copied to clipboard
Proper handling of blocking and non-blocking assigns in generated Verilog
I created the following code that takes a vector of inputs and counts the number of elements that surpass a threshold (100). The resulting verilog code uses non-blocking assigns -- and therefore won't evaluate correctly since the value of the signal is updated in the next simulator delta cycle. Therefore, a blocking assign should be used for clarity and proper handling by following tools like simulator and synthesizer.
Here my code snipped in SpinalHDL
//Hardware definition
class MyTopLevel extends Component {
val io = new Bundle {
val latch_enable = in Bool
val input_data = in Vec(UInt(16 bits), 4)
}
val larger_than_N = out UInt(8 bits)
val larger_than_N_reg = Reg(UInt(8 bits)) init(0)
val data_reg = Vec(Reg(UInt(16 bits)) init(0), 4)
when(io.latch_enable){
data_reg := io.input_data
larger_than_N_reg := 0
for(idx <- 0 to 3) {
when (data_reg(idx) >= 100) {
larger_than_N_reg := larger_than_N_reg + 1
}
}
}
larger_than_N := larger_than_N_reg
}
module MyTopLevel (
input io_latch_enable,
input [15:0] io_input_data_0,
input [15:0] io_input_data_1,
input [15:0] io_input_data_2,
input [15:0] io_input_data_3,
output [7:0] larger_than_N,
input clk,
input reset
);
reg [7:0] larger_than_N_reg;
reg [15:0] data_reg_0;
reg [15:0] data_reg_1;
reg [15:0] data_reg_2;
reg [15:0] data_reg_3;
wire when_MyTopLevel_l41;
wire when_MyTopLevel_l41_1;
wire when_MyTopLevel_l41_2;
wire when_MyTopLevel_l41_3;
assign when_MyTopLevel_l41 = (16'h0064 <= data_reg_0);
assign when_MyTopLevel_l41_1 = (16'h0064 <= data_reg_1);
assign when_MyTopLevel_l41_2 = (16'h0064 <= data_reg_2);
assign when_MyTopLevel_l41_3 = (16'h0064 <= data_reg_3);
assign larger_than_N = larger_than_N_reg;
always @(posedge clk or posedge reset) begin
if(reset) begin
larger_than_N_reg <= 8'h0;
data_reg_0 <= 16'h0;
data_reg_1 <= 16'h0;
data_reg_2 <= 16'h0;
data_reg_3 <= 16'h0;
end else begin
if(io_latch_enable) begin
data_reg_0 <= io_input_data_0;
data_reg_1 <= io_input_data_1;
data_reg_2 <= io_input_data_2;
data_reg_3 <= io_input_data_3;
larger_than_N_reg <= 8'h0; // Should use blocking assign!!!
if(when_MyTopLevel_l41) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_1) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_2) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_3) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
end
end
end
endmodule
Hi,
This is the expected behaviour, SpinalHDL kind of follow more a VHDL way of handling such things :
class MyTopLevel extends Component {
val io = new Bundle {
val latch_enable = in Bool
val input_data = in Vec(UInt(16 bits), 4)
}
val larger_than_N = out UInt(8 bits)
var larger_than_N_var = UInt(8 bits) //This is a scala var
val larger_than_N_reg = Reg(UInt(8 bits)) init(0)
val data_reg = Vec(Reg(UInt(16 bits)) init(0), 4)
when(io.latch_enable){
data_reg := io.input_data
larger_than_N_reg := 0
for(idx <- 0 to 3) {
when (data_reg(idx) >= 100) {
larger_than_N_var \= larger_than_N_var + 1 //Changed operator
}
}
}
larger_than_N := larger_than_N_reg
larger_than_N := larger_than_N_var
}
Basicaly, the idea is to build the full graph of hardware "nodes" by creating new UInt instances (larger_than_N_var + 1) and refering to it later on the next iteration. I guess this should work as you expected. It will create a new signal at each iteration for larger_than_N_var
Ok, but wouldn't it be nice to hint the user with an error message, that the generated code is potentially faulty? -- Or determine the kind of operator used from the context?
In some cases, codes structure like :
for(idx <- 0 to 3) {
when (data_reg(idx) >= 100) {
larger_than_N_reg := ???
}
}
Isn't a faulty one, but the intended one. So far, SpinalHDL consider that multiple assignement to the same variable in the same conditional scope (fully overriding the previous value) is a faulty thing, but as soon there is some when statement, between the assignement, the described behaviour may make sense.
So, i would say, you code, even if it didn't implemented the behaviour you wanted, was still something legal to do.
Yeah, but only, if and only if there is exactly one condition in the loop, where the when-condition is true. Then, I would suggest to add some kind of check for that, as for instance a piece of verilog-code that executes the check and triggers an assertion if multiple assigns would occur. Also, this could be formally proved by e.g. yosys to get rid of possible mismatches. A synthesizer would ignore this code anyway so it would not harm the compilation result.
I'm sorry, can you ilustrate an example of what should be catched ? I'm not sure i understand properly.
Just as an example, I use the code from above and insert some code & comments (sorry for the bad indent, i quickly wrote a mockup ;)):
module MyTopLevel (
input io_latch_enable,
input [15:0] io_input_data_0,
input [15:0] io_input_data_1,
input [15:0] io_input_data_2,
input [15:0] io_input_data_3,
output [7:0] larger_than_N,
input clk,
input reset
);
reg [7:0] larger_than_N_reg;
reg [15:0] data_reg_0;
reg [15:0] data_reg_1;
reg [15:0] data_reg_2;
reg [15:0] data_reg_3;
wire when_MyTopLevel_l41;
wire when_MyTopLevel_l41_1;
wire when_MyTopLevel_l41_2;
wire when_MyTopLevel_l41_3;
assign when_MyTopLevel_l41 = (16'h0064 <= data_reg_0);
assign when_MyTopLevel_l41_1 = (16'h0064 <= data_reg_1);
assign when_MyTopLevel_l41_2 = (16'h0064 <= data_reg_2);
assign when_MyTopLevel_l41_3 = (16'h0064 <= data_reg_3);
assign larger_than_N = larger_than_N_reg;
always @(posedge clk or posedge reset) begin
if(reset) begin
larger_than_N_reg <= 8'h0;
data_reg_0 <= 16'h0;
data_reg_1 <= 16'h0;
data_reg_2 <= 16'h0;
data_reg_3 <= 16'h0;
end else begin
if(io_latch_enable) begin
data_reg_0 <= io_input_data_0;
data_reg_1 <= io_input_data_1;
data_reg_2 <= io_input_data_2;
data_reg_3 <= io_input_data_3;
larger_than_N_reg <= 8'h0; // Should use blocking assign!!!
if(when_MyTopLevel_l41) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_1) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_2) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
if(when_MyTopLevel_l41_3) begin
larger_than_N_reg <= (larger_than_N_reg + 8'h01); // Should use blocking assign!!!
end
end
end
end
// Checker code:
wire when_MyTopLevel_l41_wires = {when_MyTopLevel_l41, when_MyTopLevel_l41_1, when_MyTopLevel_l41_2, when_MyTopLevel_l41_3};
int number_of_active_branches;
always @(posedge clk )
begin
number_of_active_branches = 0;
for (i=0;i<4;i=i+1)
begin
if (when_MyTopLevel_l41_wires[i])
begin
number_of_active_branches = number_of_active_branches + 1;
end
end
if( number_of_active_branches > 1)
begin
// Could also be realized as assertion, maybe put a link to this issue or corresponding doc
$display("More than one branch is active, this might result in a mismatich! Please check the operators in your code");
end
end
end
endmodule
Ahhhhhhhhhhh
So, do you mean you would like a addition in the spinal.lib, to have a thing which check of more than 1 bit is set in a array of bits ?
If yes, sure, it can be done ^^
Yes, I think that would at least help a newcommer like me to get the thing right
Hooo, seems like there is already : spinal.lib.CountOne(xxx), i guess this is what you wanted right ?