Skip to content

Implement RV32C instruction decompressor #4

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

Closed
wants to merge 8 commits into from

Conversation

kaeteyaruyo
Copy link

@kaeteyaruyo kaeteyaruyo commented Jan 18, 2021

I implemented the RV32C extension on this emulator. Excepts C.EBREAK, all the other instructions are passed the compliance test. The detail of implementation are recorded in the commit message.

I understand that the way I implement the decompressor is very different from the instruction decoder in this project. I will try to reorganize the code and make it more modularized.
Edit at 1/20: modularization has been done.

There are somethings that I am not sure whether I implement correctly:

  • The error handling on the invalid 16-bits instruction (0x0000). For now, if the emulator encounters an invalid 16-bits instruction, it will halt immediately.
  • The address alignment in the memory_read_ifetch(). I wonder if the function returns correct data at the boundary of a chunk.
  • When should I increase the rv->csr_cycle?

* Add `ENABLE_RV32C` flag in makefile to turn on / off RVC extension
* Add `step_size` field into riscv virtual machine
    * To support RVC extension, in which the instruction might be
      2 bytes long, the PC must be able to step either 2 byte or 4 bytes.
      But the current implementation always steps 4 bytes, so I introduce
      this field to manage it.
	* By default it is set to 4 (in `rv_reset()`).
    * It is rely on the instruction length, so it needs to be
      set at the instruction fetch state. If the instruction is 16-bit,
      we set it to 2, else we set it to 4.
* Modify the instruction fetch implementation in `rv_step()`
	* The IF is now implemented in 2 different version, and the
      `ENABLE_RV32C` indicates which version we are using.
        * When `ENABLE_RV32C` is defined, the machine will detect
          the instruction length by checking its lowest 2 bit. If it is
          a 16-bit instruction, it cuts down the lower 2 bytes
          (because RISC-V is little endian) as current instruction,
          and buffers higher 2 bytes in the instruction buffer.
          The remained part is treat as next instruction's lower part,
          and will be used in next instruction fetch.
        * When `ENABLE_RV32C` is not defined, the machine just uses
          a whole instruction buffer as current instruciton.
* Implement 16-bit instruction decompression
    * Instead of implementing another set of decoder for 16-bit
      instructions, which is less maintainable and error-prone,
      I just implement the instruction decompressor, to expand
      16-bit instruction into its corresponding 32-bit one.
      Then we can just dispatch it as the standard 32-bit instruction.
    * The decompressors are implemented in the `rvc.c` and are available
      via `rvc.h`. For now, only decompressors for instructions whose
      opcode = 0b00 are implemented.
    * TODO: handle illegal 16-bit instruction
* Modify PC value alignment constraint
    * The PC value is allow to be the multiple of 2
      when RVC extension is on
    * The functions being modified include:
        * `memory_read_ifetch()`
        * `op_branch()`
        * `op_jalr()`
        * `op_jal()`
* In the jump instructions, the return address are computed as
  `rv->PC + rv->step_size` instead of `rv->PC + 4`.
  Since if the jump instruction is 16-bits, the return address
  should be assigned as PC + 2, not PC + 4.
* Remove instruction buffer from `rv_step()`, and declare it as
  a member attribute of virtual machine. Since we need to clear
  instruction beffer when the branch taken and jump occurs,
  we need to be able to access it outside `rv_step()`.
* Remove `halfway` flag in `rv_step()`, and use the emptiness of
  instruction buffer instead to indicate whether to fetch next
  instruction.
* Rename `addi4spn_to_addi()` to `caddi4spn_to_addi()`.
* Decompress instructions whose opcode = 01 or 10
    * Besides C.EBREAK, all the other instructions are finished,
      and passed the verification of RISCV-compliance.
* Add `+signature` argument for the RISCV-compliance unit test
    * Use "+signature=file_name" to dump the signature
      (part of data memory) into signature file
    * The signature file is used to verify if the instructions
      are implemented correctly
@jserv
Copy link
Contributor

jserv commented Jan 20, 2021

I would like to ask ccs100203 to review.

* Follow the example of the opcode decoder implementation.
  Turn the decompressors switch case into look up table.
* Remove debug output from `rvc.c`
* Follow the example of `riscv_private.h`, separate the decoding part
  and encoding part into several static inline functions
  in `rvc_private.h`. Now the code is more modularized.
