search

kuznia-rdzeni / coreblocks

RISC-V out-of-order core for education and research purposes
https://kuznia-rdzeni.github.io/coreblocks/
BSD 3-Clause "New" or "Revised" License
33 stars 13 forks source link

Fix fetch resume #681

Closed piotro888 closed 1 month ago

piotro888 commented 2 months ago

Fixes bug found in #654 Splits fetch.resume to separate resume_from_exception and resume_from_unsafe

I tried adding formal-like verification constraints, to verify method ordering assumptions from the comments, but have mixed feelings on the final solution. What do you think?

github-actions[bot] commented 2 months ago

Benchmarks summary

Performance benchmarks

aha-mont64 crc32 minver nettle-sha256 nsichneu slre statemate ud
0.407 (0.000) 0.527 (0.000) 0.321 (0.000) 0.652 (0.000) 0.345 (0.000) 0.283 (0.000) 0.317 (0.000) 0.405 (0.000)

You can view all the metrics here.

Synthesis benchmarks (basic)

Device utilisation: (ECP5) LUTs used as DFF: (ECP5) LUTs used as carry: (ECP5) LUTs used as ram: (ECP5) Max clock frequency (Fmax)
▼ 22298 (-351) 5560 (0) ▼ 770 (-32) 1004 (0) ▲ 51 (+5)

Synthesis benchmarks (full)

Device utilisation: (ECP5) LUTs used as DFF: (ECP5) LUTs used as carry: (ECP5) LUTs used as ram: (ECP5) Max clock frequency (Fmax)
▲ 32690 (+2219) 8802 (0) 1970 (0) 1184 (0) ▲ 43 (+0)
tilk commented 2 months ago

I prefer a failed assertion to a weird, hard to debug behavior. Why not.

xThaid commented 2 months ago

I agree that having verification code is a good idea. Perhaps we could make another module (something like FetchVerification) that contains that logic to avoid polluting the main module, but then we need to expose some signals, so it is not perfect. Another option would be to create a function within the main module that is called from elaborate and contains the verification logic.