riscv-software-src / riscv-perf-model

Example RISC-V Out-of-Order/Superscalar Processor Performance Core and MSS Model
Apache License 2.0
117 stars 43 forks source link

Fix to Support SYS Instructions with Register Rename #162

Closed ah-condor closed 3 months ago

ah-condor commented 4 months ago

All instructions including SYS can possibly go through register rename. A problem occurs when a SYS instr gets a renamed dest register. SYS instrutions do not go in any execution pipe. In current code renamed registers can only be made ready in an execution pipe. So SYS instructions with dest register rename can never become ready. So any subsequent instruction waiting on the destination register just hangs. The solution I made was to give the ROB a scoreboard view and make the dest register ready when the ROB does the flush upon retiring SYS instructions. In an unrelated fix in the same block of code I also added a line to set the expectedflush variable to true or else the flush fails.

How was this tested: make regress passed 72/72

klingaard commented 3 months ago

Another method to fix this issue is to send the system instruction to an existing unit (say an ALU) and wait for it to be the oldest in the machine. The ROB has a port it uses to indicate the oldest instruction. When the system instruction is the oldest, it can execute and update the rename. This will prevent a flush for the CSR reads, which are costly.

ah-condor commented 3 months ago

Another method to fix this issue is to send the system instruction to an existing unit (say an ALU) and wait for it to be the oldest in the machine. The ROB has a port it uses to indicate the oldest instruction. When the system instruction is the oldest, it can execute and update the rename. This will prevent a flush for the CSR reads, which are costly.

I am still thinking through what you described, but would you be ok with me merging this PR first and doing the optimized version later? I am not sure how long it will be if and when I do this alternate implementation.

ah-condor commented 3 months ago

Yeah, we can move forward with this PR, but I would prefer to see the alternative CSR read that I proposed. Can you work on that in the background?

Yes Knute, I will be working on the CSR read implementation in the foreground. After discussion with my team. I may email you to discuss solutions.

ah-condor commented 3 months ago

Ok I have made the last cosmetic change and sync'ed to master and ran regress. Let me know if there is anything else I need to do.