Skip to content
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

[RFC][WIP] Fix failing ghdl test #950

Closed
wants to merge 1 commit into from
Closed

[RFC][WIP] Fix failing ghdl test #950

wants to merge 1 commit into from

Conversation

garzetti
Copy link
Contributor

While trying to fix the ghdl issues with dec_viterbi, another error popped up, this time apparently with VPI. Somehow, here cocotb fails to get the element 0. Tracking down the issue, it seems that the problem comes from here, as the returned new_handle is 0.

Unfortunately, I'm not yet able to track it down by myself, any suggestion? This seems to be quite important, as it is currently impossible to reference any instance generated with the "for ... generate" construct.

Thanks.

@garzetti
Copy link
Contributor Author

Now Travis with GHDL starts but still fails some test units. Please check the log and let me know.

@imphil
Copy link
Member

imphil commented Jun 16, 2019

Please check the log and let me know.

Can't you access https://travis-ci.org/potentialventures/cocotb/jobs/545147125?

Essentially, it's

/home/travis/build/potentialventures/cocotb/tests/test_cases/test_iteration_vhdl/../../../tests/designs/viterbi_decoder_axi4s/src/generic_sp_ram.vhd:16:10:error: unit "std_logic_unsigned" not found in library "ieee"
/home/travis/build/potentialventures/cocotb/tests/test_cases/test_iteration_vhdl/../../../tests/designs/viterbi_decoder_axi4s/src/generic_sp_ram.vhd:16:10:error: (use --ieee=synopsys for non-standard synopsys packages)
/home/travis/build/potentialventures/cocotb/tests/test_cases/test_iteration_vhdl/../../../tests/designs/viterbi_decoder_axi4s/src/generic_sp_ram.vhd:42:14:error: entity 'generic_sp_ram' was not analysed

@cmarqu
Copy link
Contributor

cmarqu commented Jun 16, 2019

So adding --ieee=synopsys to the Makefile may make it work.

This is an initial attempt to fix ghdl errors with dec_viterbi.
@garzetti
Copy link
Contributor Author

Can't you access https://travis-ci.org/potentialventures/cocotb/jobs/545147125?

With this commit the test starts, but fails with IndexError: gen_branch_distance contains no object at index 0, have a look at this.

So adding --ieee=synopsys to the Makefile may make it work.

Already done. Anyways, this was only the first step; thanks to b36337b the design it is now consistent and can be elaborated and run by GHDL.
However, it seems that the VPI interface fails in passing some objects (e.g. array object, constant object). Now it is difficult for me to investigate further, maybe someone with more experience in VPI interface can have a look.

In short now the test fails when it tries to access any element in HierarchyArrayObject or ModifiableArrayObject, so the test fails here

@themperek
Copy link
Contributor

Any idea this is an issue of cocotb or ghdl?

@garzetti
Copy link
Contributor Author

For me it is related to GHDL because with Questa Modelsim all the tests pass without error. Moreover, i have checked the dut._sub_handle_key after a manual _discover_all() and the fetched elements are more consistent using Questa respect to GHDL

@imphil
Copy link
Member

imphil commented Jun 18, 2019

Without looking deeper into this, this could be both a cocotb bug or a GHDL bug. GHDL is special in that it uses VPI, while any other VHDL simulator uses VHPI. I wouldn't therefore rule out bugs in this area as well. However, VPI support is pretty stable in cocotb and very well tested, so a GHDL bug seems more likely.

How to debug further:

  • Run cocotb with full debugging information (set COCOTB_LOG_LEVEL=debug in your environment) to get all VPI calls
  • If you see VPI calls failing report a bug to GHDL. It's hard to double-check the GHDL behavior against any other simulator, as all other simulators use VHPI for VHDL.
  • Feel free to CC me on GHDL bugs and I can try to help out if needed.

(Looks like there are other GHDL bugs open on VPI, e.g. this one ghdl/ghdl#762. Maybe you can look through their bug reports as well?)

@eric-wieser
Copy link
Member

Needs close / reopening to rerun travis

@themperek themperek closed this Jun 21, 2019
@themperek themperek reopened this Jun 21, 2019
@imphil imphil added the status:needs-info feedback from someone is required. The issue/PR text gives more details. label Jun 25, 2019
@garzetti
Copy link
Contributor Author

As you can see from travis this little PR only uncovers more errors. Unfortunately i can not go deeper in ghdl and cocotb by myself at the moment. Please let me know if you find something else

@themperek
Copy link
Contributor

I think this went in with #1052 @garzetti

Can I close this? The rest seems to be on ghdl side.

@garzetti
Copy link
Contributor Author

Yes, you can close.

@themperek
Copy link
Contributor

@garzetti You ware first but I have missed this due to [WIP] sorry and thank you!

@themperek themperek closed this Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs-info feedback from someone is required. The issue/PR text gives more details.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants