Skip to content

[HW] Series fix for corner case bugs #377

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
wayne-win wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[HW] Series fix for corner case bugs #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fc3ef2b
fix the mask operand access wrong address #243
wayne-win Nov 26, 2024
2fa69f1
Prevent extra lane operations caused by unbalanced workloads
wayne-win Nov 26, 2024
900abfd
Update CHANGELOG.md
wayne-win Nov 26, 2024
cc8f423
Merge branch 'main' of github.com:wayne-win/ara-fix into main
wayne-win Nov 26, 2024
3a5bad0
Illegal instructions should not trigger unnecessary reshuffles #276
wayne-win Nov 26, 2024
f78c3a7
Push operand request only when vl not zeor, #259
wayne-win Nov 27, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Bump upload and delete artifact actions
- Fix synthesis-unfriendly constructs
- Fix vector slicing bug in operand requesters
- Fix the mask_operand_requester access wrong address considergin vstart

### Added

Expand Down
2 changes: 1 addition & 1 deletion hardware/src/ara_dispatcher.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3272,7 +3272,7 @@ module ara_dispatcher import ara_pkg::*; import rvv_pkg::*; #(

// Check if we need to reshuffle our vector registers involved in the operation
// This operation is costly when occurs, so avoid it if possible
if ( ara_req_valid && !acc_resp_o.exception.valid ) begin
if ( ara_req_valid && !acc_resp_o.exception.valid && !illegal_insn) begin
automatic rvv_instruction_t insn = rvv_instruction_t'(acc_req_i.insn.instr);

// Is the instruction an in-lane one and could it be subject to reshuffling?
Expand Down
46 changes: 23 additions & 23 deletions hardware/src/lane/lane_sequencer.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
target_fu : ALU_SLDU,
default : '0
};
operand_request_push[AluA] = pe_req.use_vs1;
operand_request_push[AluA] = pe_req.use_vs1 && vfu_operation_valid_d;

operand_request[AluB] = '{
id : pe_req.id,
Expand All @@ -331,7 +331,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
target_fu : ALU_SLDU,
default : '0
};
operand_request_push[AluB] = pe_req.use_vs2;
operand_request_push[AluB] = pe_req.use_vs2 && vfu_operation_valid_d;

// This vector instruction uses masks
operand_request[MaskM] = '{
Expand All @@ -342,13 +342,13 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// Since this request goes outside of the lane, we might need to request an
// extra operand regardless of whether it is valid in this lane or not.
vl : (pe_req.vl / NrLanes / 8) >> unsigned'(pe_req.vtype.vsew),
vstart : vfu_operation_d.vstart,
vl : (pe_req.vl / NrLanes / 8) >> int'(pe_req.vtype.vsew),
hazard : pe_req.hazard_vm | pe_req.hazard_vd,
default: '0
};
if ((operand_request[MaskM].vl << unsigned'(pe_req.vtype.vsew)) *
NrLanes * 8 != pe_req.vl) operand_request[MaskM].vl += 1;
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;
end
VFU_MFpu: begin
operand_request[MulFPUA] = '{
Expand All @@ -369,7 +369,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
target_fu : MFPU_ADDRGEN,
default : '0
};
operand_request_push[MulFPUA] = pe_req.use_vs1;
operand_request_push[MulFPUA] = pe_req.use_vs1 && vfu_operation_valid_d;

operand_request[MulFPUB] = '{
id : pe_req.id,
Expand All @@ -391,7 +391,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
target_fu : MFPU_ADDRGEN,
default: '0
};
operand_request_push[MulFPUB] = pe_req.swap_vs2_vd_op ?
operand_request_push[MulFPUB] = !vfu_operation_valid_d ? 1'b0 : pe_req.swap_vs2_vd_op ?
pe_req.use_vd_op : pe_req.use_vs2;

operand_request[MulFPUC] = '{
Expand All @@ -413,7 +413,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
target_fu : MFPU_ADDRGEN,
default : '0
};
operand_request_push[MulFPUC] = pe_req.swap_vs2_vd_op ?
operand_request_push[MulFPUC] = !vfu_operation_valid_d ? 1'b0 : pe_req.swap_vs2_vd_op ?
pe_req.use_vs2 : pe_req.use_vd_op;

