Fusesoc and Linter #368
Fusesoc and Linter #368
Conversation
|
Here is an example for the annotations: https://github.com/wallento/riscv/actions/runs/137172924 |
|
From #367:
The minor things are to resolve warnings with recent GCCs. About 2, you are right. I kept it in there for reference. If you are happy I would replace it with |
wallento
force-pushed
the
fusesoc-and-linter
branch
from
6e58444 to
457f8e0
Compare
|
Hi @MikeOpenHWGroup, |
|
I can't really understand what Github feels about that Makefile to be a conflict, seems pretty straight-forward to me.. |
|
Ah, I see! There was a massive rename between my first PR and now :) I will amend again after rebase! |
There was a problem hiding this comment.
Hi @wallento. Looks good to me. By definition I exclude myself from approving this PR (the verification people should never make-or-approve changes to the RTL). Having said that, this is the right track to be on.
|
PR is guaranteed to be broken with this merge #360. I believe a rebase is in order. |
This fixes compilation issues with the Verilator simulations. Signed-off-by: Stefan Wallentowitz <[email protected]>
This adds support for FuseSoC, currently for the verilator model only. Signed-off-by: Stefan Wallentowitz <[email protected]>
This is based on the LibreCores CI infrastructure and doesn't need anything except this file. Signed-off-by: Stefan Wallentowitz <[email protected]>
This silences the linter for all known warnings. That eases the migration to a linter flow, because new linter warnings are identified quickly. Anyhow, I suggest to curate the waiver file by either solving the underlying problem or replacing the comment with a meaningful description. Also, we can now be pedantic, linter will fail on any warning. Signed-off-by: Stefan Wallentowitz <[email protected]>
wallento
force-pushed
the
fusesoc-and-linter
branch
from
457f8e0 to
54bd9e8
Compare
|
Rebased and amended to match the changed naming |
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v1 | ||
| - name: Lint Verilog sources with Verilator |
There was a problem hiding this comment.
I would not want any dependency on Verilator
| command: 'run' | ||
| core: 'openhw:cv32e40p:core' | ||
| target: 'lint' | ||
| tool: 'verilator' |
There was a problem hiding this comment.
I would not want any dependency on Verilator
| - files_rtl | ||
| lint: | ||
| <<: *default | ||
| default_tool: verilator |
There was a problem hiding this comment.
Can we make sure things run without Verilator as well?
There was a problem hiding this comment.
I cannot try this out because we do not have the permissions to run pip3 install git+https://github.com/olofk/fusesoc@master
Can you please make sure this flow works for xrun and Questa as well and can run without any dependency on Verilator?
|
Hi @wallento Is there a way we can merge your pull request and also have the possibility to keep the old way of working for 1-2 weeks so that we do not immediately block many people just in case there are initial problems? How about core-v-verif, that would need to be updated as well, right? |
Silabs-ArjanB
added
the
WAIVED:CV32E40P
This adds support for FuseSoC. As example, it fixes the verilator model to directly use it.
But even more interesting, it adds a github action that automatically runs the linter on each push and pull request. When the linter fails, the action fails and a PR can be polished.
As a caveat, there are 421 Linter warnings on the codebase at the moment. I have added a waiver file that suppresses them, but I suggest to fix the issues or replace the placeholder comments with an explanation. Happy to help, but too many for a "quick PR".