From 3678ab556cd82a5410c80cb83d7f449c1800ef39 Mon Sep 17 00:00:00 2001 From: David Harris Date: Fri, 3 Mar 2023 16:46:16 -0800 Subject: [PATCH 1/5] Removed unneeded diagnostic print --- testbench/common/riscvassertions.sv | 1 - 1 file changed, 1 deletion(-) diff --git a/testbench/common/riscvassertions.sv b/testbench/common/riscvassertions.sv index f733aac58..b3bc8f96f 100644 --- a/testbench/common/riscvassertions.sv +++ b/testbench/common/riscvassertions.sv @@ -23,7 +23,6 @@ module riscvassertions; initial begin - $display("IDIV_ON_FPU = %b M_SUPPORTED %b comb %b\n", `IDIV_ON_FPU, `M_SUPPORTED, ((`IDIV_ON_FPU) || (!`M_SUPPORTED))); assert (`PMP_ENTRIES == 0 || `PMP_ENTRIES==16 || `PMP_ENTRIES==64) else $error("Illegal number of PMP entries: PMP_ENTRIES must be 0, 16, or 64"); assert (`S_SUPPORTED || `VIRTMEM_SUPPORTED == 0) else $error("Virtual memory requires S mode support"); assert (`IDIV_BITSPERCYCLE == 1 || `IDIV_BITSPERCYCLE==2 || `IDIV_BITSPERCYCLE==4) else $error("Illegal number of divider bits/cycle: IDIV_BITSPERCYCLE must be 1, 2, or 4"); From a56557d847e3e280adf666cc271b4b24fb3702a5 Mon Sep 17 00:00:00 2001 From: David Harris Date: Mon, 6 Mar 2023 11:02:42 -0800 Subject: [PATCH 2/5] Improved decoding illegal instructions in controller --- src/ieu/controller.sv | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/ieu/controller.sv b/src/ieu/controller.sv index 8e89656a1..52b7f6401 100644 --- a/src/ieu/controller.sv +++ b/src/ieu/controller.sv @@ -113,7 +113,7 @@ module controller( logic FenceD, FenceE; // Fence instruction logic SFenceVmaD; // sfence.vma instruction logic IntDivM; // Integer divide instruction - + logic IFunctD, RFunctD, MFunctD; // Detect I, R, and M-type RV32IM/Rv64IM instructions // Extract fields assign OpD = InstrD[6:0]; @@ -121,6 +121,26 @@ module controller( assign Funct7D = InstrD[31:25]; assign Rs1D = InstrD[19:15]; + // Funct 7 checking + // Be rigorous about detecting illegal instructions if CSRs or bit manipulation is supported + // otherwise be cheap + + if (`ZICSR_SUPPORTED | `ZBA_SUPPORTED | `ZBB_SUPPORTED | `ZBC_SUPPORTED | `ZBS_SUPPORTED) begin // Exact integer decoding + logic Funct7ZeroD, Funct7b5D, IShiftD, INoShiftD; + + assign Funct7ZeroD = (Funct7D == 7'b0000000); // most R-type instructions + assign Funct7b5D = (Funct7D == 7'b0100000); // srai, sub + assign IShiftD = (Funct3D == 3'b001 & Funct7ZeroD) | (Funct3D == 3'b101 & (Funct7ZeroD | Funct7b5D)); // slli, srli, srai, or w forms + assign INoShiftD = (Funct3D != 3'b001 & Funct3D != 3'b101); + assign IFunctD = IShiftD | INoShiftD; + assign RFunctD = ((Funct3D == 3'b000 | Funct3D == 3'b101) & Funct7b5D) | Funct7ZeroD; + assign MFunctD = (Funct7D == 7'b0000001) & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv + end else begin + assign IFunctD = 1; // Don't bother to separate out shift decoding + assign RFunctD = ~Funct7D[0]; // Not a multiply + assign MFunctD = Funct7D[0] & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv + end + // Main Instruction Decoder always_comb case(OpD) @@ -132,9 +152,12 @@ module controller( ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_1_0_00_0; // fence else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_0; // fence treated as nop - 7'b0010011: ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_0_0_0_0_0_00_0; // I-type ALU + 7'b0010011: if (IFunctD) + ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_0_0_0_0_0_00_0; // I-type ALU + else + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0010111: ControlsD = `CTRLW'b1_100_11_00_000_0_0_0_0_0_0_0_0_0_00_0; // auipc - 7'b0011011: if (`XLEN == 64) + 7'b0011011: if (IFunctD & `XLEN == 64) ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_1_0_0_0_0_00_0; // IW-type ALU for RV64i else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction @@ -149,16 +172,16 @@ module controller( ControlsD = `CTRLW'b1_101_01_11_001_0_0_0_0_0_0_0_0_0_10_0; // amo end else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction - 7'b0110011: if (Funct7D == 7'b0000000 | Funct7D == 7'b0100000) + 7'b0110011: if (RFunctD) ControlsD = `CTRLW'b1_000_00_00_000_0_1_0_0_0_0_0_0_0_00_0; // R-type - else if (Funct7D == 7'b0000001 & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2]))) + else if (MFunctD) ControlsD = `CTRLW'b1_000_00_00_011_0_0_0_0_0_0_0_0_1_00_0; // Multiply/divide else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0110111: ControlsD = `CTRLW'b1_100_01_00_000_0_0_0_1_0_0_0_0_0_00_0; // lui - 7'b0111011: if ((Funct7D == 7'b0000000 | Funct7D == 7'b0100000) & `XLEN == 64) + 7'b0111011: if (RFunctD & (`XLEN == 64)) ControlsD = `CTRLW'b1_000_00_00_000_0_1_0_0_1_0_0_0_0_00_0; // R-type W instructions for RV64i - else if (Funct7D == 7'b0000001 & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])) & `XLEN == 64) + else if (MFunctD & (`XLEN == 64)) ControlsD = `CTRLW'b1_000_00_00_011_0_0_0_0_1_0_0_0_1_00_0; // W-type Multiply/Divide else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction From c2efdbdbbb260ca599ba016493dc3cf952ac7eba Mon Sep 17 00:00:00 2001 From: David Harris Date: Mon, 6 Mar 2023 11:15:48 -0800 Subject: [PATCH 3/5] More detailed decoding of load/store/branch/jump --- src/ieu/controller.sv | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/ieu/controller.sv b/src/ieu/controller.sv index 52b7f6401..25a634a27 100644 --- a/src/ieu/controller.sv +++ b/src/ieu/controller.sv @@ -114,6 +114,8 @@ module controller( logic SFenceVmaD; // sfence.vma instruction logic IntDivM; // Integer divide instruction logic IFunctD, RFunctD, MFunctD; // Detect I, R, and M-type RV32IM/Rv64IM instructions + logic LFunctD, SFunctD, BFunctD; // Detect load, store, branch instructions + logic JFunctD; // detect jalr instruction // Extract fields assign OpD = InstrD[6:0]; @@ -135,10 +137,20 @@ module controller( assign IFunctD = IShiftD | INoShiftD; assign RFunctD = ((Funct3D == 3'b000 | Funct3D == 3'b101) & Funct7b5D) | Funct7ZeroD; assign MFunctD = (Funct7D == 7'b0000001) & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv + assign LFunctD = Funct3D == 3'b000 | Funct3D == 3'b001 | Funct3D == 3'b010 | Funct3D == 3'b100 | Funct3D == 3'b101 | + ((`XLEN == 64) & (Funct3D == 3'b011 | Funct3D == 3'b110)); + assign SFunctD = Funct3D == 3'b000 | Funct3D == 3'b001 | Funct3D == 3'b010 | + ((`XLEN == 64) & (Funct3D == 3'b011)); + assign BFunctD = (Funct3D[2:1] != 2'b01); // legal branches + assign JFunctD = (Funct3D == 3'b000); end else begin assign IFunctD = 1; // Don't bother to separate out shift decoding assign RFunctD = ~Funct7D[0]; // Not a multiply assign MFunctD = Funct7D[0] & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv + assign LFunctD = 1; // don't bother to check Funct3 for loads + assign SFunctD = 1; // don't bother to check Funct3 for stores + assign BFunctD = 1; // don't bother to check Funct3 for branches + assign JFunctD = 1; // don't bother to check Funct3 for jumps end // Main Instruction Decoder @@ -146,7 +158,10 @@ module controller( case(OpD) // RegWrite_ImmSrc_ALUSrc_MemRW_ResultSrc_Branch_ALUOp_Jump_ALUResultSrc_W64_CSRRead_Privileged_Fence_MDU_Atomic_Illegal 7'b0000000: ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Illegal instruction - 7'b0000011: ControlsD = `CTRLW'b1_000_01_10_001_0_0_0_0_0_0_0_0_0_00_0; // lw + 7'b0000011: if (LFunctD) + ControlsD = `CTRLW'b1_000_01_10_001_0_0_0_0_0_0_0_0_0_00_0; // loads + else + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0000111: ControlsD = `CTRLW'b0_000_01_10_001_0_0_0_0_0_0_0_0_0_00_1; // flw - only legal if FP supported 7'b0001111: if (`ZIFENCEI_SUPPORTED) ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_1_0_00_0; // fence @@ -161,7 +176,10 @@ module controller( ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_1_0_0_0_0_00_0; // IW-type ALU for RV64i else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction - 7'b0100011: ControlsD = `CTRLW'b0_001_01_01_000_0_0_0_0_0_0_0_0_0_00_0; // sw + 7'b0100011: if (SFunctD) + ControlsD = `CTRLW'b0_001_01_01_000_0_0_0_0_0_0_0_0_0_00_0; // stores + else + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0100111: ControlsD = `CTRLW'b0_001_01_01_000_0_0_0_0_0_0_0_0_0_00_1; // fsw - only legal if FP supported 7'b0101111: if (`A_SUPPORTED) begin if (InstrD[31:27] == 5'b00010) @@ -185,8 +203,14 @@ module controller( ControlsD = `CTRLW'b1_000_00_00_011_0_0_0_0_1_0_0_0_1_00_0; // W-type Multiply/Divide else ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction - 7'b1100011: ControlsD = `CTRLW'b0_010_11_00_000_1_0_0_0_0_0_0_0_0_00_0; // branches - 7'b1100111: ControlsD = `CTRLW'b1_000_01_00_000_0_0_1_1_0_0_0_0_0_00_0; // jalr + 7'b1100011: if (BFunctD) + ControlsD = `CTRLW'b0_010_11_00_000_1_0_0_0_0_0_0_0_0_00_0; // branches + else + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction + 7'b1100111: if (JFunctD) + ControlsD = `CTRLW'b1_000_01_00_000_0_0_1_1_0_0_0_0_0_00_0; // jalr + else + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b1101111: ControlsD = `CTRLW'b1_011_11_00_000_0_0_1_1_0_0_0_0_0_00_0; // jal 7'b1110011: if (`ZICSR_SUPPORTED) begin if (Funct3D == 3'b000) From 7e0c96cdcc5be28c28df51464dc142bf49673f89 Mon Sep 17 00:00:00 2001 From: David Harris Date: Mon, 6 Mar 2023 11:21:11 -0800 Subject: [PATCH 4/5] Simplified decoder default to illegal instruction --- src/ieu/controller.sv | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/ieu/controller.sv b/src/ieu/controller.sv index 25a634a27..783e28a95 100644 --- a/src/ieu/controller.sv +++ b/src/ieu/controller.sv @@ -154,14 +154,13 @@ module controller( end // Main Instruction Decoder - always_comb + /* verilator lint_off CASEINCOMPLETE */ + always_comb begin + ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // default: Illegal instruction case(OpD) // RegWrite_ImmSrc_ALUSrc_MemRW_ResultSrc_Branch_ALUOp_Jump_ALUResultSrc_W64_CSRRead_Privileged_Fence_MDU_Atomic_Illegal - 7'b0000000: ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Illegal instruction - 7'b0000011: if (LFunctD) + 7'b0000011: if (LFunctD) ControlsD = `CTRLW'b1_000_01_10_001_0_0_0_0_0_0_0_0_0_00_0; // loads - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0000111: ControlsD = `CTRLW'b0_000_01_10_001_0_0_0_0_0_0_0_0_0_00_1; // flw - only legal if FP supported 7'b0001111: if (`ZIFENCEI_SUPPORTED) ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_1_0_00_0; // fence @@ -169,17 +168,11 @@ module controller( ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_0; // fence treated as nop 7'b0010011: if (IFunctD) ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_0_0_0_0_0_00_0; // I-type ALU - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0010111: ControlsD = `CTRLW'b1_100_11_00_000_0_0_0_0_0_0_0_0_0_00_0; // auipc 7'b0011011: if (IFunctD & `XLEN == 64) ControlsD = `CTRLW'b1_000_01_00_000_0_1_0_0_1_0_0_0_0_00_0; // IW-type ALU for RV64i - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0100011: if (SFunctD) ControlsD = `CTRLW'b0_001_01_01_000_0_0_0_0_0_0_0_0_0_00_0; // stores - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0100111: ControlsD = `CTRLW'b0_001_01_01_000_0_0_0_0_0_0_0_0_0_00_1; // fsw - only legal if FP supported 7'b0101111: if (`A_SUPPORTED) begin if (InstrD[31:27] == 5'b00010) @@ -188,39 +181,30 @@ module controller( ControlsD = `CTRLW'b1_101_01_01_100_0_0_0_0_0_0_0_0_0_01_0; // sc else ControlsD = `CTRLW'b1_101_01_11_001_0_0_0_0_0_0_0_0_0_10_0; // amo - end else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction + end 7'b0110011: if (RFunctD) ControlsD = `CTRLW'b1_000_00_00_000_0_1_0_0_0_0_0_0_0_00_0; // R-type else if (MFunctD) ControlsD = `CTRLW'b1_000_00_00_011_0_0_0_0_0_0_0_0_1_00_0; // Multiply/divide - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b0110111: ControlsD = `CTRLW'b1_100_01_00_000_0_0_0_1_0_0_0_0_0_00_0; // lui 7'b0111011: if (RFunctD & (`XLEN == 64)) ControlsD = `CTRLW'b1_000_00_00_000_0_1_0_0_1_0_0_0_0_00_0; // R-type W instructions for RV64i else if (MFunctD & (`XLEN == 64)) ControlsD = `CTRLW'b1_000_00_00_011_0_0_0_0_1_0_0_0_1_00_0; // W-type Multiply/Divide - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b1100011: if (BFunctD) ControlsD = `CTRLW'b0_010_11_00_000_1_0_0_0_0_0_0_0_0_00_0; // branches - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b1100111: if (JFunctD) ControlsD = `CTRLW'b1_000_01_00_000_0_0_1_1_0_0_0_0_0_00_0; // jalr - else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // Non-implemented instruction 7'b1101111: ControlsD = `CTRLW'b1_011_11_00_000_0_0_1_1_0_0_0_0_0_00_0; // jal 7'b1110011: if (`ZICSR_SUPPORTED) begin if (Funct3D == 3'b000) ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_1_0_0_00_0; // privileged; decoded further in priveleged modules else ControlsD = `CTRLW'b1_000_00_00_010_0_0_0_0_0_1_0_0_0_00_0; // csrs - end else - ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // non-implemented instruction - default: ControlsD = `CTRLW'b0_000_00_00_000_0_0_0_0_0_0_0_0_0_00_1; // non-implemented instruction + end endcase + end + /* verilator lint_on CASEINCOMPLETE */ // Unswizzle control bits // Squash control signals if coming from an illegal compressed instruction From 7ecf4cdea886297c83b177820581825582628c5e Mon Sep 17 00:00:00 2001 From: David Harris Date: Mon, 6 Mar 2023 13:10:51 -0800 Subject: [PATCH 5/5] Fixed bug about rv64 shifts only using 6 bits of funct7 --- src/ieu/controller.sv | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ieu/controller.sv b/src/ieu/controller.sv index 783e28a95..876fdaa1a 100644 --- a/src/ieu/controller.sv +++ b/src/ieu/controller.sv @@ -127,13 +127,16 @@ module controller( // Be rigorous about detecting illegal instructions if CSRs or bit manipulation is supported // otherwise be cheap - if (`ZICSR_SUPPORTED | `ZBA_SUPPORTED | `ZBB_SUPPORTED | `ZBC_SUPPORTED | `ZBS_SUPPORTED) begin // Exact integer decoding + if (`ZICSR_SUPPORTED | `ZBA_SUPPORTED | `ZBB_SUPPORTED | `ZBC_SUPPORTED | `ZBS_SUPPORTED) begin:legalcheck // Exact integer decoding logic Funct7ZeroD, Funct7b5D, IShiftD, INoShiftD; + logic Funct7ShiftZeroD, Funct7Shiftb5D; assign Funct7ZeroD = (Funct7D == 7'b0000000); // most R-type instructions assign Funct7b5D = (Funct7D == 7'b0100000); // srai, sub - assign IShiftD = (Funct3D == 3'b001 & Funct7ZeroD) | (Funct3D == 3'b101 & (Funct7ZeroD | Funct7b5D)); // slli, srli, srai, or w forms - assign INoShiftD = (Funct3D != 3'b001 & Funct3D != 3'b101); + assign Funct7ShiftZeroD = (`XLEN==64) ? (Funct7D[6:1] == 6'b000000) : Funct7ZeroD; + assign Funct7Shiftb5D = (`XLEN==64) ? (Funct7D[6:1] == 6'b010000) : Funct7b5D; + assign IShiftD = (Funct3D == 3'b001 & Funct7ShiftZeroD) | (Funct3D == 3'b101 & (Funct7ShiftZeroD | Funct7Shiftb5D)); // slli, srli, srai, or w forms + assign INoShiftD = ((Funct3D != 3'b001) & (Funct3D != 3'b101)); assign IFunctD = IShiftD | INoShiftD; assign RFunctD = ((Funct3D == 3'b000 | Funct3D == 3'b101) & Funct7b5D) | Funct7ZeroD; assign MFunctD = (Funct7D == 7'b0000001) & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv @@ -143,7 +146,7 @@ module controller( ((`XLEN == 64) & (Funct3D == 3'b011)); assign BFunctD = (Funct3D[2:1] != 2'b01); // legal branches assign JFunctD = (Funct3D == 3'b000); - end else begin + end else begin:legalcheck2 assign IFunctD = 1; // Don't bother to separate out shift decoding assign RFunctD = ~Funct7D[0]; // Not a multiply assign MFunctD = Funct7D[0] & (`M_SUPPORTED | (`ZMMUL_SUPPORTED & ~Funct3D[2])); // muldiv