// This vector instruction uses masks
Expand All @@ -425,13 +425,13 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// Since this request goes outside of the lane, we might need to request an
// extra operand regardless of whether it is valid in this lane or not.
vl : (pe_req.vl / NrLanes / 8) >> unsigned'(pe_req.vtype.vsew),
vstart : vfu_operation_d.vstart,
vstart : (pe_req.vstart / NrLanes / 8) >> int'(pe_req.vtype.vsew),
hazard : pe_req.hazard_vm | pe_req.hazard_vd,
default: '0
};
if ((operand_request[MaskM].vl << unsigned'(pe_req.vtype.vsew)) *
NrLanes * 8 != pe_req.vl) operand_request[MaskM].vl += 1;
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;
end
VFU_LoadUnit : begin
// This vector instruction uses masks
Expand All @@ -443,13 +443,13 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// Since this request goes outside of the lane, we might need to request an
// extra operand regardless of whether it is valid in this lane or not.
vl : (pe_req.vl / NrLanes / 8) >> unsigned'(pe_req.vtype.vsew),
vstart : vfu_operation_d.vstart,
vstart : (pe_req.vstart / NrLanes / 8) >> int'(pe_req.vtype.vsew),
hazard : pe_req.hazard_vm | pe_req.hazard_vd,
default: '0
};
if ((operand_request[MaskM].vl << unsigned'(pe_req.vtype.vsew)) *
NrLanes * 8 != pe_req.vl) operand_request[MaskM].vl += 1;
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;

// Load indexed
operand_request[SlideAddrGenA] = '{
Expand All @@ -469,7 +469,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// extra operand regardless of whether it is valid in this lane or not.
if (operand_request[SlideAddrGenA].vl * NrLanes != pe_req_i.vl)
operand_request[SlideAddrGenA].vl += 1;
operand_request_push[SlideAddrGenA] = pe_req_i.op == VLXE;
operand_request_push[SlideAddrGenA] = pe_req_i.op == VLXE && vfu_operation_valid_d;
end

VFU_StoreUnit : begin
Expand Down Expand Up @@ -504,13 +504,13 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// Since this request goes outside of the lane, we might need to request an
// extra operand regardless of whether it is valid in this lane or not.
vl : (pe_req.vl / NrLanes / 8) >> unsigned'(pe_req.vtype.vsew),
vstart : vfu_operation_d.vstart,
vstart : (pe_req.vstart / NrLanes / 8) >> int'(pe_req.vtype.vsew),
hazard : pe_req.hazard_vm | pe_req.hazard_vd,
default: '0
};
if ((operand_request[MaskM].vl << unsigned'(pe_req.vtype.vsew)) *
NrLanes * 8 != pe_req.vl) operand_request[MaskM].vl += 1;
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;

// Store indexed
// TODO: add vstart support here
Expand All @@ -532,7 +532,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
if (operand_request[SlideAddrGenA].vl * NrLanes != pe_req_i.vl) begin
operand_request[SlideAddrGenA].vl += 1;
end
operand_request_push[SlideAddrGenA] = pe_req_i.op == VSXE;
operand_request_push[SlideAddrGenA] = pe_req_i.op == VSXE && vfu_operation_valid_d;
end

VFU_SlideUnit: begin
Expand All @@ -549,7 +549,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
hazard : pe_req.hazard_vs2 | pe_req.hazard_vd,
default : '0
};
operand_request_push[SlideAddrGenA] = pe_req.use_vs2;
operand_request_push[SlideAddrGenA] = pe_req.use_vs2 && vfu_operation_valid_d;

unique case (pe_req.op)
VSLIDEUP: begin
Expand Down Expand Up @@ -608,7 +608,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
hazard : pe_req.hazard_vm | pe_req.hazard_vd,
default : '0
};
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;

