Skip to content
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

Fixed todo's related to mul_en/div_en i EX stage #902

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions rtl/cv32e40x_div.sv
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ module cv32e40x_div import cv32e40x_pkg::*;
output logic [5:0] alu_shift_amt_o,
input logic [31:0] alu_op_b_shifted_i,

// Divider enable
input logic div_en_i,
// Controller inputs
input logic halt_i,
input logic kill_i,

// Input handshake
input logic valid_i,
Expand Down Expand Up @@ -141,12 +142,12 @@ module cv32e40x_div import cv32e40x_pkg::*;
end
endgenerate

assign alu_clz_en_o = div_en_i;
assign alu_clz_en_o = valid_i;

// Deternmine initial shift of divisor
assign op_b_is_neg = op_b_i[31] & div_signed;
assign alu_shift_amt_o = alu_clz_result_i ;
assign alu_shift_en_o = div_en_i;
assign alu_shift_amt_o = alu_clz_result_i;
assign alu_shift_en_o = valid_i;

// Check for op_b_i == 0
assign op_b_is_zero = !(|op_b_i);
Expand Down Expand Up @@ -224,8 +225,9 @@ module cv32e40x_div import cv32e40x_pkg::*;
divisor_en = 1'b0;
quotient_en = 1'b0;

// Case statement assumes valid_i = 1; the valid_i = 0 scenario
// is handled after the case statement.
// Case statement assumes valid_i = 1, halt_i = 0 and kill_i = 0.
// the valid_i = 0 / halt_i = 1 / kill_i = 1 scenarios
// are handled after the case statement.
case (state)
DIV_IDLE: begin
remainder_en = 1'b1;
Expand Down Expand Up @@ -259,6 +261,7 @@ module cv32e40x_div import cv32e40x_pkg::*;

DIV_FINISH: begin
valid_o = 1'b1;

if (ready_i) begin
ready_o = 1'b1;
next_state = DIV_IDLE;
Expand All @@ -270,12 +273,32 @@ module cv32e40x_div import cv32e40x_pkg::*;
endcase

// Allow kill at any time
if (!valid_i) begin
next_state = DIV_IDLE;
if (!valid_i || kill_i) begin
ready_o = 1'b1;
valid_o = 1'b0;
end
next_state = DIV_IDLE;

init_en = 1'b0;
init_dummy_cnt = 1'b0;

remainder_en = 1'b0;
divisor_en = 1'b0;
quotient_en = 1'b0;
end else begin
// Neither ready nor valid when halted
if (halt_i) begin
ready_o = 1'b0;
valid_o = 1'b0;
next_state = state;

init_en = 1'b0;
init_dummy_cnt = 1'b0;

remainder_en = 1'b0;
divisor_en = 1'b0;
quotient_en = 1'b0;
end
end
end

///////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -304,6 +327,13 @@ module cv32e40x_div import cv32e40x_pkg::*;
comp_inv_q <= 1'b0;
res_inv_q <= 1'b0;
end else begin
// Update flops on valid input or when killed. No updates while halted.
// When a divide instruction is killed, the cnt_q will decrement unless
// the divider is finished in the same cycle as the kill.
// All flops below will keep their values until initialized for the next
// divide (init_en, remainder_en, divisor_en, quotient_en) and no flops
// are used between kill and init.
if ((valid_i && !halt_i) || kill_i) begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark as for multiplier

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, SEC clean.

state <= next_state;
remainder_q <= remainder_d;
divisor_q <= divisor_d;
Expand All @@ -312,6 +342,7 @@ module cv32e40x_div import cv32e40x_pkg::*;
div_rem_q <= div_rem_d;
comp_inv_q <= comp_inv_d;
res_inv_q <= res_inv_d;
end
end
end

Expand Down
30 changes: 17 additions & 13 deletions rtl/cv32e40x_ex_stage.sv
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
logic [31:0] div_result;

// Gated enable signals factoring in instr_valid)
logic mul_en_gated;
logic div_en_gated;
logic lsu_en_gated;

// Divider signals
Expand All @@ -116,6 +114,9 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
logic [5:0] div_shift_amt;
logic [31:0] div_op_b_shifted;

// Multiplier signals
logic mul_en;

// Misc signals
logic previous_exception;
logic first_op;
Expand All @@ -126,14 +127,15 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;

assign instr_valid = id_ex_pipe_i.instr_valid && !ctrl_fsm_i.kill_ex && !ctrl_fsm_i.halt_ex;

// todo: consider not factoring halt_ex into the mul/div/lsu_en_gated below
// Halting EX currently reset state of these units. The IF stage sequencer is _not_ reset on a halt_if.
// Maybe we need to split out valid and halt into the submodules?
assign mul_en_gated = id_ex_pipe_i.mul_en && instr_valid; // Factoring in instr_valid to kill mul instructions on kill/halt
assign div_en_gated = id_ex_pipe_i.div_en && instr_valid; // Factoring in instr_valid to kill div instructions on kill/halt
// The multiplier and divider both factor in halt_ex and kill_ex.
// MUL/DIV instructions in flight will keep state while halted, and reset state on kill.
assign mul_en = id_ex_pipe_i.mul_en && id_ex_pipe_i.instr_valid; // Valid MUL in EX, not affected by kill/halt
assign div_en = id_ex_pipe_i.div_en && id_ex_pipe_i.instr_valid; // Valid DIV in EX, not affected by kill/halt