* Rename some of the decompressor functions.
* Rewrite comment on the decompressor.
@kaeteyaruyo kaeteyaruyo changed the title WIP: Implement RV32C instruction decompressor Implement RV32C instruction decompressor Jan 20, 2021
* Rename `step_size` to `inst_length`
* Fix comment in `rvc.c` (I type `funct` as `opcode`
  and `opcode` as `funct` by mistake).
@kaeteyaruyo
Copy link
Author

I would like to ask ccs100203 to review.

Sorry for late reply, but should I contact ccs100203 myself to ask for a review?

* Rename `rvc*` to `compressed*`, including:
    * `rvc.h` -> `compressed.h`
    * `rvc.c` -> `compressed.c`
    * `rvc_private.h` -> `compressed_private.h`
@kaeteyaruyo kaeteyaruyo requested a review from jserv February 1, 2021 12:00
@jserv
Copy link
Contributor

jserv commented Mar 11, 2021

I implemented the RV32C extension on this emulator. Excepts C.EBREAK, all the other instructions are passed the compliance test. The detail of implementation are recorded in the commit message.

Add corresponding FIXME/TODO in the comment of source files.

There are somethings that I am not sure whether I implement correctly:

  • The error handling on the invalid 16-bits instruction (0x0000). For now, if the emulator encounters an invalid 16-bits instruction, it will halt immediately.
  • The address alignment in the memory_read_ifetch(). I wonder if the function returns correct data at the boundary of a chunk.
  • When should I increase the rv->csr_cycle?

Instead of halting, can you raise verbose error messages when executing invalid 16-bits instruction?

* Raise `rv_except_illegal_inst` error when executing invalid 16-bit
  instruction (0x0000).
* Remove legacy code
@kaeteyaruyo
Copy link
Author

kaeteyaruyo commented Mar 13, 2021

I implemented the RV32C extension on this emulator. Excepts C.EBREAK, all the other instructions are passed the compliance test. The detail of implementation are recorded in the commit message.

Add corresponding FIXME/TODO in the comment of source files.

I think I implement the decoder for C.EBREAK correctly. I tried to print some message to see if the emulator executes EBREAK when it encounters C.EBREAK, and the result shows that it actually does. But in compliance test, it checks the correctness of implementation by verifying the memory dump, and it shows that the case C.EBREAK doesn't manipulate memory correctly (there should be something in the memory, but the range it tests are all filled with deadbeef). I am not sure whether the problem comes from emulator or the device configuration in compliance test. Should I still add the TODO comment?

There are somethings that I am not sure whether I implement correctly:

  • The error handling on the invalid 16-bits instruction (0x0000). For now, if the emulator encounters an invalid 16-bits instruction, it will halt immediately.
  • The address alignment in the memory_read_ifetch(). I wonder if the function returns correct data at the boundary of a chunk.
  • When should I increase the rv->csr_cycle?

Instead of halting, can you raise verbose error messages when executing invalid 16-bits instruction?

I raise rv_except_illegal_inst error when encountering that.

@kaeteyaruyo
Copy link
Author

I noticed that there is a ENABLE_COMPUTED_GOTO feature in current implementation, but the DISPATCH() part do not support compressed instruction yet. Should I also add the decoder part into DISPATCH()?

@jserv
Copy link
Contributor

jserv commented Jan 7, 2022

Drop in favor of #11

@jserv jserv closed this Jan 7, 2022
vacantron pushed a commit to vacantron/rv32emu that referenced this pull request Dec 14, 2024
-fsanitize=address report when CTRL+a x exiting:
=================================================================
==297977==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 23400 byte(s) in 117 object(s) allocated from:
    #0 0x761b706fd340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x633ad9c5e310 in block_translate src/emulate.c:649
    sysprog21#2 0x633ad9c5e310 in block_find_or_translate src/emulate.c:865
    sysprog21#3 0x633ad9c5e310 in rv_step src/emulate.c:1029
    sysprog21#4 0x633ad9c5e310 in rv_run src/riscv.c:498
    sysprog21#5 0x633ad9c5e310 in main src/main.c:279

Direct leak of 3136 byte(s) in 125 object(s) allocated from:
    #0 0x761b706fd9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x633ad9c5fea4 in match_pattern src/emulate.c:767
    sysprog21#2 0x633ad9c5fea4 in block_find_or_translate src/emulate.c:872
    sysprog21#3 0x633ad9c5fea4 in rv_step src/emulate.c:1029
    sysprog21#4 0x633ad9c5fea4 in rv_run src/riscv.c:498
    sysprog21#5 0x633ad9c5fea4 in main src/main.c:279

Register a clean up callback, async_block_clear() to free all the
allocated memory fix this when emulator exits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants