stnolting / neorv32-riscof

✔️Port of RISCOF to check the NEORV32 for RISC-V ISA compatibility.
https://github.com/stnolting/neorv32
BSD 3-Clause "New" or "Revised" License
25 stars 6 forks source link

coverage support #65

Open LeFl0w opened 1 year ago

LeFl0w commented 1 year ago

hi, it is more like an information for you than a real pull request.

Thanks to GHDL with GCC backend, we can compute coverage.
You can see in the diff that there is not many changes , the only 'difficulty' is to recompile GHDL with gcc backend.
Feel free to merge it (be careful as if your ghdl has not got covergage builtin, the simulation won't work) or to refuse it and get inspired to include coverage in your repository later on if wanted.

You can have a look at the lcov coverage output: html.zip

cheers.

stnolting commented 1 year ago

This is really cool! :+1:

I've never (really) worked with HDL code coverage analysis so far, but I think this might help a lot to improve code quality. It would be nice to have this code-coverage-enabled version of GHDL as default simulator version (and to deploy the results to GitHub pages automatically).

Are there any prebuild (and up-to-date) packages of GHDL with GCC as backend available?

LeFl0w commented 1 year ago

I ran a little test and search. It seems that any gcc support version pack the coverage support .

I tried docker version fedora36-gcc-11.3.0 from GHDL docker hub and the simple adder coverage example worked.

I tried binaires ghdl-gha-ubuntu-22.04-gcc.tgz from ghdl but even of ubuntu 22.04 I got a library error libgcov profiling error:./ghdl-coverage-master/projects/adder/adder.gcda:Version mismatch - expected 11.3 (release) (B13*) got 11.2 (release) (B12*) .

so the best way to go would be the docker image or maybe the binary if you achieve the same gcc version

LeFl0w commented 1 year ago

if you want to give a try, here is an extract of the github action we are using to apply the coverage on the vunit part of the neorv32 project (neorv32 was included as a submodule). It is not directly related to the riscof project but very similar anyway.

name: Full CI/CD
on:
  push:
    branches:
      - main
      - develop

jobs:
  coverage:
    name: Neorv32 coverage
    timeout-minutes: 60
    runs-on:  ubuntu-latest
    container: 
      image: ghdl/ghdl:fedora36-gcc-11.3.0
      volumes:
        - /$(pwd):/src
    steps:
      - name: Install Prerequisite
        run: |
          yum install -y python3-pip
          yum  -y install git
          yum install -y patch
          pip install gcovr 
          pip install vunit_hdl
          pip install beautifulsoup4
      - name: Git Checkout test Neorv32
        uses: actions/checkout@v3
        with:
          fetch-depth: 0
          submodules: 'true'

      - name: Create coverage report
        continue-on-error: true
        run: |
          patch -u ./neorv32/sim/run.py -i run.py.patch
          patch -u ./neorv32/rtl/core/neorv32_cpu.vhd -i neorv32_cpu.vhd.patch
          cd neorv32/sim
          VUNIT_SIMULATOR=ghdl ./run.py
          cd ../../

      - name: Archive code coverage results
        uses: actions/upload-artifact@v3
        with:
          name: neorv32-code-coverage-report
          path: ./coverage.xml

neorv32_cpu.vhd.patch run.py.patch

you can notice we are using two patches :

be careful as the pipeline could take 40 min to 1h to perform the coverage on the Vunit part of neorv32

stnolting commented 1 year ago

Thanks for the code (and sorry for the late reply). I will do some tests here. I am still hoping to include this in the default CI configuration of this repository.

the second to comment two assertion in the neorv32/rtl/core/neorv32_cpu.vhd file which trigger an error when the associated generic is 0 (which is posisble in the VHDL but not allowed in the assertion)

Could you explain this a little bit more in detail? 🤔

LeFl0w commented 1 year ago

No worries. We integrated that already in a custom ci-cd with sonarqube .We got a graphical feedback about the missing coverage related to the code produced within a new version( which is a more interesting way of dealing with coverage that a whole absolute number). When everything is set up, we will release the link to the web instance.

For the second element , it is linked with The assertions https://github.com/stnolting/neorv32/blob/49236537a223708b7366c55f40140f0d0b1e7817/rtl/core/neorv32_cpu.vhd#L214 and https://github.com/stnolting/neorv32/blob/49236537a223708b7366c55f40140f0d0b1e7817/rtl/core/neorv32_cpu.vhd#L226

They trigger an assertion failure with example https://github.com/stnolting/neorv32/blob/main/rtl/processor_templates/neorv32_ProcessorTop_UP5KDemo.vhd in which PMP_NUM_REGIONS and HPM_NUM_CNTS are set to 0. A failure is triggered with any Verific based VHDL frontend. I did not investigate to see if the was legit , we simply commented them out , as it is only assertions.