From 97bddf0d54aeda08bcc790224e04054f31844eaa Mon Sep 17 00:00:00 2001 From: David Harris Date: Fri, 13 Jan 2023 21:09:29 -0800 Subject: [PATCH] csr cleanup --- pipelined/src/privileged/csr.sv | 73 +++++----- pipelined/src/privileged/csrc.sv | 229 +++++++++++++++---------------- 2 files changed, 151 insertions(+), 151 deletions(-) diff --git a/pipelined/src/privileged/csr.sv b/pipelined/src/privileged/csr.sv index 740634125..fb1067c4c 100644 --- a/pipelined/src/privileged/csr.sv +++ b/pipelined/src/privileged/csr.sv @@ -198,44 +198,49 @@ module csr #(parameter /////////////////////////////////////////// csri csri(.clk, .reset, .InstrValidNotFlushedM, - .CSRMWriteM, .CSRSWriteM, .CSRWriteValM, .CSRAdrM, - .MExtInt, .SExtInt, .MTimerInt, .MSwInt, - .MIP_REGW, .MIE_REGW, .MIP_REGW_writeable); + .CSRMWriteM, .CSRSWriteM, .CSRWriteValM, .CSRAdrM, + .MExtInt, .SExtInt, .MTimerInt, .MSwInt, + .MIP_REGW, .MIE_REGW, .MIP_REGW_writeable); csrsr csrsr(.clk, .reset, .StallW, - .WriteMSTATUSM, .WriteMSTATUSHM, .WriteSSTATUSM, - .TrapM, .FRegWriteM, .NextPrivilegeModeM, .PrivilegeModeW, - .mretM, .sretM, .WriteFRMM, .WriteFFLAGSM, .CSRWriteValM, .SelHPTW, - .MSTATUS_REGW, .SSTATUS_REGW, .MSTATUSH_REGW, - .STATUS_MPP, .STATUS_SPP, .STATUS_TSR, .STATUS_TW, - .STATUS_MIE, .STATUS_SIE, .STATUS_MXR, .STATUS_SUM, .STATUS_MPRV, .STATUS_TVM, - .STATUS_FS, .BigEndianM); - csrc counters(.clk, .reset, .StallE, .StallM, .FlushM, - .InstrValidNotFlushedM, .LoadStallD, .CSRMWriteM, - .DirPredictionWrongM, .BTBPredPCWrongM, .RASPredPCWrongM, .PredictionInstrClassWrongM, - .InstrClassM, .DCacheMiss, .DCacheAccess, .ICacheMiss, .ICacheAccess, - .CSRAdrM, .PrivilegeModeW, .CSRWriteValM, - .MCOUNTINHIBIT_REGW, .MCOUNTEREN_REGW, .SCOUNTEREN_REGW, - .MTIME_CLINT, .CSRCReadValM, .IllegalCSRCAccessM); + .WriteMSTATUSM, .WriteMSTATUSHM, .WriteSSTATUSM, + .TrapM, .FRegWriteM, .NextPrivilegeModeM, .PrivilegeModeW, + .mretM, .sretM, .WriteFRMM, .WriteFFLAGSM, .CSRWriteValM, .SelHPTW, + .MSTATUS_REGW, .SSTATUS_REGW, .MSTATUSH_REGW, + .STATUS_MPP, .STATUS_SPP, .STATUS_TSR, .STATUS_TW, + .STATUS_MIE, .STATUS_SIE, .STATUS_MXR, .STATUS_SUM, .STATUS_MPRV, .STATUS_TVM, + .STATUS_FS, .BigEndianM); csrm csrm(.clk, .reset, .InstrValidNotFlushedM, - .CSRMWriteM, .MTrapM, .CSRAdrM, - .NextEPCM, .NextCauseM, .NextMtvalM, .MSTATUS_REGW, .MSTATUSH_REGW, - .CSRWriteValM, .CSRMReadValM, .MTVEC_REGW, - .MEPC_REGW, .MCOUNTEREN_REGW, .MCOUNTINHIBIT_REGW, - .MEDELEG_REGW, .MIDELEG_REGW,.PMPCFG_ARRAY_REGW, .PMPADDR_ARRAY_REGW, - .MIP_REGW, .MIE_REGW, .WriteMSTATUSM, .WriteMSTATUSHM, - .IllegalCSRMAccessM, .IllegalCSRMWriteReadonlyM); + .CSRMWriteM, .MTrapM, .CSRAdrM, + .NextEPCM, .NextCauseM, .NextMtvalM, .MSTATUS_REGW, .MSTATUSH_REGW, + .CSRWriteValM, .CSRMReadValM, .MTVEC_REGW, + .MEPC_REGW, .MCOUNTEREN_REGW, .MCOUNTINHIBIT_REGW, + .MEDELEG_REGW, .MIDELEG_REGW,.PMPCFG_ARRAY_REGW, .PMPADDR_ARRAY_REGW, + .MIP_REGW, .MIE_REGW, .WriteMSTATUSM, .WriteMSTATUSHM, + .IllegalCSRMAccessM, .IllegalCSRMWriteReadonlyM); csrs csrs(.clk, .reset, .InstrValidNotFlushedM, - .CSRSWriteM, .STrapM, .CSRAdrM, - .NextEPCM, .NextCauseM, .NextMtvalM, .SSTATUS_REGW, - .STATUS_TVM, .CSRWriteValM, .PrivilegeModeW, - .CSRSReadValM, .STVEC_REGW, .SEPC_REGW, - .SCOUNTEREN_REGW, - .SATP_REGW, .MIP_REGW, .MIE_REGW, .MIDELEG_REGW, - .WriteSSTATUSM, .IllegalCSRSAccessM); + .CSRSWriteM, .STrapM, .CSRAdrM, + .NextEPCM, .NextCauseM, .NextMtvalM, .SSTATUS_REGW, + .STATUS_TVM, .CSRWriteValM, .PrivilegeModeW, + .CSRSReadValM, .STVEC_REGW, .SEPC_REGW, + .SCOUNTEREN_REGW, + .SATP_REGW, .MIP_REGW, .MIE_REGW, .MIDELEG_REGW, + .WriteSSTATUSM, .IllegalCSRSAccessM); csru csru(.clk, .reset, .InstrValidNotFlushedM, - .CSRUWriteM, .CSRAdrM, .CSRWriteValM, .STATUS_FS, .CSRUReadValM, - .SetFflagsM, .FRM_REGW, .WriteFRMM, .WriteFFLAGSM, - .IllegalCSRUAccessM); + .CSRUWriteM, .CSRAdrM, .CSRWriteValM, .STATUS_FS, .CSRUReadValM, + .SetFflagsM, .FRM_REGW, .WriteFRMM, .WriteFFLAGSM, + .IllegalCSRUAccessM); + if (`ZICOUNTERS_SUPPORTED) begin:counters + csrc counters(.clk, .reset, .StallE, .StallM, .FlushM, + .InstrValidNotFlushedM, .LoadStallD, .CSRMWriteM, + .DirPredictionWrongM, .BTBPredPCWrongM, .RASPredPCWrongM, .PredictionInstrClassWrongM, + .InstrClassM, .DCacheMiss, .DCacheAccess, .ICacheMiss, .ICacheAccess, + .CSRAdrM, .PrivilegeModeW, .CSRWriteValM, + .MCOUNTINHIBIT_REGW, .MCOUNTEREN_REGW, .SCOUNTEREN_REGW, + .MTIME_CLINT, .CSRCReadValM, .IllegalCSRCAccessM); + end else begin + assign CSRCReadValM = 0; + assign IllegalCSRCAccessM = 1; // counters aren't enabled + end // merge CSR Reads assign CSRReadValM = CSRUReadValM | CSRSReadValM | CSRMReadValM | CSRCReadValM; diff --git a/pipelined/src/privileged/csrc.sv b/pipelined/src/privileged/csrc.sv index e5032898b..b339de20d 100644 --- a/pipelined/src/privileged/csrc.sv +++ b/pipelined/src/privileged/csrc.sv @@ -39,127 +39,122 @@ module csrc #(parameter TIME = 12'hC01, TIMEH = 12'hC81 ) ( - input logic clk, reset, - input logic StallE, StallM, - input logic FlushM, - input logic InstrValidNotFlushedM, LoadStallD, CSRMWriteM, - input logic DirPredictionWrongM, - input logic BTBPredPCWrongM, - input logic RASPredPCWrongM, - input logic PredictionInstrClassWrongM, - input logic [3:0] InstrClassM, - input logic DCacheMiss, - input logic DCacheAccess, - input logic ICacheMiss, - input logic ICacheAccess, - input logic [11:0] CSRAdrM, - input logic [1:0] PrivilegeModeW, - input logic [`XLEN-1:0] CSRWriteValM, - input logic [31:0] MCOUNTINHIBIT_REGW, MCOUNTEREN_REGW, SCOUNTEREN_REGW, - input logic [63:0] MTIME_CLINT, - output logic [`XLEN-1:0] CSRCReadValM, - output logic IllegalCSRCAccessM + input logic clk, reset, + input logic StallE, StallM, + input logic FlushM, + input logic InstrValidNotFlushedM, LoadStallD, CSRMWriteM, + input logic DirPredictionWrongM, + input logic BTBPredPCWrongM, + input logic RASPredPCWrongM, + input logic PredictionInstrClassWrongM, + input logic [3:0] InstrClassM, + input logic DCacheMiss, + input logic DCacheAccess, + input logic ICacheMiss, + input logic ICacheAccess, + input logic [11:0] CSRAdrM, + input logic [1:0] PrivilegeModeW, + input logic [`XLEN-1:0] CSRWriteValM, + input logic [31:0] MCOUNTINHIBIT_REGW, MCOUNTEREN_REGW, SCOUNTEREN_REGW, + input logic [63:0] MTIME_CLINT, + output logic [`XLEN-1:0] CSRCReadValM, + output logic IllegalCSRCAccessM ); - if (`ZICOUNTERS_SUPPORTED) begin:counters - logic [4:0] CounterNumM; - (* mark_debug = "true" *) logic [`XLEN-1:0] HPMCOUNTER_REGW[`COUNTERS-1:0]; - logic [`XLEN-1:0] HPMCOUNTERH_REGW[`COUNTERS-1:0]; - logic LoadStallE, LoadStallM; - logic [`COUNTERS-1:0] WriteHPMCOUNTERM; - logic [`COUNTERS-1:0] CounterEvent; - logic [63:0] HPMCOUNTERPlusM[`COUNTERS-1:0]; - logic [`XLEN-1:0] NextHPMCOUNTERM[`COUNTERS-1:0]; - genvar i; + logic [4:0] CounterNumM; + (* mark_debug = "true" *) logic [`XLEN-1:0] HPMCOUNTER_REGW[`COUNTERS-1:0]; + logic [`XLEN-1:0] HPMCOUNTERH_REGW[`COUNTERS-1:0]; + logic LoadStallE, LoadStallM; + logic [`COUNTERS-1:0] WriteHPMCOUNTERM; + logic [`COUNTERS-1:0] CounterEvent; + logic [63:0] HPMCOUNTERPlusM[`COUNTERS-1:0]; + logic [`XLEN-1:0] NextHPMCOUNTERM[`COUNTERS-1:0]; + genvar i; - // Interface signals - flopenrc #(1) LoadStallEReg(.clk, .reset, .clear(1'b0), .en(~StallE), .d(LoadStallD), .q(LoadStallE)); // don't flush the load stall during a load stall. - flopenrc #(1) LoadStallMReg(.clk, .reset, .clear(FlushM), .en(~StallM), .d(LoadStallE), .q(LoadStallM)); - - // Determine when to increment each counter - assign CounterEvent[0] = 1'b1; // MCYCLE always increments - assign CounterEvent[1] = 1'b0; // Counter 1 doesn't exist - assign CounterEvent[2] = InstrValidNotFlushedM; - if(`QEMU) begin: cevent // No other performance counters in QEMU - assign CounterEvent[`COUNTERS-1:3] = 0; - end else begin: cevent // User-defined counters - assign CounterEvent[3] = LoadStallM; // don't want to suppress on flush as this only happens if flushed. - assign CounterEvent[4] = DirPredictionWrongM & InstrValidNotFlushedM; - assign CounterEvent[5] = InstrClassM[0] & InstrValidNotFlushedM; - assign CounterEvent[6] = BTBPredPCWrongM & InstrValidNotFlushedM; - assign CounterEvent[7] = (InstrClassM[3] | InstrClassM[1]) & InstrValidNotFlushedM; - assign CounterEvent[8] = RASPredPCWrongM & InstrValidNotFlushedM; - assign CounterEvent[9] = InstrClassM[2] & InstrValidNotFlushedM; - assign CounterEvent[10] = PredictionInstrClassWrongM & InstrValidNotFlushedM; - assign CounterEvent[11] = DCacheAccess; - assign CounterEvent[12] = DCacheMiss; - assign CounterEvent[13] = ICacheAccess; - assign CounterEvent[14] = ICacheMiss; - assign CounterEvent[`COUNTERS-1:15] = 0; // eventually give these sources, including FP instructions, I$/D$ misses, branches and mispredictions - end - - // Counter update and write logic - for (i = 0; i < `COUNTERS; i = i+1) begin:cntr - assign WriteHPMCOUNTERM[i] = CSRMWriteM & (CSRAdrM == MHPMCOUNTERBASE + i); - assign NextHPMCOUNTERM[i][`XLEN-1:0] = WriteHPMCOUNTERM[i] ? CSRWriteValM : HPMCOUNTERPlusM[i][`XLEN-1:0]; - always_ff @(posedge clk) //, posedge reset) // ModelSim doesn't like syntax of passing array element to flop - if (reset) HPMCOUNTER_REGW[i][`XLEN-1:0] <= #1 0; - else HPMCOUNTER_REGW[i][`XLEN-1:0] <= #1 NextHPMCOUNTERM[i]; - - if (`XLEN==32) begin // write high and low separately - logic [`COUNTERS-1:0] WriteHPMCOUNTERHM; - logic [`XLEN-1:0] NextHPMCOUNTERHM[`COUNTERS-1:0]; - assign HPMCOUNTERPlusM[i] = {HPMCOUNTERH_REGW[i], HPMCOUNTER_REGW[i]} + {63'b0, CounterEvent[i] & ~MCOUNTINHIBIT_REGW[i]}; - assign WriteHPMCOUNTERHM[i] = CSRMWriteM & (CSRAdrM == MHPMCOUNTERHBASE + i); - assign NextHPMCOUNTERHM[i] = WriteHPMCOUNTERHM[i] ? CSRWriteValM : HPMCOUNTERPlusM[i][63:32]; - always_ff @(posedge clk) //, posedge reset) // ModelSim doesn't like syntax of passing array element to flop - if (reset) HPMCOUNTERH_REGW[i][`XLEN-1:0] <= #1 0; - else HPMCOUNTERH_REGW[i][`XLEN-1:0] <= #1 NextHPMCOUNTERHM[i]; - end else begin // XLEN=64; write entire register - assign HPMCOUNTERPlusM[i] = HPMCOUNTER_REGW[i] + {63'b0, CounterEvent[i] & ~MCOUNTINHIBIT_REGW[i]}; - end - end - - // Read Counters, or cause excepiton if insufficient privilege in light of COUNTEREN flags - assign CounterNumM = CSRAdrM[4:0]; // which counter to read? - always_comb - if (PrivilegeModeW == `M_MODE | - MCOUNTEREN_REGW[CounterNumM] & (!`S_SUPPORTED | PrivilegeModeW == `S_MODE | SCOUNTEREN_REGW[CounterNumM])) begin - IllegalCSRCAccessM = 0; - if (`XLEN==64) begin // 64-bit counter reads - // Veri lator doesn't realize this only occurs for XLEN=64 - /* verilator lint_off WIDTH */ - if (CSRAdrM == TIME) CSRCReadValM = MTIME_CLINT; // TIME register is a shadow of the memory-mapped MTIME from the CLINT - /* verilator lint_on WIDTH */ - else if (CSRAdrM >= MHPMCOUNTERBASE & CSRAdrM < MHPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; - else if (CSRAdrM >= HPMCOUNTERBASE & CSRAdrM < HPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; - else begin - CSRCReadValM = 0; - IllegalCSRCAccessM = 1; // requested CSR doesn't exist - end - end else begin // 32-bit counter reads - // Veri lator doesn't realize this only occurs for XLEN=32 - /* verilator lint_off WIDTH */ - if (CSRAdrM == TIME) CSRCReadValM = MTIME_CLINT[31:0];// TIME register is a shadow of the memory-mapped MTIME from the CLINT - else if (CSRAdrM == TIMEH) CSRCReadValM = MTIME_CLINT[63:32]; - /* verilator lint_on WIDTH */ - else if (CSRAdrM >= MHPMCOUNTERBASE & CSRAdrM < MHPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; - else if (CSRAdrM >= HPMCOUNTERBASE & CSRAdrM < HPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; - else if (CSRAdrM >= MHPMCOUNTERHBASE & CSRAdrM < MHPMCOUNTERHBASE+`COUNTERS) CSRCReadValM = HPMCOUNTERH_REGW[CounterNumM]; - else if (CSRAdrM >= HPMCOUNTERHBASE & CSRAdrM < HPMCOUNTERHBASE+`COUNTERS) CSRCReadValM = HPMCOUNTERH_REGW[CounterNumM]; - else begin - CSRCReadValM = 0; - IllegalCSRCAccessM = 1; // requested CSR doesn't exist - end - end - end else begin - CSRCReadValM = 0; - IllegalCSRCAccessM = 1; // no privileges for this csr - end - end else begin - assign CSRCReadValM = 0; - assign IllegalCSRCAccessM = 1; // counters aren't enabled + // Interface signals + flopenrc #(1) LoadStallEReg(.clk, .reset, .clear(1'b0), .en(~StallE), .d(LoadStallD), .q(LoadStallE)); // don't flush the load stall during a load stall. + flopenrc #(1) LoadStallMReg(.clk, .reset, .clear(FlushM), .en(~StallM), .d(LoadStallE), .q(LoadStallM)); + + // Determine when to increment each counter + assign CounterEvent[0] = 1'b1; // MCYCLE always increments + assign CounterEvent[1] = 1'b0; // Counter 1 doesn't exist + assign CounterEvent[2] = InstrValidNotFlushedM; + if(`QEMU) begin: cevent // No other performance counters in QEMU + assign CounterEvent[`COUNTERS-1:3] = 0; + end else begin: cevent // User-defined counters + assign CounterEvent[3] = LoadStallM; // don't want to suppress on flush as this only happens if flushed. + assign CounterEvent[4] = DirPredictionWrongM & InstrValidNotFlushedM; + assign CounterEvent[5] = InstrClassM[0] & InstrValidNotFlushedM; + assign CounterEvent[6] = BTBPredPCWrongM & InstrValidNotFlushedM; + assign CounterEvent[7] = (InstrClassM[3] | InstrClassM[1]) & InstrValidNotFlushedM; + assign CounterEvent[8] = RASPredPCWrongM & InstrValidNotFlushedM; + assign CounterEvent[9] = InstrClassM[2] & InstrValidNotFlushedM; + assign CounterEvent[10] = PredictionInstrClassWrongM & InstrValidNotFlushedM; + assign CounterEvent[11] = DCacheAccess; + assign CounterEvent[12] = DCacheMiss; + assign CounterEvent[13] = ICacheAccess; + assign CounterEvent[14] = ICacheMiss; + assign CounterEvent[`COUNTERS-1:15] = 0; // eventually give these sources, including FP instructions, I$/D$ misses, branches and mispredictions end + + // Counter update and write logic + for (i = 0; i < `COUNTERS; i = i+1) begin:cntr + assign WriteHPMCOUNTERM[i] = CSRMWriteM & (CSRAdrM == MHPMCOUNTERBASE + i); + assign NextHPMCOUNTERM[i][`XLEN-1:0] = WriteHPMCOUNTERM[i] ? CSRWriteValM : HPMCOUNTERPlusM[i][`XLEN-1:0]; + always_ff @(posedge clk) //, posedge reset) // ModelSim doesn't like syntax of passing array element to flop + if (reset) HPMCOUNTER_REGW[i][`XLEN-1:0] <= #1 0; + else HPMCOUNTER_REGW[i][`XLEN-1:0] <= #1 NextHPMCOUNTERM[i]; + + if (`XLEN==32) begin // write high and low separately + logic [`COUNTERS-1:0] WriteHPMCOUNTERHM; + logic [`XLEN-1:0] NextHPMCOUNTERHM[`COUNTERS-1:0]; + assign HPMCOUNTERPlusM[i] = {HPMCOUNTERH_REGW[i], HPMCOUNTER_REGW[i]} + {63'b0, CounterEvent[i] & ~MCOUNTINHIBIT_REGW[i]}; + assign WriteHPMCOUNTERHM[i] = CSRMWriteM & (CSRAdrM == MHPMCOUNTERHBASE + i); + assign NextHPMCOUNTERHM[i] = WriteHPMCOUNTERHM[i] ? CSRWriteValM : HPMCOUNTERPlusM[i][63:32]; + always_ff @(posedge clk) //, posedge reset) // ModelSim doesn't like syntax of passing array element to flop + if (reset) HPMCOUNTERH_REGW[i][`XLEN-1:0] <= #1 0; + else HPMCOUNTERH_REGW[i][`XLEN-1:0] <= #1 NextHPMCOUNTERHM[i]; + end else begin // XLEN=64; write entire register + assign HPMCOUNTERPlusM[i] = HPMCOUNTER_REGW[i] + {63'b0, CounterEvent[i] & ~MCOUNTINHIBIT_REGW[i]}; + end + end + + // Read Counters, or cause excepiton if insufficient privilege in light of COUNTEREN flags + assign CounterNumM = CSRAdrM[4:0]; // which counter to read? + always_comb + if (PrivilegeModeW == `M_MODE | + MCOUNTEREN_REGW[CounterNumM] & (!`S_SUPPORTED | PrivilegeModeW == `S_MODE | SCOUNTEREN_REGW[CounterNumM])) begin + IllegalCSRCAccessM = 0; + if (`XLEN==64) begin // 64-bit counter reads + // Veri lator doesn't realize this only occurs for XLEN=64 + /* verilator lint_off WIDTH */ + if (CSRAdrM == TIME) CSRCReadValM = MTIME_CLINT; // TIME register is a shadow of the memory-mapped MTIME from the CLINT + /* verilator lint_on WIDTH */ + else if (CSRAdrM >= MHPMCOUNTERBASE & CSRAdrM < MHPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; + else if (CSRAdrM >= HPMCOUNTERBASE & CSRAdrM < HPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; + else begin + CSRCReadValM = 0; + IllegalCSRCAccessM = 1; // requested CSR doesn't exist + end + end else begin // 32-bit counter reads + // Veri lator doesn't realize this only occurs for XLEN=32 + /* verilator lint_off WIDTH */ + if (CSRAdrM == TIME) CSRCReadValM = MTIME_CLINT[31:0];// TIME register is a shadow of the memory-mapped MTIME from the CLINT + else if (CSRAdrM == TIMEH) CSRCReadValM = MTIME_CLINT[63:32]; + /* verilator lint_on WIDTH */ + else if (CSRAdrM >= MHPMCOUNTERBASE & CSRAdrM < MHPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; + else if (CSRAdrM >= HPMCOUNTERBASE & CSRAdrM < HPMCOUNTERBASE+`COUNTERS) CSRCReadValM = HPMCOUNTER_REGW[CounterNumM]; + else if (CSRAdrM >= MHPMCOUNTERHBASE & CSRAdrM < MHPMCOUNTERHBASE+`COUNTERS) CSRCReadValM = HPMCOUNTERH_REGW[CounterNumM]; + else if (CSRAdrM >= HPMCOUNTERHBASE & CSRAdrM < HPMCOUNTERHBASE+`COUNTERS) CSRCReadValM = HPMCOUNTERH_REGW[CounterNumM]; + else begin + CSRCReadValM = 0; + IllegalCSRCAccessM = 1; // requested CSR doesn't exist + end + end + end else begin + CSRCReadValM = 0; + IllegalCSRCAccessM = 1; // no privileges for this csr + end endmodule // To Do: