simulator: Add support for NVC

[m42uko]

This PR adds support for the NVC VHDL simulator.

This PR is a ready for review:

• Add NVC as a simulator
• Integrate NVC into CI tests
• Check that all error messages printed are not something to be fixed in cocotb-test
• Figure out how to handle pre-command compile options vs post-command compile options
• Wait until NVC support has been added upstream in cocotb, tracked as Add NVC support by reverting removal cocotb/cocotb#3427

[m42uko]

@nickg: If you have the time, I'd greatly appreciate it if you could review this PR.

[nickg]

Looks ok to me apart from the ghw/fst thing.

[nickg]

The extension should be .fst here.

[m42uko]

Thanks, fixed!

[m42uko]

Now that upstream support has been merged, I ran the simple DFF test case again and it passes, but throws some error messages along the way. Does anyone have some pointers on where I need to look or whether these are sort of expected?

Here's the command output:

[builduser@737d440cb9bd tests]$ cocotb-config -v
1.9.0.dev0
[builduser@737d440cb9bd tests]$ cocotb-test -v
0.2.4
[builduser@737d440cb9bd tests]$ nvc -v
nvc 1.10.4 (1.10.4.r0.g085af504) (Using LLVM 16.0.6)
Copyright (C) 2011-2023  Nick Gasson
This program comes with ABSOLUTELY NO WARRANTY. This is free software, and
you are welcome to redistribute it under certain conditions. See the GNU
General Public Licence for details.
[builduser@737d440cb9bd tests]$ SIM=nvc python test_dff.py 
INFO cocotb: Running command: nvc --work=dff_test_vhdl -L /home/builduser/cocotb-test/src/cocotb-test/tests/sim_build -a /home/builduser/cocotb-test/src/cocotb-test/tests/dff.vhdl
INFO cocotb: Running command: nvc --work=dff_test_vhdl -L /home/builduser/cocotb-test/src/cocotb-test/tests/sim_build -e dff_test_vhdl
INFO cocotb: Running command: nvc --work=dff_test_vhdl -L /home/builduser/cocotb-test/src/cocotb-test/tests/sim_build -r dff_test_vhdl --load /usr/lib/python3.11/site-packages/cocotb/libs/libcocotbvhpi_nvc.so
ERROR cocotb: ** Note: loading VHPI plugin /usr/lib/python3.11/site-packages/cocotb/libs/libcocotbvhpi_nvc.so
INFO cocotb:      -.--ns INFO     gpi                                ..mbed/gpi_embed.cpp:76   in set_program_name_in_venv        Did not detect Python virtual environment. Using system-wide Python interpreter
INFO cocotb:      -.--ns INFO     gpi                                ../gpi/GpiCommon.cpp:101  in gpi_print_registered_impl       VHPI registered
INFO cocotb:      0.00ns INFO     cocotb                             Running on nvc version 1.10.4
INFO cocotb:      0.00ns INFO     cocotb                             Running tests with cocotb v1.9.0.dev0 from /usr/lib/python3.11/site-packages/cocotb
INFO cocotb:      0.00ns INFO     cocotb                             Seeding Python random module with 1697738709
ERROR cocotb: ** Error: (init): class kind vhpiRootInstK is not an object declaration
ERROR cocotb: ** Error: (init): relationship DEPRECATED_vhpiSubtype is deprecated and not implemented in vhpi_handle
INFO cocotb:      0.00ns INFO     cocotb.regression                  Found test dff_cocotb.test_dff_simple
INFO cocotb:      0.00ns INFO     cocotb.regression                  running test_dff_simple (1/1)
INFO cocotb:                                                           Test that d propagates to q
ERROR cocotb: ** Error: (init): class kind vhpiPortDeclK is not a prefixed name
ERROR cocotb: /home/builduser/cocotb-test/src/cocotb-test/tests/dff_cocotb.py:15: DeprecationWarning: cocotb.fork has been deprecated in favor of cocotb.start_soon and cocotb.start.
ERROR cocotb: In most cases you can simply substitute cocotb.fork with cocotb.start_soon.
ERROR cocotb: For more information about when you would want to use cocotb.start see the docs,
ERROR cocotb: https://docs.cocotb.org/en/latest/coroutines.html#concurrent-execution
ERROR cocotb:   cocotb.fork(clock.start())  # Start the clock
ERROR cocotb: ** Error: 5us+1: class kind vhpiPortDeclK is not a prefixed name
ERROR cocotb: ** Error: 15us+1: class kind vhpiPortDeclK is not a prefixed name
INFO cocotb: 105000.00ns INFO     cocotb.regression                  test_dff_simple passed
INFO cocotb: 105000.00ns INFO     cocotb.regression                  **************************************************************************************
INFO cocotb:                                                         ** TEST                          STATUS  SIM TIME (ns)  REAL TIME (s)  RATIO (ns/s) **
INFO cocotb:                                                         **************************************************************************************
INFO cocotb:                                                         ** dff_cocotb.test_dff_simple     PASS      105000.00           0.00   41313500.94  **
INFO cocotb:                                                         **************************************************************************************
INFO cocotb:                                                         ** TESTS=1 PASS=1 FAIL=0 SKIP=0             105000.00           0.11     927657.99  **
INFO cocotb:                                                         **************************************************************************************
INFO cocotb: 
ERROR cocotb: ** Note: 105000000001fs+0: VHPI plugin requested end of simulation
INFO cocotb: Results file: /home/builduser/cocotb-test/src/cocotb-test/tests/sim_build/bfpc95uh_results.xml