// The lsu_en_gated factors in halt_ex and kill_ex via instr_valid.
// A halted or killed LSU instruction must not generate a data_req to ensure no OBI protocol is violated.
assign lsu_en_gated = id_ex_pipe_i.lsu_en && instr_valid; // Factoring in instr_valid to suppress bus transactions on kill/halt

assign div_en = id_ex_pipe_i.div_en && id_ex_pipe_i.instr_valid; // Valid DIV in EX, not affected by kill/halt

// If pipeline is handling a valid CSR AND the same instruction is accepted by the eXtension interface
// we need to convert the instruction to an illegal instruction and signal commit_kill to the eXtension interface.
Expand Down Expand Up @@ -250,11 +252,10 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
// Result
.result_o ( div_result ),

// divider enable, not affected by kill/halt
.div_en_i ( div_en ),

// Handshakes
.valid_i ( div_en_gated ),
.halt_i ( ctrl_fsm_i.halt_ex ),
.kill_i ( ctrl_fsm_i.kill_ex ),
.valid_i ( div_en ),
.ready_o ( div_ready ),
.valid_o ( div_valid ),
.ready_i ( wb_ready_i )
Expand Down Expand Up @@ -300,8 +301,11 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
// Result
.result_o ( mul_result ),

.halt_i ( ctrl_fsm_i.halt_ex ),
.kill_i ( ctrl_fsm_i.kill_ex ),

// Handshakes
.valid_i ( mul_en_gated ),
.valid_i ( mul_en ),
.ready_o ( mul_ready ),
.valid_o ( mul_valid ),
.ready_i ( wb_ready_i )
Expand Down
38 changes: 29 additions & 9 deletions rtl/cv32e40x_mult.sv
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ module cv32e40x_mult import cv32e40x_pkg::*;

output logic [31:0] result_o,

input logic halt_i,
input logic kill_i,

output logic ready_o,
output logic valid_o,
input logic ready_i
Expand Down Expand Up @@ -114,8 +117,9 @@ module cv32e40x_mult import cv32e40x_pkg::*;
valid_o = 1'b0;
mulh_acc_next = mulh_acc;

// Case statement assumes valid_i = 1; the valid_i = 0 scenario
// is handled after the case statement.
// Case statement assumes valid_i = 1, halt_i = 0 and kill_i = 0.
// the valid_i = 0 / halt_i = 1 / kill_i = 1 scenarios
// are handled after the case statement.
case (mulh_state)
MUL_ALBL: begin
if (operator_i == MUL_H) begin
Expand All @@ -129,7 +133,7 @@ module cv32e40x_mult import cv32e40x_pkg::*;
valid_o = 1'b1;

if (ready_i) begin
ready_o = 1'b1;
ready_o = 1'b1;
end
end
end
Expand Down Expand Up @@ -164,11 +168,24 @@ module cv32e40x_mult import cv32e40x_pkg::*;
endcase

// Allow kill at any time
if (!valid_i) begin
if (!valid_i || kill_i) begin
mulh_state_next = MUL_ALBL;
ready_o = 1'b1;
valid_o = 1'b0;
mulh_acc_next = '0;
valid_o = 1'b0;
ready_o = 1'b1;
mulh_acc_next = '0;
mulh_shift = 1'b0;
mulh_a = mulh_al;
mulh_b = mulh_bl;
end else begin
if (halt_i) begin
mulh_state_next = mulh_state;
valid_o = 1'b0;
ready_o = 1'b0;
mulh_acc_next = mulh_acc;
mulh_shift = 1'b0;
mulh_a = mulh_al;
mulh_b = mulh_bl;
end
end
end // always_comb

Expand All @@ -177,8 +194,11 @@ module cv32e40x_mult import cv32e40x_pkg::*;
mulh_acc <= '0;
mulh_state <= MUL_ALBL;
end else begin
mulh_acc <= mulh_acc_next;
mulh_state <= mulh_state_next;
// Update flops on valid input or when killed. No updates while halted.
if ((valid_i && !halt_i) || kill_i) begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above mulh_state_next is changed also for !valid, but here that scenario is not handled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, SEC clean.

mulh_acc <= mulh_acc_next;
mulh_state <= mulh_state_next;
end
end
end

Expand Down
13 changes: 9 additions & 4 deletions sva/cv32e40x_mult_sva.sv
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ module cv32e40x_mult_sva
input logic [16:0] mulh_bl,
input logic [16:0] mulh_ah,
input logic [16:0] mulh_bh,
input logic [33:0] int_result);
input logic [33:0] int_result,
input logic halt_i,
input logic kill_i);

////////////////////////////////////////
//// Assertions on module boundary ////
Expand All @@ -54,13 +56,16 @@ module cv32e40x_mult_sva
assert property (@(posedge clk) disable iff (!rst_n)
(valid_i && (operator_i == MUL_M32)) |-> (result_o == mul_result))
else `uvm_error("mult", "MUL result check failed")
a_mul_valid : // check that MUL result is immediately qualified by valid_o
a_mul_valid : // check that MUL result is immediately qualified by valid_o unless EX is halted or killed
assert property (@(posedge clk) disable iff (!rst_n)
(valid_i && (operator_i == MUL_M32)) |-> valid_o)
(valid_i && (operator_i == MUL_M32) &&
!(halt_i || kill_i))
|->
valid_o)
else `uvm_error("mult", "MUL result wasn't immediately valid")


// Check result for all MULH flavors
// Check result for all MULH flavors
logic mulh_result_valid;
assign mulh_result_valid = ((operator_i == MUL_H) && valid_i) && valid_o;

Expand Down