-
Notifications
You must be signed in to change notification settings - Fork 50
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
Gvsoc ci #142
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @haugoug!
I have a few major comments:
- To add the GVSoC tests to the CI the workflow file needs to be adapted as well, see
snitch_cluster/.github/workflows/ci.yml
Line 47 in e90dceb
./util/run.py sw/run.yaml --simulator verilator -j - For all tests which make use of the IPC verification interface (i.e. those which have no BIST verification), just running the binaries with GVSoC doesn't really check anything. Perhaps we want to extend the IPC verification interface to work also with GVSoC, otherwise we just need to make clear that the GVSoC simulation will not really verify those tests.
Minor comments follow on the code.
class GvsocSimulator(RTLSimulator): | ||
"""Gvsoc simulator | ||
|
||
An [RTL simulator][Simulator.RTLSimulator], identified by the name | ||
`vsim`, tailored to the creation of | ||
[Gvsoc simulations][Simulation.GvsocSimulation]. | ||
""" | ||
|
||
def __init__(self, binary): | ||
"""Constructor for the GvsocSimulator class. | ||
|
||
Arguments: | ||
binary: The Gvsoc simulation binary. | ||
""" | ||
super().__init__(binary, name='gvsoc', simulation_cls=GvsocSimulation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it inherit from the RTLSimulator
class?
No description provided.