Bugfix: mnxti data forwarding causing wrong operand values #511
Conversation
…sed wrong operand values as the data forwareded is not the data written to the register file. Signed-off-by: Oystein Knauserud <[email protected]>
Silabs-ArjanB
added
the
Component:RTL
rtl/cv32e40x_controller_bypass.sv
Outdated
| if (alu_jmpr_id_i && alu_en_raw_id_i && ((rf_we_wb && rf_rd_wb_jalr_match && lsu_en_wb) || (rf_we_ex && rf_rd_ex_jalr_match))) begin | ||
| if ((alu_jmpr_id_i && alu_en_raw_id_i) && | ||
| ((rf_we_wb && rf_rd_wb_jalr_match && lsu_en_wb) || | ||
| (rf_we_wb && rf_rd_wb_jalr_match && (ex_wb_pipe_i.csr_addr == CSR_MNXTI) && ex_wb_pipe_i.csr_en) || |
There was a problem hiding this comment.
Wouldn't it be cheaper if the CSR_MNXTI comparison from the CS register file is re-used here (delayed to have the correct timing)?
Why do we actually need this MNXTI based stall (as the required bypass is actually in the RTL)? Please add a comment explaning why.
There was a problem hiding this comment.
Added comment. The reason is that the jalr_fw is using the flopped ex_wb_pipe.rf_wdata (as opposed to the wb_stage.rf_wdata_o which causes timing paths from data_rvalid_i). If forwarding the mnxti read data the jalr_fw mux would need to also consider the pointer address from cs_register, and the use case for this CSR would likely not generate jumps following a CSR access to mnxti.
I agree on sharing the logic, I will implement that.
There was a problem hiding this comment.
Thanks for the explanation. That is okay then; I had not seen that the registered rf_wdata was used in that bypass. This actually also means that there is a jalr_fw bug in case there is a result incoming via XIF in the WB stage that is supposed to be used by the JALR. Can you just add a todo for that in the code (no need to fix it as the XIF logic will get reorganized all together).
| @@ -361,6 +374,35 @@ always_ff @(posedge clk , negedge rst_ni) | |||
| endgenerate | |||
|
|
|||
|
|
|||
| // Check that operand_a data forwarded from EX to ID actually is written to RF in WB | |||
There was a problem hiding this comment.
Can you add the equivalent assertions when forwarding from WB?
Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Fixed bug where a results forwarded from EX due to a mnxti access caused wrong operand values
as the data forwareded is not the data written to the register file.
Signed-off-by: Oystein Knauserud [email protected]