riscv / sail-riscv

Sail RISC-V model
https://lists.riscv.org/g/tech-golden-model
Other
433 stars 159 forks source link

Refactor MUL instruction #466

Closed Alasdair closed 4 months ago

Alasdair commented 4 months ago

It was brought to my attention that this instruction had a bit of a case of the 'boolean blindness' code smell, where the mul operation was represented as a triple of booleans. This commit refactors the implemention to use a struct with named fields for high, signed1, and signed2.

The C_MUL instruction in Zcb also needs to be changed appropriately

The mul_op struct was added in riscv_types

While I was there I did some light housekeeping w.r.t a comment about a workaround for Sail < 0.15.1, as this is no longer needed. I could split this into a separate commit, but not sure if it's worthwhile to do so.

github-actions[bot] commented 4 months ago

Test Results

712 tests  ±0   712 :white_check_mark: ±0   0s :stopwatch: ±0s   6 suites ±0     0 :zzz: ±0    1 files   ±0     0 :x: ±0 

Results for commit f852d653. ± Comparison against base commit 82acf052.

:recycle: This comment has been updated with latest results.

Alasdair commented 4 months ago

I made the suggested change so signed1 is now signed_rs1 and likewise for signed2, as well as rebased to the latest commit.