Skip to content

[hardware] 🐛 Fix two vfirst.m defects in the mask unit (#500) #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
renzoandri wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[hardware] 🐛 Fix two vfirst.m defects in the mask unit (#500) #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b6df1b
[hardware] Fix vfirst.m index accumulation over all-zero mask slices
renzoandri Sep 3, 2026
16edd3b
[hardware] Run vfirst.m to completion so it drains its MASKU operands
renzoandri Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Fixed

- Run `vfirst.m` to completion so it drains its MASKU operands, instead of aborting on the first set bit
- Fix `vfirst.m` index under-reporting by `floor(idx/VfirstParallelism)` on all-zero mask slices
- Fix dump vtrace script for vsetvli instructions without x0 (ideal dispatcher)
- Fix Pathfinder and FFT performance
- Stall Ara and wait for ara_idle upon CSR write/read
Expand Down
49 changes: 43 additions & 6 deletions hardware/src/masku/masku.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
logic [$clog2(VfirstParallelism)-1:0] vfirst_count;
logic [$clog2(VLEN)-1:0] vfirst_count_d, vfirst_count_q;
logic vfirst_empty;
// VFIRST no longer terminates as soon as it finds the first set bit, so it
// needs to remember that it already found one.
logic vfirst_found_d, vfirst_found_q;
// counter to keep track of how many slices of the vcpop_operand have been processed
logic [VcpopParallelism-1:0] vcpop_slice;
logic [VfirstParallelism-1:0] vfirst_slice;
Expand Down Expand Up @@ -904,6 +907,7 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(

popcount_d = popcount_q;
vfirst_count_d = vfirst_count_q;
vfirst_found_d = vfirst_found_q;

mask_queue_d = mask_queue_q;
mask_queue_valid_d = mask_queue_valid_q;
Expand Down Expand Up @@ -1283,7 +1287,20 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
end
// Write the scalar accumulator
popcount_d = popcount_q + popcount;
vfirst_count_d = vfirst_count_q + vfirst_count;
// An all-zero slice must advance the index by VfirstParallelism.
// common_cells' lzc asserts empty_o and returns cnt_o = WIDTH-1 (not
// WIDTH) for an all-zero input (see its header comment), so the
// unconditional accumulate lost exactly one count per skipped slice and
// vfirst.m under-reported by floor(idx/VfirstParallelism): a set bit at
// index 16 was reported as 15, 100 as 94, 255 as 240.
// Stop accumulating once the first set bit has been seen: VFIRST no
// longer aborts the instruction at that point (see below), so the
// remaining slices must not move the index.
if (!vfirst_found_q) begin
vfirst_count_d = vfirst_empty ? vfirst_count_q + VfirstParallelism
: vfirst_count_q + vfirst_count;
vfirst_found_d = ~vfirst_empty;
end

// Bump MASKU ALU state
found_one_d = found_one;
Expand All @@ -1304,15 +1321,25 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
// Request new input (by completing ready-valid handshake) once all slices have been processed
// Alu input is accessed in different widths
// VRGATHER and VCOMPRESS handle the ALU operand for the index generation before the MASKU ALU gets the operands
if ((((in_ready_cnt_q == in_ready_threshold_q) || (issue_cnt_d == '0)) && !(vinsn_issue.op inside {[VRGATHER:VCOMPRESS]})) || (!vfirst_empty && (vinsn_issue.op == VFIRST))) begin
// The `|| (!vfirst_empty && op == VFIRST)` term used to acknowledge the
// ALU operand word here the moment VFIRST found its bit, and the
// companion term at the out_scalar_valid site below then ended the
// instruction -- leaving the operand words the lanes were still
// delivering for the rest of that vl stranded in the per-lane MASKU
// operand queue, where the next mask instruction consumed them as its
// own input. VFIRST now runs to completion like VCPOP, so the ack is
// driven by the slice threshold alone.
if ((((in_ready_cnt_q == in_ready_threshold_q) || (issue_cnt_d == '0)) && !(vinsn_issue.op inside {[VRGATHER:VCOMPRESS]}))) begin
in_ready_cnt_clr = 1'b1;
if (vinsn_issue.op != VID) begin
masku_operand_alu_ready = '1;
end
end
// Mask is always accessed at bit level
// VMADC, VMSBC handle masks in the mask queue
if ((((in_m_ready_cnt_q == in_m_ready_threshold_q) || (issue_cnt_d == '0)) && !(vinsn_issue.op inside {[VMADC:VMSBC]})) || (!vfirst_empty && (vinsn_issue.op == VFIRST))) begin
// Same VFIRST early-ack term removed here; this one stranded the *mask*
// operand of a masked vfirst.m.
if ((((in_m_ready_cnt_q == in_m_ready_threshold_q) || (issue_cnt_d == '0)) && !(vinsn_issue.op inside {[VMADC:VMSBC]}))) begin
in_m_ready_cnt_clr = 1'b1;
if (!vinsn_issue.vm) begin
masku_operand_m_ready = '1;
Expand All @@ -1336,8 +1363,11 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
end

// The scalar result is valid for write back at the end of the operation.
// VFIRST can also interrupt the operation in advance when the 1 is found.
if (issue_cnt_d == '0 || (!vfirst_empty && (vinsn_issue.op == VFIRST))) begin
// VFIRST used to interrupt the operation in advance when the 1 was
// found. It no longer does -- vfirst_found_q carries "the answer is
// already latched", so the remaining slices are consumed (and therefore
// acknowledged) instead of being left in the operand queue.
if (issue_cnt_d == '0) begin
// Assert valid scalar output
out_scalar_valid = vd_scalar(vinsn_issue.op);
end
Expand Down Expand Up @@ -1379,7 +1409,10 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(

// The scalar result has been sent to and acknowledged by the dispatcher
if (out_scalar_valid) begin
result_scalar_d = (vinsn_issue.op == VCPOP) ? popcount_d : ((vfirst_empty) ? -1 : vfirst_count_d);
// "No set bit anywhere in vl" is now vfirst_found_d, not "the slice we
// happened to stop on was empty" -- with the early exit gone, the last
// slice of a successful vfirst.m is normally empty.
result_scalar_d = (vinsn_issue.op == VCPOP) ? popcount_d : ((!vfirst_found_d) ? -1 : vfirst_count_d);
result_scalar_valid_d = '1;

// The instruction is over
Expand Down Expand Up @@ -1495,6 +1528,7 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
// Reset the popcount and vfirst_count
popcount_d = '0;
vfirst_count_d = '0;
vfirst_found_d = '0;
end

//////////////////////////////
Expand Down Expand Up @@ -1597,6 +1631,7 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
end
VFIRST: begin
vfirst_count_d = '0;
vfirst_found_d = '0;

// Mask to scalar
delta_elm_d = VfirstParallelism;
Expand Down Expand Up @@ -1646,6 +1681,7 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
result_final_gnt_q <= '0;
popcount_q <= '0;
vfirst_count_q <= '0;
vfirst_found_q <= '0;
delta_elm_q <= '0;
in_ready_threshold_q <= '0;
in_m_ready_threshold_q <= '0;
Expand All @@ -1670,6 +1706,7 @@ module masku import ara_pkg::*; import rvv_pkg::*; #(
result_final_gnt_q <= result_final_gnt_d;
popcount_q <= popcount_d;
vfirst_count_q <= vfirst_count_d;
vfirst_found_q <= vfirst_found_d;
delta_elm_q <= delta_elm_d;
in_ready_threshold_q <= in_ready_threshold_d;
in_m_ready_threshold_q <= in_m_ready_threshold_d;
Expand Down