From 68f89f184f19143ceb808c28dc933a6ff4e343ea Mon Sep 17 00:00:00 2001 From: Pasquale Davide Schiavone Date: Tue, 28 Feb 2017 16:23:58 +0100 Subject: [PATCH] Fixed debug and changed controller --- controller.sv | 207 ++++++++++++++++++---------------------------- cs_registers.sv | 8 +- debug_unit.sv | 56 +------------ decoder.sv | 24 ++---- id_stage.sv | 66 +++++++++------ if_stage.sv | 1 - zeroriscy_core.sv | 7 +- 7 files changed, 135 insertions(+), 234 deletions(-) diff --git a/controller.sv b/controller.sv index 1549c5aa..98d8a534 100644 --- a/controller.sv +++ b/controller.sv @@ -65,10 +65,14 @@ module zeroriscy_controller input logic data_load_event_i, // jump/branch signals - input logic branch_taken_ex_i, // branch taken signal from EX ALU + input logic branch_in_id_i, // branch in id + input logic branch_taken_ex_i, // branch taken signal + input logic branch_set_i, // branch taken set signal input logic [1:0] jump_in_id_i, // jump is being calculated in ALU input logic [1:0] jump_in_dec_i, // jump is being calculated in ALU + input logic instr_multicyle_i, // multicycle instructions active + // Exception Controller Signals input logic int_req_i, input logic ext_req_i, @@ -76,7 +80,6 @@ module zeroriscy_controller output logic irq_ack_o, output logic exc_save_if_o, output logic exc_save_id_o, - output logic exc_save_takenbranch_o, output logic exc_restore_id_o, // Debug Signals @@ -94,9 +97,7 @@ module zeroriscy_controller output logic halt_id_o, output logic misaligned_stall_o, - output logic jr_stall_o, input logic load_stall_i, - output logic branch_2nd_stage_o, input logic id_ready_i, // ID stage is ready @@ -110,13 +111,13 @@ module zeroriscy_controller // FSM state encoding enum logic [3:0] { RESET, BOOT_SET, SLEEP, FIRST_FETCH, - DECODE, BRANCH_2ND_STAGE, + DECODE, FLUSH_WB, DBG_SIGNAL, DBG_SIGNAL_SLEEP, DBG_WAIT, DBG_WAIT_BRANCH, DBG_WAIT_SLEEP } ctrl_fsm_cs, ctrl_fsm_ns; - logic jump_done, jump_done_q; + logic jump_in_dec; logic exc_req; `ifndef SYNTHESIS @@ -153,12 +154,10 @@ module zeroriscy_controller exc_ack_o = 1'b0; exc_save_if_o = 1'b0; exc_save_id_o = 1'b0; - exc_save_takenbranch_o = 1'b0; exc_restore_id_o = 1'b0; pc_mux_o = PC_BOOT; pc_set_o = 1'b0; - jump_done = jump_done_q; ctrl_fsm_ns = ctrl_fsm_cs; @@ -169,8 +168,8 @@ module zeroriscy_controller halt_id_o = 1'b0; dbg_ack_o = 1'b0; - branch_2nd_stage_o = 1'b0; irq_ack_o = 1'b0; + jump_in_dec = jump_in_dec_i == BRANCH_JALR || jump_in_dec_i == BRANCH_JAL; unique case (ctrl_fsm_cs) // We were just reset, wait for fetch_enable @@ -249,119 +248,87 @@ module zeroriscy_controller begin is_decoding_o = 1'b0; - // decode and execute instructions only if the current conditional - // branch in the EX stage is either not taken, or there is no - // conditional branch in the EX stage - - if (instr_valid_i) - begin // now analyze the current instruction in the ID stage - is_decoding_o = 1'b1; + // decode and execute instructions only if the current conditional + // branch in the EX stage is either not taken, or there is no + // conditional branch in the EX stage + if (instr_valid_i) + begin // now analyze the current instruction in the ID stage + is_decoding_o = 1'b1; - // handle conditional branches - if ((jump_in_dec_i == BRANCH_COND) & id_ready_i) begin - halt_if_o = branch_taken_ex_i; - ctrl_fsm_ns = branch_taken_ex_i ? BRANCH_2ND_STAGE : DECODE; + + unique case (1'b1) + + branch_set_i: begin + pc_mux_o = PC_JUMP; + pc_set_o = 1'b1; + if (dbg_req_i) + ctrl_fsm_ns = DBG_SIGNAL; + end + jump_in_dec: begin + pc_mux_o = PC_JUMP; + pc_set_o = 1'b1; + if (dbg_req_i) + ctrl_fsm_ns = DBG_SIGNAL; + end + int_req_i: begin + pc_mux_o = PC_EXCEPTION; + pc_set_o = 1'b1; + exc_ack_o = 1'b1; + exc_save_id_o = 1'b1; + if (dbg_req_i) + ctrl_fsm_ns = DBG_SIGNAL; + end + eret_insn_i: begin + pc_mux_o = PC_ERET; + pc_set_o = 1'b1; + exc_restore_id_o = 1'b1; + if (dbg_req_i) + ctrl_fsm_ns = DBG_SIGNAL; + end + default: begin + + if (ext_req_i & ~instr_multicyle_i & ~branch_in_id_i) begin + pc_mux_o = PC_EXCEPTION; + pc_set_o = 1'b1; + exc_ack_o = 1'b1; + irq_ack_o = 1'b1; + exc_save_if_o = 1'b1; + end + // handle WFI instruction, flush pipeline and (potentially) go to + // sleep + // also handles eret when the core should go back to sleep + else if (pipe_flush_i || (eret_insn_i && (~fetch_enable_i))) begin + halt_if_o = 1'b1; + halt_id_o = 1'b1; + ctrl_fsm_ns = FLUSH_WB; + end + else if (dbg_req_i & ~branch_taken_ex_i) + begin + halt_if_o = 1'b1; + if (id_ready_i) begin + ctrl_fsm_ns = DBG_SIGNAL; + end + end + end + endcase end - else begin - // handle unconditional jumps - // we can jump directly since we know the address already - // we don't need to worry about conditional branches here as they - // will be evaluated in the EX stage - if ((jump_in_dec_i == BRANCH_JALR || jump_in_dec_i == BRANCH_JAL) & id_ready_i) begin - pc_mux_o = PC_JUMP; - - pc_set_o = 1'b1; - jump_done = 1'b1; - - end else begin - //ecall or illegal - if (int_req_i) begin + else if (~instr_valid_i) + begin + if (ext_req_i) begin pc_mux_o = PC_EXCEPTION; pc_set_o = 1'b1; exc_ack_o = 1'b1; - exc_save_id_o = 1'b1; - - end else if (ext_req_i) begin - pc_mux_o = PC_EXCEPTION; - pc_set_o = 1'b1; - exc_ack_o = 1'b1; - irq_ack_o = 1'b1; - exc_save_if_o = 1'b1; + exc_save_if_o = 1'b1; + irq_ack_o = 1'b1; + // we don't have to change our current state here as the prefetch + // buffer is automatically invalidated, thus the next instruction + // that is served to the ID stage is the one of the jump to the + // exception handler end - end - - if (eret_insn_i) begin - pc_mux_o = PC_ERET; - exc_restore_id_o = 1'b1; - - if ((~jump_done_q)) begin - pc_set_o = 1'b1; - jump_done = 1'b1; - end - end - - // handle WFI instruction, flush pipeline and (potentially) go to - // sleep - // also handles eret when the core should go back to sleep - if (pipe_flush_i || (eret_insn_i && (~fetch_enable_i))) - begin - halt_if_o = 1'b1; - halt_id_o = 1'b1; - - ctrl_fsm_ns = FLUSH_WB; - end - else if (dbg_req_i) - begin - // take care of debug - // branch conditional will be handled in next state - // halt pipeline immediately - halt_if_o = 1'b1; - - // make sure the current instruction has been executed - // before changing state to non-decode - if (id_ready_i) begin - ctrl_fsm_ns = DBG_SIGNAL; - end - end end - end - - - else if (~instr_valid_i) - begin - if (ext_req_i) begin - pc_mux_o = PC_EXCEPTION; - pc_set_o = 1'b1; - exc_ack_o = 1'b1; - exc_save_if_o = 1'b1; - irq_ack_o = 1'b1; - // we don't have to change our current state here as the prefetch - // buffer is automatically invalidated, thus the next instruction - // that is served to the ID stage is the one of the jump to the - // exception handler - end - end - end - - BRANCH_2ND_STAGE: - begin - // there is a branch in the EX stage that is taken - branch_2nd_stage_o = 1'b1; - pc_mux_o = PC_BRANCH; - pc_set_o = 1'b1; - ctrl_fsm_ns = DECODE; - halt_if_o = 1'b1; - - if (dbg_req_i) - begin - ctrl_fsm_ns = DBG_SIGNAL; - end - end - - // now we can signal to the debugger that our pipeline is empty and it // can examine our current state DBG_SIGNAL: @@ -455,7 +422,6 @@ module zeroriscy_controller always_comb begin - jr_stall_o = 1'b0; deassert_we_o = 1'b0; // deassert WE when the core is not decoding instructions @@ -465,14 +431,6 @@ module zeroriscy_controller // deassert WE in case of illegal instruction if (illegal_insn_i) deassert_we_o = 1'b1; - - // Stall because of load operation - - - // Stall because of jr path - // - always stall if a result is to be forwarded to the PC - // we don't care about in which state the ctrl_fsm is as we deassert_we - // anyway when we are not in DECODE end // Forwarding control unit @@ -484,20 +442,19 @@ module zeroriscy_controller if ( rst_n == 1'b0 ) begin ctrl_fsm_cs <= RESET; - jump_done_q <= 1'b0; + //jump_done_q <= 1'b0; end else begin ctrl_fsm_cs <= ctrl_fsm_ns; - // clear when id is valid (no instruction incoming) - jump_done_q <= jump_done & (~id_ready_i); + //jump_done_q <= jump_done & (~id_ready_i); end end // Performance Counters assign perf_jump_o = (jump_in_id_i == BRANCH_JAL || jump_in_id_i == BRANCH_JALR); - assign perf_jr_stall_o = jr_stall_o; + assign perf_jr_stall_o = 1'b0; assign perf_ld_stall_o = load_stall_i; diff --git a/cs_registers.sv b/cs_registers.sv index 6b957581..ccbbb86e 100644 --- a/cs_registers.sv +++ b/cs_registers.sv @@ -65,7 +65,6 @@ module zeroriscy_cs_registers input logic data_load_event_ex_i, input logic exc_save_if_i, input logic exc_save_id_i, - input logic exc_save_takenbranch_i, input logic exc_restore_i, input logic [5:0] exc_cause_i, @@ -185,13 +184,11 @@ module zeroriscy_cs_registers endcase // exception controller gets priority over other writes - if (exc_save_if_i || exc_save_id_i || exc_save_takenbranch_i) begin + if (exc_save_if_i || exc_save_id_i) begin mstatus_n = {mie,1'b0}; if (data_load_event_ex_i) begin mepc_n = pc_id_i; - end else if (exc_save_takenbranch_i) begin - mepc_n = branch_target_i; end else begin if (exc_save_if_i) mepc_n = pc_if_i; @@ -439,7 +436,4 @@ module zeroriscy_cs_registers end end - assert property ( - @(posedge clk) (~(exc_save_takenbranch_i & data_load_event_ex_i)) ) else $display("Both exc_save_takenbranch_i and data_load_event_ex_i are active"); - endmodule diff --git a/debug_unit.sv b/debug_unit.sv index 61da468d..13ed9d76 100644 --- a/debug_unit.sv +++ b/debug_unit.sv @@ -117,7 +117,6 @@ module zeroriscy_debug_unit // ppc/npc tracking - enum logic [1:0] {IFID, IFEX, IDEX} pc_tracking_fsm_cs, pc_tracking_fsm_ns; logic [31:0] ppc_int, npc_int; @@ -416,63 +415,13 @@ module zeroriscy_debug_unit //---------------------------------------------------------------------------- // NPC/PPC selection //---------------------------------------------------------------------------- - always_comb - begin - pc_tracking_fsm_ns = pc_tracking_fsm_cs; - - ppc_int = pc_id_i; - npc_int = pc_if_i; - - // PPC/NPC mux - unique case (pc_tracking_fsm_cs) - IFID: begin - ppc_int = pc_id_i; - npc_int = pc_if_i; - end - - IFEX: begin - ppc_int = pc_id_i; - npc_int = pc_if_i; - end - - IDEX: begin - ppc_int = pc_id_i; - npc_int = pc_if_i; - - - if (jump_req_o) - pc_tracking_fsm_ns = IFEX; - end - - default: begin - pc_tracking_fsm_ns = IFID; - end - endcase - - // set state if trap is encountered - if (dbg_ack_i) begin - pc_tracking_fsm_ns = IFID; - - if (branch_in_ex_i) begin - if (branch_taken_i) - pc_tracking_fsm_ns = IFEX; - else - pc_tracking_fsm_ns = IDEX; - end else if (data_load_event_i) begin - // for p.elw - if (instr_valid_id_i) - pc_tracking_fsm_ns = IDEX; - else - pc_tracking_fsm_ns = IFEX; - end - end - end + assign ppc_int = pc_id_i; + assign npc_int = pc_if_i; always_ff @(posedge clk, negedge rst_n) begin if (~rst_n) begin - pc_tracking_fsm_cs <= IFID; addr_q <= '0; wdata_q <= '0; @@ -484,7 +433,6 @@ module zeroriscy_debug_unit settings_q <= 1'b0; end else begin - pc_tracking_fsm_cs <= pc_tracking_fsm_ns; // settings settings_q <= settings_n; diff --git a/decoder.sv b/decoder.sv index 2e7aafa0..9fc03d24 100644 --- a/decoder.sv +++ b/decoder.sv @@ -35,7 +35,7 @@ module zeroriscy_decoder // singals running to/from controller input logic deassert_we_i, // deassert we, we are stalled or not active input logic data_misaligned_i, // misaligned data load/store in progress - input logic branch_2nd_stage_i, + input logic branch_set_i, output logic illegal_insn_o, // illegal instruction encountered output logic ebrk_insn_o, // trap instruction encountered @@ -82,7 +82,8 @@ module zeroriscy_decoder // jump/branches output logic [1:0] jump_in_dec_o, // jump_in_id without deassert - output logic [1:0] jump_in_id_o // jump is being calculated in ALU + output logic [1:0] jump_in_id_o, // jump is being calculated in ALU + output logic branch_in_id_o ); // write enable/request control @@ -94,6 +95,7 @@ module zeroriscy_decoder logic pipe_flush; logic mult_int_en; + logic branch_in_id; logic [1:0] jump_in_id; logic [1:0] csr_op; @@ -111,7 +113,7 @@ module zeroriscy_decoder always_comb begin jump_in_id = BRANCH_NONE; - + branch_in_id = 1'b0; alu_operator_o = ALU_SLTU; alu_op_a_mux_sel_o = OP_A_REGA_OR_FWD; alu_op_b_mux_sel_o = OP_B_REGB_OR_FWD; @@ -195,8 +197,9 @@ module zeroriscy_decoder rega_used_o = 1'b1; regb_used_o = 1'b1; + branch_in_id = 1'b1; - if (~branch_2nd_stage_i) + if (~branch_set_i) begin unique case (instr_rdata_i[14:12]) 3'b000: alu_operator_o = ALU_EQ; @@ -205,18 +208,6 @@ module zeroriscy_decoder 3'b101: alu_operator_o = ALU_GES; 3'b110: alu_operator_o = ALU_LTU; 3'b111: alu_operator_o = ALU_GEU; - 3'b010: begin - alu_operator_o = ALU_EQ; - regb_used_o = 1'b0; - alu_op_b_mux_sel_o = OP_B_IMM; - imm_b_mux_sel_o = IMMB_BI; - end - 3'b011: begin - alu_operator_o = ALU_NE; - regb_used_o = 1'b0; - alu_op_b_mux_sel_o = OP_B_IMM; - imm_b_mux_sel_o = IMMB_BI; - end default: begin illegal_insn_o = 1'b1; end @@ -538,6 +529,7 @@ module zeroriscy_decoder assign data_req_o = (deassert_we_i) ? 1'b0 : data_req; assign csr_op_o = (deassert_we_i) ? CSR_OP_NONE : csr_op; assign jump_in_id_o = (deassert_we_i) ? BRANCH_NONE : jump_in_id; + assign branch_in_id_o = (deassert_we_i) ? 1'b0 : branch_in_id; assign ebrk_insn_o = (deassert_we_i) ? 1'b0 : ebrk_insn; assign eret_insn_o = (deassert_we_i) ? 1'b0 : eret_insn; // TODO: do not deassert? assign pipe_flush_o = (deassert_we_i) ? 1'b0 : pipe_flush; // TODO: do not deassert? diff --git a/id_stage.sv b/id_stage.sv index 618ddf06..d4e4fe59 100644 --- a/id_stage.sv +++ b/id_stage.sv @@ -116,7 +116,6 @@ module zeroriscy_id_stage output logic exc_save_if_o, output logic exc_save_id_o, - output logic exc_save_takenbranch_o, output logic exc_restore_id_o, input logic lsu_load_err_i, @@ -164,12 +163,14 @@ module zeroriscy_id_stage logic regb_used_dec; logic branch_taken_ex; + logic branch_in_id; + logic branch_set; logic [1:0] jump_in_id; logic [1:0] jump_in_dec; - logic branch_2nd_stage; - logic jr_stall; + logic instr_multicyle; logic load_stall; + logic branch_stall; logic halt_id; //FSM signals to write back multi cycles instructions @@ -274,7 +275,7 @@ module zeroriscy_id_stage // signal to 0 for instructions that are done assign clear_instr_valid_o = id_ready_o | halt_id; - assign branch_taken_ex = branch_in_ex_o & (branch_decision_i | branch_2nd_stage); + assign branch_taken_ex = branch_in_id & branch_decision_i; //////////////////////////////////////////////////////// // ___ _ _ // @@ -455,7 +456,7 @@ module zeroriscy_id_stage // controller related signals .deassert_we_i ( deassert_we ), .data_misaligned_i ( data_misaligned_i ), - .branch_2nd_stage_i ( branch_2nd_stage ), + .branch_set_i ( branch_set ), .illegal_insn_o ( illegal_insn_dec ), .ebrk_insn_o ( ebrk_insn ), @@ -497,7 +498,8 @@ module zeroriscy_id_stage // jump/branches .jump_in_dec_o ( jump_in_dec ), - .jump_in_id_o ( jump_in_id ) + .jump_in_id_o ( jump_in_id ), + .branch_in_id_o ( branch_in_id ) ); //////////////////////////////////////////////////////////////////// @@ -541,10 +543,13 @@ module zeroriscy_id_stage .data_load_event_i ( data_load_event_ex_o ), // jump/branch control + .branch_in_id_i ( branch_in_id ), .branch_taken_ex_i ( branch_taken_ex ), + .branch_set_i ( branch_set ), .jump_in_id_i ( jump_in_id ), .jump_in_dec_i ( jump_in_dec ), + .instr_multicyle_i ( instr_multicyle ), // Exception Controller Signals .int_req_i ( int_req ), .ext_req_i ( ext_req ), @@ -552,7 +557,6 @@ module zeroriscy_id_stage .irq_ack_o ( irq_ack_o ), .exc_save_if_o ( exc_save_if_o ), .exc_save_id_o ( exc_save_id_o ), - .exc_save_takenbranch_o ( exc_save_takenbranch_o ), .exc_restore_id_o ( exc_restore_id_o ), // Debug Unit Signals @@ -568,9 +572,6 @@ module zeroriscy_id_stage .halt_if_o ( halt_if_o ), .halt_id_o ( halt_id ), - .branch_2nd_stage_o ( branch_2nd_stage ), - .jr_stall_o ( jr_stall ), - .id_ready_i ( id_ready_o ), .if_valid_i ( if_valid_i ), @@ -652,7 +653,7 @@ module zeroriscy_id_stage assign data_reg_offset_ex_o = data_reg_offset_id; assign data_load_event_ex_o = ((data_req_id & (~halt_id)) ? data_load_event_id : 1'b0); - assign branch_in_ex_o = (jump_in_dec == BRANCH_COND); + assign branch_in_ex_o = branch_in_id; enum logic { IDLE, WAIT_MULTICYCLE } id_wb_fsm_cs, id_wb_fsm_ns; @@ -663,11 +664,9 @@ module zeroriscy_id_stage begin : EX_WB_Pipeline_Register if (~rst_n) begin - //regfile_we_q <= 1'b0; id_wb_fsm_cs <= IDLE; end else begin - //regfile_we_q <= regfile_mem_we_id & load_stall; id_wb_fsm_cs <= id_wb_fsm_ns; end end @@ -679,32 +678,47 @@ module zeroriscy_id_stage always_comb begin id_wb_fsm_ns = id_wb_fsm_cs; - regfile_we = regfile_we_id & (~halt_id); + regfile_we = regfile_we_id & (~halt_id); load_stall = 1'b0; + branch_stall = 1'b0; select_data_rf = RF_EX; + instr_multicyle = 1'b0; + branch_set = 1'b0; unique case (id_wb_fsm_cs) IDLE: begin - if(data_req_id) begin + unique case (1'b1) + data_req_id: begin //LSU operation - regfile_we = 1'b0; - id_wb_fsm_ns = WAIT_MULTICYCLE; - load_stall = 1'b1; - end + regfile_we = 1'b0; + id_wb_fsm_ns = WAIT_MULTICYCLE; + load_stall = 1'b1; + instr_multicyle = 1'b1; + end + branch_in_id: begin + //Cond Branch operation + id_wb_fsm_ns = branch_decision_i ? WAIT_MULTICYCLE : IDLE; + branch_stall = branch_decision_i; + instr_multicyle = branch_decision_i; + end + default:; + endcase end WAIT_MULTICYCLE: begin if(ex_ready_i) begin - regfile_we = regfile_we_id; - id_wb_fsm_ns = IDLE; - load_stall = 1'b0; - select_data_rf = data_req_id ? RF_LSU : RF_EX; + regfile_we = regfile_we_id; + id_wb_fsm_ns = IDLE; + load_stall = 1'b0; + select_data_rf = data_req_id ? RF_LSU : RF_EX; + branch_set = branch_in_id; end else begin - regfile_we = 1'b0; - load_stall = 1'b1; + instr_multicyle = 1'b1; + regfile_we = 1'b0; + load_stall = 1'b1; end end @@ -713,7 +727,7 @@ module zeroriscy_id_stage end // stall control - assign id_ready_o = (~jr_stall) & (~load_stall); + assign id_ready_o = (~load_stall) & (~branch_stall); assign id_valid_o = (~halt_id) & id_ready_o; diff --git a/if_stage.sv b/if_stage.sv index 59ae1b81..6b0dc3fd 100644 --- a/if_stage.sv +++ b/if_stage.sv @@ -65,7 +65,6 @@ module zeroriscy_if_stage #( // from hwloop controller // from debug unit input logic [31:0] dbg_jump_addr_i, - input logic dbg_jump_req_i, // pipeline stall input logic halt_if_i, output logic if_ready_o, diff --git a/zeroriscy_core.sv b/zeroriscy_core.sv index 374b783e..8b748c8d 100644 --- a/zeroriscy_core.sv +++ b/zeroriscy_core.sv @@ -309,7 +309,6 @@ module zeroriscy_core // from debug unit .dbg_jump_addr_i ( dbg_jump_addr ), - .dbg_jump_req_i ( dbg_jump_req ), // Jump targets .jump_target_ex_i ( jump_target_ex ), @@ -411,7 +410,6 @@ module zeroriscy_core .save_exc_cause_o ( save_exc_cause ), .exc_save_if_o ( exc_save_if ), // control signal to save pc .exc_save_id_o ( exc_save_id ), // control signal to save pc - .exc_save_takenbranch_o ( exc_save_takenbranch_ex ), // control signal to save target taken branch .exc_restore_id_o ( exc_restore_id ), // control signal to restore pc .lsu_load_err_i ( lsu_load_err ), .lsu_store_err_i ( lsu_store_err ), @@ -558,7 +556,6 @@ module zeroriscy_core .data_load_event_ex_i ( data_load_event_ex ), // from ID/EX pipeline .exc_save_if_i ( exc_save_if ), .exc_save_id_i ( exc_save_id ), - .exc_save_takenbranch_i ( exc_save_takenbranch_ex ), .exc_restore_i ( exc_restore_id ), .exc_cause_i ( exc_cause ), @@ -647,8 +644,8 @@ module zeroriscy_core // signals for PPC and NPC .pc_if_i ( pc_if ), // from IF stage - .pc_id_i ( pc_id ), // from IF stage - + .pc_id_i ( pc_id ), // from ID stage + .pc_branch_i ( jump_target_ex ), .data_load_event_i ( data_load_event_ex ), .instr_valid_id_i ( instr_valid_id ),