🐛 Remove RVC float load/store instructions (#771)

stnolting · Jan 24, 2024 · 5281589 · 5281589
2 parents 9009143 + e2c37c1
commit 5281589
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@ mimpid = 0x01040312 -> Version 01.04.03.12 -> v1.4.3.12
 
 | Date | Version | Comment | Link |
 |:----:|:-------:|:--------|:----:|
+| 23.01.2024 | 1.9.3.3 | :bug: remove compressed floating point load/store operations as they are **not** supported by `Zfinx` | [#](https://github.com/stnolting/neorv32/pull/771) |
 | 20.01.2024 | 1.9.3.2 | optimize bus switch; minor RTL and comment edits | [#769](https://github.com/stnolting/neorv32/pull/769) |
 | 14.01.2024 | 1.9.3.1 | minor rtl cleanups and optimizations | [#764](https://github.com/stnolting/neorv32/pull/764) |
 | 11.01.2024 | [**:rocket:1.9.3**](https://github.com/stnolting/neorv32/releases/tag/v1.9.3) | **New release** | |

diff --git a/rtl/core/neorv32_cpu_control.vhd b/rtl/core/neorv32_cpu_control.vhd
@@ -477,9 +477,6 @@ begin
   neorv32_cpu_decompressor_inst_true:
   if (CPU_EXTENSION_RISCV_C = true) generate
     neorv32_cpu_decompressor_inst: entity neorv32.neorv32_cpu_decompressor
-    generic map (
-      FPU_ENABLE => CPU_EXTENSION_RISCV_Zfinx -- floating-point instructions
-    )
     port map (
       ci_instr16_i => issue_engine.ci_i16, -- compressed instruction
       ci_instr32_o => issue_engine.ci_i32  -- decompressed instruction

diff --git a/rtl/core/neorv32_cpu_decompressor.vhd b/rtl/core/neorv32_cpu_decompressor.vhd
@@ -6,7 +6,7 @@
 -- # ********************************************************************************************* #
 -- # BSD 3-Clause License                                                                          #
 -- #                                                                                               #
--- # Copyright (c) 2023, Stephan Nolting. All rights reserved.                                     #
+-- # Copyright (c) 2024, Stephan Nolting. All rights reserved.                                     #
 -- #                                                                                               #
 -- # Redistribution and use in source and binary forms, with or without modification, are          #
 -- # permitted provided that the following conditions are met:                                     #
@@ -43,9 +43,6 @@ library neorv32;
 use neorv32.neorv32_package.all;
 
 entity neorv32_cpu_decompressor is
-  generic (
-    FPU_ENABLE : boolean -- floating-point instruction enabled
-  );
   port (
     ci_instr16_i : in  std_ulogic_vector(15 downto 0); -- compressed instruction
     ci_instr32_o : out std_ulogic_vector(31 downto 0)  -- decompressed instruction
@@ -125,7 +122,28 @@ begin
       when "00" => -- C0: Register-Based Loads and Stores
         case ci_instr16_i(ci_funct3_msb_c downto ci_funct3_lsb_c) is
 
-          when "010" | "011" => -- C.LW / C.FLW (integer and float are identical as the FPU implements the Zfinx ISA extension)
+          when "000" => -- Illegal_instruction, C.ADDI4SPN
+          -- ----------------------------------------------------------------------------------------------------------
+            decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_alui_c;
+            decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "00010"; -- stack pointer
+            decoded(instr_rd_msb_c downto instr_rd_lsb_c)         <= "01" & ci_instr16_i(ci_rd_3_msb_c downto ci_rd_3_lsb_c);
+            decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_subadd_c;
+            decoded(instr_imm12_msb_c downto instr_imm12_lsb_c)   <= (others => '0'); -- zero extend
+            decoded(instr_imm12_lsb_c + 0)                        <= '0';
+            decoded(instr_imm12_lsb_c + 1)                        <= '0';
+            decoded(instr_imm12_lsb_c + 2)                        <= ci_instr16_i(6);
+            decoded(instr_imm12_lsb_c + 3)                        <= ci_instr16_i(5);
+            decoded(instr_imm12_lsb_c + 4)                        <= ci_instr16_i(11);
+            decoded(instr_imm12_lsb_c + 5)                        <= ci_instr16_i(12);
+            decoded(instr_imm12_lsb_c + 6)                        <= ci_instr16_i(7);
+            decoded(instr_imm12_lsb_c + 7)                        <= ci_instr16_i(8);
+            decoded(instr_imm12_lsb_c + 8)                        <= ci_instr16_i(9);
+            decoded(instr_imm12_lsb_c + 9)                        <= ci_instr16_i(10);
+            if (ci_instr16_i(12 downto 5) = "00000000") then -- canonical illegal C instruction or C.ADDI4SPN with nzuimm = 0
+              illegal <= '1';
+            end if;
+
+          when "010" => -- C.LW
           -- ----------------------------------------------------------------------------------------------------------
             decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_load_c;
             decoded(21 downto 20)                                 <= "00";
@@ -138,11 +156,8 @@ begin
             decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_lw_c;
             decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "01" & ci_instr16_i(ci_rs1_3_msb_c downto ci_rs1_3_lsb_c); -- x8 - x15
             decoded(instr_rd_msb_c downto instr_rd_lsb_c)         <= "01" & ci_instr16_i(ci_rd_3_msb_c downto ci_rd_3_lsb_c);   -- x8 - x15
-            if (ci_instr16_i(ci_funct3_lsb_c) = '1') and (FPU_ENABLE = false) then -- C.FLW
-              illegal <= '1';
-            end if;
 
-          when "110" | "111" => -- C.SW / C.FSW (integer and float are identical as the FPU implements the Zfinx ISA extension)
+          when "110" => -- C.SW
           -- ----------------------------------------------------------------------------------------------------------
             decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_store_c;
             decoded(08 downto 07)                                 <= "00";
@@ -155,34 +170,10 @@ begin
             decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_sw_c;
             decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "01" & ci_instr16_i(ci_rs1_3_msb_c downto ci_rs1_3_lsb_c); -- x8 - x15
             decoded(instr_rs2_msb_c downto instr_rs2_lsb_c)       <= "01" & ci_instr16_i(ci_rs2_3_msb_c downto ci_rs2_3_lsb_c); -- x8 - x15
-            if (ci_instr16_i(ci_funct3_lsb_c) = '1') and (FPU_ENABLE = false) then -- C.FSW
-              illegal <= '1';
-            end if;
 
-          when others => -- "000": Illegal_instruction, C.ADDI4SPN; others: illegal
+          when others => -- "011": C.FLW, "111": C.FSW, "001": C.FLS / C.LQ, "100": reserved, "101": C.FSD / C.SQ
           -- ----------------------------------------------------------------------------------------------------------
-            decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_alui_c;
-            decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "00010"; -- stack pointer
-            decoded(instr_rd_msb_c downto instr_rd_lsb_c)         <= "01" & ci_instr16_i(ci_rd_3_msb_c downto ci_rd_3_lsb_c);
-            decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_subadd_c;
-            decoded(instr_imm12_msb_c downto instr_imm12_lsb_c)   <= (others => '0'); -- zero extend
-            decoded(instr_imm12_lsb_c + 0)                        <= '0';
-            decoded(instr_imm12_lsb_c + 1)                        <= '0';
-            decoded(instr_imm12_lsb_c + 2)                        <= ci_instr16_i(6);
-            decoded(instr_imm12_lsb_c + 3)                        <= ci_instr16_i(5);
-            decoded(instr_imm12_lsb_c + 4)                        <= ci_instr16_i(11);
-            decoded(instr_imm12_lsb_c + 5)                        <= ci_instr16_i(12);
-            decoded(instr_imm12_lsb_c + 6)                        <= ci_instr16_i(7);
-            decoded(instr_imm12_lsb_c + 7)                        <= ci_instr16_i(8);
-            decoded(instr_imm12_lsb_c + 8)                        <= ci_instr16_i(9);
-            decoded(instr_imm12_lsb_c + 9)                        <= ci_instr16_i(10);
-            --
-            if (ci_instr16_i(12 downto 5) = "00000000") or -- canonical illegal C instruction or C.ADDI4SPN with nzuimm = 0
-               (ci_instr16_i(ci_funct3_msb_c downto ci_funct3_lsb_c) = "001") or -- C.FLS / C.LQ
-               (ci_instr16_i(ci_funct3_msb_c downto ci_funct3_lsb_c) = "100") or -- reserved
-               (ci_instr16_i(ci_funct3_msb_c downto ci_funct3_lsb_c) = "101") then -- C.FSD / C.SQ
-              illegal <= '1';
-            end if;
+            illegal <= '1';
 
         end case;
 
@@ -350,7 +341,7 @@ begin
               illegal <= '1';
             end if;
 
-          when "010" | "011" => -- C.LWSP / C.FLWSP (integer and float are identical as the FPU implements the Zfinx ISA extension)
+          when "010" | "011" => -- C.LWSP / C.FLWSP
           -- ----------------------------------------------------------------------------------------------------------
             decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_load_c;
             decoded(21 downto 20)                                 <= "00";
@@ -364,11 +355,11 @@ begin
             decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_lw_c;
             decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "00010"; -- stack pointer
             decoded(instr_rd_msb_c downto instr_rd_lsb_c)         <= ci_instr16_i(ci_rd_5_msb_c downto ci_rd_5_lsb_c);
-            if (ci_instr16_i(ci_funct3_lsb_c) = '1') and (FPU_ENABLE = false) then -- C.FLWSP
+            if (ci_instr16_i(ci_funct3_lsb_c) = '1') then -- C.FLWSP is illegal
               illegal <= '1';
             end if;
 
-          when "110" | "111" => -- C.SWSP / C.FSWSP (integer and float are identical as the FPU implements the Zfinx ISA extension)
+          when "110" | "111" => -- C.SWSP / C.FSWSP
           -- ----------------------------------------------------------------------------------------------------------
             decoded(instr_opcode_msb_c downto instr_opcode_lsb_c) <= opcode_store_c;
             decoded(08 downto 07)                                 <= "00";
@@ -382,7 +373,7 @@ begin
             decoded(instr_funct3_msb_c downto instr_funct3_lsb_c) <= funct3_sw_c;
             decoded(instr_rs1_msb_c downto instr_rs1_lsb_c)       <= "00010"; -- stack pointer
             decoded(instr_rs2_msb_c downto instr_rs2_lsb_c)       <= ci_instr16_i(ci_rs2_5_msb_c downto ci_rs2_5_lsb_c);
-            if (ci_instr16_i(ci_funct3_lsb_c) = '1') and (FPU_ENABLE = false) then -- C.FSWSP
+            if (ci_instr16_i(ci_funct3_lsb_c) = '1') then -- C.FSWSP is illegal
               illegal <= '1';
             end if;
 

diff --git a/rtl/core/neorv32_package.vhd b/rtl/core/neorv32_package.vhd
@@ -56,7 +56,7 @@ package neorv32_package is
 
   -- Architecture Constants -----------------------------------------------------------------
   -- -------------------------------------------------------------------------------------------
-  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01090302"; -- hardware version
+  constant hw_version_c : std_ulogic_vector(31 downto 0) := x"01090303"; -- hardware version
   constant archid_c     : natural := 19; -- official RISC-V architecture ID
   constant XLEN         : natural := 32; -- native data path width
 

diff --git a/sw/example/processor_check/main.c b/sw/example/processor_check/main.c
@@ -638,6 +638,10 @@ int main() {
 
   {
     asm volatile (".align 4");
+    if (neorv32_cpu_csr_read(CSR_MISA) & (1<<CSR_MISA_C)) { // C extension enabled
+      asm volatile (".half 0x0000"); // canonical compressed illegal
+      asm volatile (".half 0x66aa"); // c.flwsp (illegal since F is not supported)
+    }
     asm volatile (".word 0x0e00202f"); // amoswap.w x0, x0, (x0)
     asm volatile (".word 0x34004073"); // illegal CSR access funct3 (using mscratch)
     asm volatile (".word 0x30200077"); // mret with illegal opcode
@@ -651,11 +655,19 @@ int main() {
     asm volatile (".align 4");
   }
 
-  tmp_b = trap_cnt; // number of traps we are expecting
+  // number of traps we are expecting
+  if (neorv32_cpu_csr_read(CSR_MISA) & (1<<CSR_MISA_C)) { // C extension enabled
+    tmp_a += 12;
+  }
+  else { // C extension disabled
+    tmp_a += 10;
+  }
+
+  tmp_b = trap_cnt; // number of traps we have seen
 
   if ((neorv32_cpu_csr_read(CSR_MCAUSE) == TRAP_CODE_I_ILLEGAL) && // illegal instruction exception
       (neorv32_cpu_csr_read(CSR_MTINST) == 0xfe002fe3) && // instruction word of last illegal instruction
-      ((tmp_a + 10) == tmp_b)) { // right amount of illegal instruction exceptions
+      (tmp_a == tmp_b)) { // right amount of illegal instruction exceptions
     test_ok();
   }
   else {