Support Questa qrun/QIS flow

[imphil]

Support the qrun/QIS flow in Questa in the Makefile flow

v2 from #3274:

• Default to the new flow from Questa 2023.1 on (the first version which includes Visualizer and hence is guaranteed to work with the flow)
• Give users the option to choose the old or the new flow by using SIM=questa-qrun or SIM=questa-compat.
• More documentation updates.
• Implement the Questa version detection in Python to avoid adding a dependency to awk and sed.

@AbdouTharwat I kept the original commit attributed to you, since you did the majority of the work. Thank you for it! Can you please have a look and give it a try?

Patches:

• Support the Questa qrun flow
• Questa: Remove unnecessary FLI library check
• Add minimal documentation for the custom qrun flow

[imphil]

If we're OK with the flow in general I'll add the same commands to the runner as well (probably in a separate PR).

[AbdouTharwat]

@imphil Yeah sure, thanks for you efforts. I am looking into this and testing it now, I will update you once I am done

[ktbarrett]

Needs a newsfragment.

[AbdouTharwat]

Thanks for your efforts @imphil, It is working well, the info messages also are great and keep the user aware of the changes done. also variables names looks better now. We are OK with this, you can go head. Thanks

[ktbarrett]

Looks good. After the newsfragment of course.

[AbdouTharwat]

Sorry, can we add the python architecture check again to Makefile.questa-qrun, I assumed user will point to only one binary and forgot the case when user has both 32 and 64 binaries in same path, this should not impact anything else.
it will only be adding these 3 lines just before the line 121 starting with: $(COCOTB_RESULTS_FILE):
ifeq ($(PYTHON_ARCH),64bit)
QRUN_CMD += -64
endif
Thanks,

[imphil]

Updated to include the 64b switch as proposed as well as a newsfragment. I'll wait with merging until we have Questa 2023 in CI (currently blocked for unrelated test failures).

[imphil]

OK, we now got Questa 2023.2 in CI, together with Questa 2022.4. All tests pass with 2022.4 using the +acc old flow. Unfortunately, a number of tests fail with the new QIS/Qrun flow:

Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_read_write' with parameters 'all'
Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_discover_all' with parameters 'all'
Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_direct_constant_indexing' with parameters 'all'
Failure in testsuite: 'all' classname: 'test_iteration' testcase: 'recursive_discovery' with parameters 'all'

The failures look on first sight like differences in signal visibility due to the QIS flow.

@AbdouTharwat Can you reproduce these test failures with this PR? You should get the same failures if you run the tests against Questa 2023.2, as described at https://github.com/cocotb/cocotb/blob/master/CONTRIBUTING.md#running-tests-locally.

I can try to piece together steps to reproduce these failures and file an issue with Siemens, but you're probably in a much better position to do so. Let me know if you need further input from my side.

[AbdouTharwat]

Thanks @imphil, I have reproduced these 4 test failures and filed an issue. We are now looking into this

[ktbarrett]

So what can we do for now to get this in for 1.8? Do we add conditionals for both version and whether the qrun mode is used? We can only really detect if we are using qrun if use the environment variable, but perhaps that's fine here since this is a part of the regression where we are only using makefiles (for now).

[imphil]

I'd just leave this out of 1.8, we're not ready yet to move users over to the new flow without regressions. Users can still use the older flow until we understand the regressions better.

[AbdouTharwat]

Hello @imphil, after having a look on the failures you mentioned I have some observations:

in test_iteration_verilog --> testcase: recursive_discovery

• test_iteration_es.py line 40: states that vpiAlways does not show, and thus removed from pass_total to be 259 instead of 265
• acc flow: Found a total of 259
• QIS flow: Found a total of 265
• Which means QIS flow now has correct result, do you agree?

in test_iteration_mixedlang --> testcase: recursive_discovery

• test_iteration.py line 75: states that vpiAlways does not show, and thus removed from pass_total to be 991 instead of 1024
• acc flow: Found a total of 991
• QIS flow: Found a total of 1024
• Which means QIS flow now has correct result, do you agree?

in test_iteration_vhdl (FLI) --> testcase: recursive_discovery

• test_iteration_es.py line 44: states that FLI starting 2023.1 does not discover certain objects, thus removed from pass_total to be 34553 instead of 34569
• acc flow: Found a total of 34553
• QIS flow: Found a total of 34569
• Which means QIS flow now has correct result, do you agree?

in test_array --> testcase: recursive_discovery

• QIS flow found 17 more objects due to vpiAlways is now shown, is this expected?

We are investigating other issues of:

