Changed logic for sequence progress detection #638
Conversation
…B and ID by using flipflops, and the SVA part uses the old combinatorial method. Done to avoid combinatorial loops in future cleanup. SEC is undetermined at the point of commit. Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Silabs-ArjanB
added
the
Component:RTL
bhv/cv32e40x_wrapper.sv
Outdated
| .first_op_if_i (core_i.first_op_if), | ||
| .first_op_ex_i (core_i.first_op_ex), | ||
| .instr_valid_if_i (core_i.if_stage_i.prefetch_valid), | ||
| .ptr_in_if_i (core_i.if_stage_i.prefetch_is_tbljmp_ptr), |
There was a problem hiding this comment.
No signal renaming like this in connections, especially because ptr_in_if seems to imply both table jump pointer and clic pointer
There was a problem hiding this comment.
I will fix it.
| logic id_stage_haltable; | ||
|
|
||
| assign sequence_interruptible = !sequence_in_progress_wb; |
rtl/cv32e40x_controller_fsm.sv
Outdated
| sequence_in_progress_wb <= 1'b0; | ||
| end else begin | ||
| if (!sequence_in_progress_wb) begin | ||
| if (wb_valid_i && ex_wb_pipe_i.first_op && !last_op_wb_i) begin |
There was a problem hiding this comment.
I would expect this to also be qualified by ex_wb_pipe_i.instr_valid; shouldn't it?
There was a problem hiding this comment.
That should not be needed, as wb_valid_i already depends on ex_wb_pipe_i.instr_valid
| end | ||
| end else begin | ||
| // sequence_in_progress_wb is set, clear when last_op retires | ||
| if (wb_valid_i && last_op_wb_i) begin |
There was a problem hiding this comment.
Same reply as above
rtl/cv32e40x_controller_fsm.sv
Outdated
| end | ||
| end | ||
|
|
||
| // Helper logic to track first_op and last_op through the ID stage to detect unhaltable sequences |
There was a problem hiding this comment.
Update comment; this is not 'helper logic'
| end | ||
| end else begin | ||
| // sequence_in_progress_id is set, clear when last_op retires | ||
| if (id_valid_i && ex_ready_i && last_op_id_i) begin |
There was a problem hiding this comment.
Same remarks as above related to usage of isntr_valid. Is adding the instr_valid signals a SEC clean change?
There was a problem hiding this comment.
It would likely be SEC clean, but as for the others the instr_valid is baked into the valid
Signed-off-by: Oystein Knauserud <[email protected]>
Swapped the RTL detection logic with the logic used in the SVA file. This is done to avoid a combinatorial loop is using 'seq_valid' to calculate first_op of the IF stage.
First PR of a series to clean up Zc code and in particular handling of first_op and last_op.
SEC not determined at the time of PR (for both ZC_EXT=0 and ZC_EXT=1)