openhwgroup / cv32e41p

4 stage, in-order, secure RISC-V core based on the CV32E40P with Zfinx and Zce ISA extentions
https://docs.openhwgroup.org/projects/cv32e41p-user-manual/
Other
26 stars 11 forks source link

Move register id decoding from decode to fetch stage #14

Open abukharmeh opened 2 years ago

abukharmeh commented 2 years ago

Register name decoding is part of the critical path, moving that from ID to IF should improve timing.

Silabs-ArjanB commented 2 years ago

Hi @abukharmeh For the CV32E40X and CV32E40S we are now also implementing Zc and we are wondering about exactly this issue. Should the register file source IDs come straight from IF/ID registers or not? I believe that the jumps that are taken from ID directly based on a register file source operand plus some immediate are quite critical timing wise (their paths consists of register file read, immediate addition, output on OBI instruction address), so any gates making register file source reads slower would hurt. Of course the jumps would just need one source operand, so maybe there is something to gain there.

If register ID decoding is moved (back) to IF, is there still a point in having a 'merged decoder' in your opinion?

abukharmeh commented 2 years ago

Hi @Silabs-ArjanB,

Should the register file source IDs come straight from IF/ID registers or not >> We thought that this was a good idea looking at timing half way implementing the design, however, after I finished most of what I am planning to implement, I could not see it any more in timing report, so maybe thats not as important now, I am not sure. I am planning to have a good look at what we should optimise at some point after moving to 0.7 but I have many other pending items that have priority, so that would take a while.

The idea of merged decoder stemmed from the need to have total control of the data path for some 16-bit instructions that we could not map 1:1 with 32-bit instructions, and we didn't want to forward some of the 16 bit instructions to the complete decoder, and leave the mapper as is as we thought that would be messy, we also thought that making the decoder wider would not increase the critical path and would be better design in general. The 1:1 mapping issue does not exist anymore in Zc extension as all instructions map to either 1 or n 32 bit instructions with the exception of tblj which should be handled in IF stage directly as per #12.

Since we introduced sequencer in IF stage as in #24 I am not sure it would makes a huge difference if 16-bit instructions are decoded in IF or ID, as that mapper could be simply changed to a mapper and sequencer (1:1 or 1:N), i.e I am not sure that would change timing much !

Maybe @tariqkurd-repo have some further comments on this !

Thanks, Ibrahim