• FLI with test_array --> testcase test_read_write and test_discover_all
• VHPI with test_iteration_vhdl (VHPI) --> testcase: recursive_discovery

[ktbarrett]

Why are we even trying to discover always, initial, and function blocks?

[imphil]

@AbdouTharwat Thanks for looking into the fails, much appreciated!

I pushed a number of commits to address some of the issues that you found plus a couple more, most notably:

• Wrong handling of include multiple Verilog include directories. That's fixed.
• Multi-library VHDL tests were disabled due to the renaming of the SIM variable. Re-enable them.
• I updated the test expectations to address the newly visible vpiAlways blocks that you pointed out, plus some more.

TODOs:

• Re-enabling the VHDL library tests uncovere missing functionality in the QIS/Qrun makefile: the make_lib part in the questa-compat makefile. This makes tests/test_cases/test_vhdl_libraries_multiple fail.
• As you noted, there are some tests still not matching the expectations. Let me know if you have updates on those and I'll try to incorporate them.

[imphil]

Why are we even trying to discover always, initial, and function blocks?

History, I guess :-) I don't think we have much use for these blocks ultimately.

[imphil]

I looked at the runs and that's what I'm getting with the QIS/Qrun flow with Questa 2023.2:

VHDL-VHPI

• Failure in testsuite: 'all' classname: 'test_iteration' testcase: 'recursive_discovery' with parameters 'all':
  AssertionError: assert 66960 == 66959

VHDL-FLI

• Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_read_write' with parameters 'all'
  AttributeError: param_rec contains no object named a
• Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_discover_all' with parameters 'all'
  AssertionError: assert 822 == 856
• Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_direct_constant_indexing' with parameters 'all'
  AttributeError: param_rec contains no object named a

Verilog-VPI

• Failure in testsuite: 'all' classname: 'test_array' testcase: 'test_discover_all' with parameters 'all'
  AssertionError: assert 1095 == 1078
• Failure in testsuite: 'all' classname: 'test_cocotb_array' testcase: 'test_in_vect_packed_packed_unpacked' with parameters 'all'
  test_in_vect_packed_packed_unpacked failed: passed but we expected an error
• Failure in testsuite: 'all' classname: 'test_cocotb_array' testcase: 'test_in_arr_packed_unpacked' with parameters 'all'
  test_in_arr_packed_unpacked failed: passed but we expected an error
• Failure in testsuite: 'all' classname: 'test_cocotb_array' testcase: 'test_in_2d_arr_unpacked' with parameters 'all'
  test_in_2d_arr_unpacked failed: passed but we expected an error

[AbdouTharwat]

Hi @imphil,

Regarding the VPI tests, there are some observations:

test_array

• QIS flow found 1095 instead of 1078 objects, comparing both transcripts the extra 17 objects found by QIS are due to vpiAlways here is the output of those extra objects:

50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[20].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[16].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[17].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[18].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[19].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[21].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[22].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.asc_gen[23].#ALWAYS#182) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[0].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[1].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[2].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[3].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[4].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[5].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[6].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.desc_gen[7].#ALWAYS#195) (GPI_MODULE)
50.00ns INFO cocotb.test ------Found HierarchyObject(array_module.#ALWAYS#111) (GPI_MODULE)

Reviewing analog_module.sv we find:

1. line 182 has generate block with always statement indexing from 16 to 23
2. line 195 has generate block with always statement indexing from 7 to 0
3. line 111 has always statement

In conclusion it appears that QIS shows objects that were not shown by old flow but are correct as shown above

test_cocotb_array

the test was run with LOG_LEVEL DEBUG and for the 3 mentioned tests test_in_vect_packed_packed_unpacked, test_in_arr_packed_unpacked, and test_in_2d_arr_unpacked it appears that old flow failed to find handle for those while new QIS flow succeeded. I am attaching the debug log for the 3 tests both in QIS flow and +acc flow so you can have a look
vpi QIS.txt
vpi +acc.txt

Reviewing the test_cocotb_array.py file we find 3 comments at lines 85, 125, 154 stating that Questa is unable to access elements of a logic array if the last dimension is unpacked (gh-2605) which means it is an open issue and it appears to be solved with new QIS flow

In conclusion it appears that these tests were failing earlier but works well with QIS flow. So they should be modified now that QIS flow is able to access those elements, do you agree?

Finally regarding FLI and VHPI failures, we already opened issues for those and are being investigated, so we are on track.
Are there any failures other than those that we are tracking?

Thanks,

[ktbarrett]

@AbdouTharwat I'm going to fix #3353 then this back and forth should be moot.