EDIT: I expect the cocotb.fork to be unrelated, but I'm worried about the vhpiPortDeclK and that the loading VHPI plugin line is classified as an error.

[nickg]

If you update to the latest master branch of nvc those VHPI errors should be gone (actually they are hidden unless you pass the --vhp-debug flag).

By default all diagnostic messages are printed to stderr, I guess that's why they are labelled as "ERROR" above. You can change that with the --stderr option - e.g. nvc --stderr=error ... prints messages with severity error and above to stderr and the rest to stdout.

[m42uko]

@cocotb-test maintainers: I have a question about argument handling. NVC has two types of arguments, before-COMMAND (general, e.g. --std=08 to set the language standard), and after-COMMAND (specific to the step, e.g. -g for settings generics during elaboration). And these two types must be places in different places in the command string.

Now, in theory, cocotb-test has this as well in the form of extra_args and <step>_args. But https://github.com/themperek/cocotb-test/blob/master/cocotb_test/simulator.py#L135 and following already combines this into the compile and simulation args instead of providing it as a value to the Simulator class.

Do you have a suggestions on how to handle this? I think to get support for NVC off the ground, it's OK to only support before-COMMAND arguments for now as those are the more important ones (such as setting the language revision), but I would like to hear your thoughts.

EDIT: My implementation right now "abuses" compile_args as before-COMMAND and vhdl_compile_args as after-COMMAND, but that isn't really great either...

[themperek]

I would do the same as here:

https://github.com/cocotb/cocotb/blob/master/cocotb/share/makefiles/simulators/Makefile.nvc#L35-L45

https://github.com/themperek/cocotb-test/blob/master/cocotb_test/simulator.py#L135 looks like a bug too.

[m42uko]

I moved the extra_args handling to the individual simulator code, meaning instead of one concatenation in the initialization of the class, I do it in each simulator. The only special case would be NVC, where instead of just concatenating, I place it at the right point in the call.

I also "cleaned up" the initialization code with all of the if not None statements -- I hope that's agreeable.

Also, I rebased everything onto the latest master.

[m42uko]

Hmmm, the NVC CI run fails because the used cocotb version is too old. Probably because NVC support hasn't made it into a release yet (but it is merged). Any opinions on how to handle that for now?

[themperek]

Seems like NVC support is not released yet. Would need to use the git/devel version for NVC somehow.

[m42uko]

Rebased onto latest master.

Do you think it'd make sense to implement something like 8c7c060 to test NVC support against the latest master (v2.x.x) while the other tests run against the last official release?

I guess the upside would be that we can merge this, but it would also mean that cocotb-test would show support for NVC for people running the old cocotb, which then breaks when trying to use it. My guess is we'll really just have to wait for cocotb 2.0.0...

[themperek]

1.9 is realised with NVC support. I guess we can move to 1.9.

[themperek]

@m42uko Can you force push to trigger CI? There is something wrong with CI.

[m42uko]

force push to trigger CI

Done.

[m42uko]

I hope there are no other breaking changes in Cocotb 1.9. I recently built cocotb-test "against" Cocotb master (2.x) and there were various API changes that needed updating in cocotb-test (like change of environment variable names etc.).

[m42uko]

Should I edit

cocotb-test/azure-pipelines.yml

Line 123 in d2e70f5

conda install --yes pip && git clone -b v1.6.0 https://github.com/cocotb/cocotb.git && cd cocotb && cd .. && pip install -e cocotb

to actually use the latest 1.9 release? Or is this not necessary?

[themperek]

I have updated it to 1.9 in master.

Now is this: https://dev.azure.com/themperek/themperek/_build/results?buildId=924&view=logs&j=2e4c3efa-3be9-5087-16d3-fd6e7b71d6f8&t=e2ac3808-912e-5cb8-bbd3-6b8e289a4d6d&l=127

PS. I will fix the cocotb version to 1.9 in the setup script for now.

[m42uko]

Rebased. EDIT: Sorry, just saw now that you ran it with the change already -- will investigate.

[m42uko]

Two bugs fixed:

1. Generics must be passed as ["-g", "KEY=VALUE"], not as ["-gKEY=VALUE"]
2. They must be passed during elaboration, not for run
3. I forgot to disable one of the Verilog test cases for NVC

🤞 that it passes now :)

[themperek]

Thank you for PR and refactoring.