PeakRDL-regblock
PeakRDL-regblock copied to clipboard
Saturating counters perform inequality check twice in generated logic
For this counter description:
signal {
sync;
activehigh;
} latch_stats;
reg stats_cnt_reg_varincr #(
string regdesc = "",
string fielddesc = "",
longint regwidth = 32,
longint fieldwidth = 32,
longint incrwidth = 11,
boolean saturate = true
) {
desc = regdesc;
regwidth = regwidth;
buffer_reads = true;
rbuffer_trigger = latch_stats;
field {
desc = fielddesc;
hwclr = latch_stats;
sw = r;
hw = na;
counter;
incrwidth = incrwidth;
saturate = saturate;
} cnt[fieldwidth-1:0];
};
regfile some_regfile {
stats_cnt_reg_varincr #(
.fieldwidth(16)
) bytes_dropped @ 0x38;
}
PeakRDL-regblock geneates this block of code:
// Field: eth_ingress_csr.glbl.bytes_dropped.cnt
always_comb begin
automatic logic [15:0] next_c;
automatic logic load_next_c;
next_c = field_storage.glbl.bytes_dropped.cnt.value;
load_next_c = '0;
if(latch_stats) begin // HW Clear
next_c = '0;
load_next_c = '1;
end
if(hwif_in.glbl.bytes_dropped.cnt.incr) begin // increment
if(((17)'(next_c) + hwif_in.glbl.bytes_dropped.cnt.incrvalue) > 16'hffff) begin // up-counter saturated
next_c = 16'hffff;
end else begin
next_c = next_c + hwif_in.glbl.bytes_dropped.cnt.incrvalue;
end
load_next_c = '1;
end
field_combo.glbl.bytes_dropped.cnt.incrthreshold = (field_storage.glbl.bytes_dropped.cnt.value >= 16'hffff);
field_combo.glbl.bytes_dropped.cnt.incrsaturate = (field_storage.glbl.bytes_dropped.cnt.value >= 16'hffff);
if(next_c > 16'hffff) begin
next_c = 16'hffff;
load_next_c = '1;
end
field_combo.glbl.bytes_dropped.cnt.next = next_c;
field_combo.glbl.bytes_dropped.cnt.load_next = load_next_c;
end
The comparison if(next_c > 16'hffff)
is unnecessary. It's tautological too, so maybe synthesis tools would be able to optimize it away (ie, a 16 bit value can never be larger than 16'hffff).
This throws a linter error in Verilator which says something like "this comparison will always be false". I'm imagining the generated code is an artifact of generalized overflow checking, if for example the limit was less than 2**N-1 and we had some other way to update the value like sw = w
or hw = w
. But, as it is now, the sum is already checked for overflow about 9-10 lines above, and then it's checked a second time.
What do you think? If you think synthesis tools would be able to optimize this away, then I guess no harm in leaving it. But I wanted to bring it up since those inequality comparisons can get expensive for saturating logic, particularly with wider fields, so it's desirable not to do all of them twice!