case (pe_req.op)
VSLIDEUP: begin
Expand Down Expand Up @@ -667,7 +667,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
if ((operand_request[AluA].vl << (unsigned'(EW64) - unsigned'(pe_req.eew_vs1))) * NrLanes !=
pe_req.vl) operand_request[AluA].vl += 1;
end
operand_request_push[AluA] = pe_req.use_vs1 && !(pe_req.op inside {[VMFEQ:VMFGE], VCPOP, VMSIF, VMSOF, VMSBF});
operand_request_push[AluA] = pe_req.use_vs1 && !(pe_req.op inside {[VMFEQ:VMFGE], VCPOP, VMSIF, VMSOF, VMSBF}) && vfu_operation_valid_d;

operand_request[AluB] = '{
id : pe_req.id,
Expand All @@ -694,7 +694,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
if ((operand_request[AluB].vl << (unsigned'(EW64) - unsigned'(pe_req.eew_vs2))) * NrLanes !=
pe_req.vl) operand_request[AluB].vl += 1;
end
operand_request_push[AluB] = pe_req.use_vs2 && !(pe_req.op inside {[VMFEQ:VMFGE], VCPOP, VMSIF, VMSOF, VMSBF, VFIRST});
operand_request_push[AluB] = pe_req.use_vs2 && !(pe_req.op inside {[VMFEQ:VMFGE], VCPOP, VMSIF, VMSOF, VMSBF, VFIRST}) && vfu_operation_valid_d;

operand_request[MulFPUA] = '{
id : pe_req.id,
Expand All @@ -710,7 +710,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// This is an operation that runs normally on the ALU, and then gets *condensed* and
// reshuffled at the Mask Unit.
operand_request[MulFPUA].vl = vfu_operation_d.vl;
operand_request_push[MulFPUA] = pe_req.use_vs1 && pe_req.op inside {[VMFEQ:VMFGE]} && !(pe_req.op inside {VCPOP, VMSIF, VMSOF, VMSBF});
operand_request_push[MulFPUA] = pe_req.use_vs1 && pe_req.op inside {[VMFEQ:VMFGE]} && !(pe_req.op inside {VCPOP, VMSIF, VMSOF, VMSBF}) && vfu_operation_valid_d;

operand_request[MulFPUB] = '{
id : pe_req.id,
Expand Down Expand Up @@ -744,7 +744,7 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
if ((pe_req.vl % (NrLanes*ELEN)) != 0) begin
operand_request[MaskB].vl += 1'b1;
end
operand_request_push[MaskB] = pe_req.use_vs2 && pe_req.op inside {VCPOP, VFIRST, VMSIF, VMSOF, VMSBF};
operand_request_push[MaskB] = pe_req.use_vs2 && pe_req.op inside {VCPOP, VFIRST, VMSIF, VMSOF, VMSBF} && vfu_operation_valid_d;

operand_request[MaskM] = '{
id : pe_req.id,
Expand All @@ -754,14 +754,14 @@ module lane_sequencer import ara_pkg::*; import rvv_pkg::*; import cf_math_pkg::
// Since this request goes outside of the lane, we might need to request an
// extra operand regardless of whether it is valid in this lane or not.
vl : (pe_req.vl / NrLanes / ELEN),
vstart : vfu_operation_d.vstart,
vstart : (pe_req.vstart / NrLanes / ELEN),
hazard : pe_req.hazard_vm,
default: '0
};
if ((operand_request[MaskM].vl * NrLanes * ELEN) != pe_req.vl) begin
operand_request[MaskM].vl += 1;
end
operand_request_push[MaskM] = !pe_req.vm;
operand_request_push[MaskM] = !pe_req.vm && vfu_operation_valid_d;
end
VFU_None: begin
operand_request[MaskB] = '{
Expand Down
2 changes: 1 addition & 1 deletion hardware/src/lane/operand_requester.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ module operand_requester import ara_pkg::*; import rvv_pkg::*; #(


// Mute the requisition if the vl is zero
if (operand_request_i[requester_index].vl == '0) begin : zero_vl
if (operand_request_i[requester_index].vl == '0 || | (operand_request_i[requester].vl == operand_request_i[requester].vstart)) begin : zero_vl
state_d = IDLE;
operand_queue_cmd_valid_o[requester_index] = 1'b0;
end : zero_vl
Expand Down