Calculating rolling sum of array

Question

I am trying to implement a rolling average of an array of 12 bit samples in SystemVerilog. New samples are generated and shift into an array via a clocked flip flop. The goal is to have a register that represents the sum of all of the current samples, so I can then take the average with a right-shift.

enter image description here

I've attempted to implement this with the following mix of sequential logic to update the array of data, and combinational logic to total all the array elements. Note that the number of measurements to average will eventually be more like 128-1024:

localparam numMeasurementsToAverage = 8'd4;
reg [11:0] voltages [numMeasurementsToAverage:0];
// shift in latest voltage measurement on rising clock
always_ff @ (posedge clk)
 if(~rst_n) begin
 voltages <= '{default:11'd0};
 end
 else begin 
 voltages <= {voltages[numMeasurementsToAverage-1:0], v2p_out_voltage};
 end
// calculate the current sum of voltages array
always_comb
 if(~rst_n) begin
 sumOfVoltages <= 0;
 end
 else begin
 for (int i = 0; i < numMeasurementsToAverage; i=i+1) begin
 sumOfVoltages += voltages[i];
 end
 end

This runs, but the sumOfVoltages keeps increasing... the new array total is added to the array total calculated on the previous clock edge. I need the sumOfVoltages to only reflect the total of the elements currently in the array.

Because the voltages[] array is updating on the clock, the sum should also only update on the clock edge, so I can see an argument for moving the summing logic to a clocked flip flop. However, I don't think I can use a for loop for sequential logic.

I like the for loop approach because it makes it way easier to modify the number of elements in the array, compared to hardcoding addition operators between each element in the array. Is there a shorthand for summing all values within an array, using sequential logic instead of a for loop?

Or alternately, is there a clean way to reset sumOfVoltages to zero on each clock edge, without creating a race condition between the sequential and combinational logic? The target is a Xilinx Zynq FPGA, so I need synthesizable code, not just test bench.

• TimWescott: This looks like you're working your way up to a CIC filter -- if so, there's a lot of literature out there on how to do it.

Answer 1

Your approach is extremely wasteful of resources. You don't need to keep adding up the numbers in the middle of the FIFO. Instead, you add the numbers to an accumulator as they go in, and you subtract them back out of the accumulator when they come out of the FIFO.

enter image description here

This requires just two adders, regardless of the amount of data, and a dual-port block RAM can be used for the FIFO, since you no longer need to access all of the data in it on every clock cycle.

Answer 2

I think your reset technique is the problem. When you hit reset you clear both the voltage array and the voltage sum. Then, as you shift voltages in they get added to the voltage sum at every clock, even as they are being shifted in. You are also using non-blocking assignments in your combinational adder, which doesn't make sense.

I would suggest setting sumOfVoltages = voltages[0] (blocking assignment, it's combinational logic) and then having your for loop start at i = 1.

I don't see any need to reset the array of voltages, the sum will not be valid until you fill the array anyway.

Answer 3

I elaborated your code snippet in Vivado to generate a schematic. By using the sumOfVoltage in the loop you are causing the output to be used again on the next iteration. You can see the feedback into sumOfVoltage. This explains what you are seeing. Also note the latch because of the reset in the combinatorial block.

feedback into sumOfVoltage

I updated your code. I believe this is what you want. Please note, that serially adding this way can be quite slow and you should consider putting the adder in a synchronous block. Also, if you do not really need pure average you can think of running average as Dave Tweed indicated.

without feedback Here is the updated code.

module rollingsum 
(input logic clk, input logic rst_n, input logic [11:0] v2p_out_voltage,
 output logic [13:0] sumOut
);
localparam numMeasurementsToAverage = 8'd4;
reg [11:0] voltages [numMeasurementsToAverage:0] ;
logic [13:0] sumOfVoltages;
// shift in latest voltage measurement on rising clock
always_ff @ (posedge clk)
 if(~rst_n) begin
 voltages <= '{default:11'd0};
 end
 else begin 
 voltages <= {voltages[numMeasurementsToAverage-1:0], v2p_out_voltage};
 end
// calculate the current sum of voltages array
logic [13:0] vsum [numMeasurementsToAverage:0];
always_comb
/*
 if(~rst_n) begin
 sumOfVoltages <= 0;
 end
 else */ begin
 for (int i = 0; i < numMeasurementsToAverage; i=i+1) begin
 //sumOfVoltages += voltages[i];
 vsum[i+1] = vsum[i] + voltages[i];
 end
 end
assign sumOut = vsum[numMeasurementsToAverage];//sumOfVoltages;
endmodule