BUG-008: BLT/BGE/BLTU/BGEU branch on the wrong condition

Field	Value
Severity	CRITICAL
Component	RTL
File(s)	rtl/riscv_core.sv
Status	Fixed
Date	2026-07-23

Symptom

No existing test caught this — the standard regression suite (cpu_peak_tests.asm) and the only other control-flow test in the repo (test_rv32m_control_flow_regression.asm, not wired into the suite — see BUG-010) only exercise JAL and BNE. Found while building a new regression (tb/branch_regression.s) specifically to exercise BLT/BGE/BLTU/BGEU, none of which had ever been tested: s3 came back 12 instead of the expected 15, meaning the BLT and BGE branches silently failed to take when they should have.

Root Cause

OP_BRANCH decode set id_alu_ctrl = ALU_SUB unconditionally, regardless of funct3:

OP_BRANCH: begin id_is_branch=1; id_alu_ctrl=ALU_SUB; end

Branch resolution in EX then read ex_alu_result[0] for BLT/BLTU (and its inverse for BGE/BGEU) as if it were a comparison result. But with alu_ctrl fixed at ALU_SUB, ex_alu_result is a - b, and bit 0 of a 32-bit subtraction is just the parity of the difference — completely unrelated to whether a < b. Only BEQ/BNE were actually correct, since they only need the zero flag (a - b == 0), which subtraction does compute correctly.

Fix

Select the ALU op per funct3, matching what each branch actually needs:

- OP_BRANCH: begin id_is_branch=1; id_alu_ctrl=ALU_SUB; end
+ OP_BRANCH: begin
+     id_is_branch=1;
+     case (id_funct3)
+         3'h4, 3'h5: id_alu_ctrl = ALU_SLT;   // BLT/BGE  — signed compare
+         3'h6, 3'h7: id_alu_ctrl = ALU_SLTU;  // BLTU/BGEU — unsigned compare
+         default:    id_alu_ctrl = ALU_SUB;   // BEQ/BNE  — needs zero flag
+     endcase
+ end

and simplified branch resolution accordingly (no longer needs to OR in zero to compensate for using the wrong ALU op):

- 3'h4: branch_taken = ex_alu_result[0];
- 3'h5: branch_taken = !ex_alu_result[0] | ex_zero;
- 3'h6: branch_taken = ex_alu_result[0];
- 3'h7: branch_taken = !ex_alu_result[0] | ex_zero;
+ 3'h4: branch_taken = ex_alu_result[0];      // BLT  (SLT result)
+ 3'h5: branch_taken = !ex_alu_result[0];     // BGE  (!SLT)
+ 3'h6: branch_taken = ex_alu_result[0];      // BLTU (SLTU result)
+ 3'h7: branch_taken = !ex_alu_result[0];     // BGEU (!SLTU)

Notes

tb/branch_regression.s deliberately uses operands where signed and unsigned comparisons disagree (s4 = -1 = 0xFFFFFFFF, s5 = 1) so that a BLTU/BGEU implementation that was accidentally doing a signed compare (or vice versa) would also be caught, not just the SUB-vs-SLT confusion above.

Fixing this exposed a second, more significant bug in the same code path — see BUG-009 — found while verifying this fix against the new regression test.