In Verilog, counting and outputting the number of 1's in an 8bit input?

Question

What I am trying to do in my mind is take 8 1-bit inputs and count the 1's. Then represent those 1's.

01010111 should output 0101 (There are five 1's from input)

module 8to4 (in,out,hold,clk,reset);
input [7:0] in; //1 bit inputs
reg [7:0] hold; //possible use for case statement
output [3:0] out; //Shows the count of bits

always @(clk)
  begin
    out = in[0] + in[1] + in[2] + in[3] + in[4] + in[5] + in[6] + in[7]; //Adds the inputs from testbench and outputs it
  end
endmodule

Questions:

Is that the proper way to have 8 1-bit inputs? Or do I need to declare each variable as one bit ex: input A,B,C,D,E,F,G,H;
If my above code is close to being correct, is that the proper way to get out to display the count of 1's? Would I need a case statement?

I'm really new to verilog, so I don't even want to think about a test bench yet.

dave_59 · Accepted Answer · 2017-10-13T14:03:45.140

0

The way you wrote it is probably the better way of writing it because it makes it easier to parameterize the number of bits. But technically, you have one 8-bit input.

module 8to4 #(parameter WIDTH=8) (input [WIDTH-1:0] in, 
                        output reg [3:0] out,hold,
                        input clk,reset);
  reg [WIDTH-1:0] temp;
  integer ii;
  always @(clk)
    begin
      temp = 0;
      for(ii=0; ii<WIDTH; i = i + 1)   
         temp = temp + in[ii];
      out <= temp;
    end
endmodule

edited Oct 13 '17 at 14:03

answered Oct 13 '17 at 05:56

dave_59

39,096
3
24
63

Sorry, I'm still new to this. Could you possibly explain `#(WIDTH-8)`? Also after a bunch of googling `integer ii;` Honestly I didn't know that Verilog could make ints like that and I wouldn't have a clue how to write a testbench for it. – Link Page Oct 13 '17 at 13:36
Sorry, that was a typo. I was parameterizing your module so that it could be used for different sizes of inputs. When WIDTH=8, the `for` loop does the exact same thing as your original expression. Synthesis tools will unroll my loop into your expression. But now that it is parameterized, you could override `WIDTH` to 16 or whatever. – dave_59 Oct 13 '17 at 14:08

score 0 · Answer 2 · answered Oct 14 '17 at 01:23

Logically the code is proper.

However you can improve it like the following.

Make out as a reg, because you are using it in a procedural assignment.
Usage of reset. Ideally any code should have reset state, which is missing in your code.
Declare the direction (input/output) for hold, clk & reset port, which is currently not specified.
As dave mentioned, you can use parameters for your code.

In Verilog, counting and outputting the number of 1's in an 8bit input?

2 Answers2