-
Notifications
You must be signed in to change notification settings - Fork 52
Add support for RV32E #695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Oivind Ekelund <[email protected]>
rtl/cv32e40x_id_stage.sv
Outdated
@@ -739,16 +746,20 @@ module cv32e40x_id_stage import cv32e40x_pkg::*; | |||
xif_issue_if.issue_req.rs_valid = '0; | |||
if (xif_issue_if.X_NUM_RS > 0) begin | |||
xif_issue_if.issue_req.rs [0] = operand_a_fw; | |||
xif_issue_if.issue_req.rs_valid[0] = 1'b1; | |||
xif_issue_if.issue_req.rs_valid[0] = (ctrl_byp_i.operand_a_fw_mux_sel == SEL_REGFILE) ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the select based on ctrl_byp_i.operand_a_fw_mux_sel == SEL_REGFILE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because values can be forwarded from EX or WB stage. Upon forwarding, the RF address check in the decode stage will not be applicable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that the RF address check from the decoder stage would always apply, so it should be:
xif_issue_if.issue_req.rs_valid[0] = !rf_illegal_raddr[0];
(Of course in reality ctrl_byp_i.operand_a_fw_mux_sel == SEL_REGFILE will always hold and then your code proposal and my proposal are the same. Also note that stalls/bypass usage is not correct yet, see #277)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, you're right. Updated.
…ch sub-decoder Signed-off-by: Oivind Ekelund <[email protected]>
); | ||
|
||
assign dec_a_rf_illegal_addr = (decoder_a_ctrl_int.rf_re[0] && rf_illegal_raddr_o[0]) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assign this signal to 1'b0 in 'else' part of generate statement so that we do net get floating signals (even though they are not used). Same remark for other similar signals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
rtl/cv32e40x_id_stage.sv
Outdated
@@ -739,16 +746,20 @@ module cv32e40x_id_stage import cv32e40x_pkg::*; | |||
xif_issue_if.issue_req.rs_valid = '0; | |||
if (xif_issue_if.X_NUM_RS > 0) begin | |||
xif_issue_if.issue_req.rs [0] = operand_a_fw; | |||
xif_issue_if.issue_req.rs_valid[0] = 1'b1; | |||
xif_issue_if.issue_req.rs_valid[0] = (ctrl_byp_i.operand_a_fw_mux_sel == SEL_REGFILE) ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say that the RF address check from the decoder stage would always apply, so it should be:
xif_issue_if.issue_req.rs_valid[0] = !rf_illegal_raddr[0];
(Of course in reality ctrl_byp_i.operand_a_fw_mux_sel == SEL_REGFILE will always hold and then your code proposal and my proposal are the same. Also note that stalls/bypass usage is not correct yet, see #277)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also see the XIF related comments.
… generation. Signed-off-by: Oivind Ekelund <[email protected]>
SEC clean when parameter RV32=RV32I
Signed-off-by: Oivind Ekelund [email protected]