Fix rvp by chihminchao · Pull Request #2310 · riscv-software-src/riscv-isa-sim

chihminchao · 2026-06-03T09:30:26Z

Fix implementation by riscv-software-src/riscv-tests#648

aswaterman

This is moving in the right direction. Thanks!

I will do a second round of review once all of the unusual shift instructions are rewritten how I suggested. It's much easier to spot the bugs with fewer special cases.

chihminchao · 2026-06-08T16:01:56Z

Extra change

I have apply the std::min for other sshl/sshlr instructions
replace magic constants by standard UINTXX_T/INTXX_T definition.

aswaterman · 2026-06-09T10:20:40Z

+  WRITE_RD((RS1 & 0x80000000) ? sext32(0xffffffff) : 0);
 else
-  WRITE_RD(sshamt >= 0 ? P_SAT(32, static_cast<sreg_t> (RS1) << sshamt) : (RS1 >> -sshamt));
+  WRITE_RD(sshamt >= 0 ? P_SAT(32, static_cast<sreg_t> (RS1) << sshamt) : (RS1 >> -sshamt));


This looks incorrect: it needs an sext32, too.

I recommend only having the WRITE_RD(sext32(result)) in one place, with the result being written in each of the if-statement cases. And please do the same thing in all of the instructions.

follow the suggestion to simplify writing path

aswaterman · 2026-06-09T10:26:58Z

+    WRITE_RD(sext32((((RS1 * RS2) >> 30) + 1) >> 1));
  } else {
    WRITE_RD(INT32_MAX);
    P.set_vxsat();


Similar comment as https://github.com/riscv-software-src/riscv-isa-sim/pull/2310/changes#r3379857166

aswaterman

Looks much better, thanks @chihminchao. Ping me once my remaining issues are resolved and I'll merge it.

Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>

also refine writing path to RD Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>

Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>

Corrects shift amount for extracting bits [47:16] from 48-bit multiply-accumulate products. Implementation used 32-bit shift but spec requires 16-bit shift to extract high portion. Fixed instructions: - MHACC.H0: P-ext spec - MHACC.H1: P-ext spec - MHACCSU.H0: P-ext spec - MHACCSU.H1: P-ext spec Changed: mres >> 32 → mres >> 16 Signed-off-by: Max Chou <max.chou@sifive.com>

Corrects swapped add/subtract operators in packed 2-way multiply loop. Spec requires (p0 - p1) but implementation computed (p1 - p0). Even elements should add, odd elements should subtract. Fixed instructions: - PM2WSUB.H: P-ext spec - PM2WSUBA.H: P-ext spec Changed: Swapped += and -= for (j & 1) conditional Signed-off-by: Max Chou <max.chou@sifive.com>

Use 5-bit mask (0x1F) for shift amount instead of 8-bit field extraction. Spec defines rs2[4:0] as shift amount. Also removes incorrect >= 32 check that zeroed result. Fixed instruction: - PSLL.DWS: P-ext spec Changed: P_FIELD(RS2, 0, 8) with >= 32 check → RS2 & 0x1F Signed-off-by: Max Chou <max.chou@sifive.com>

Signed-off-by: Chih-Min Chao <chihmin.chao@sifive.com>

chihminchao · 2026-07-06T14:55:20Z

Update: 2026/07/06
1 simplify writing path to RD
2.fix more signed-extension issues

chihminchao force-pushed the fix-rvp branch from dcb462b to 3b3ba2c Compare June 4, 2026 08:46