-
Notifications
You must be signed in to change notification settings - Fork 263
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
Add Cadence Xcelium support #728
base: master
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.
I added two suggestions, apply them to the other options of the same name.
Which one @cmarqu ? both of them? (I have no access to incisive). |
I broke something. In master,
|
To be able to launch a test, I removed Incisive from my local copy. I have complaints about |
Ah, the GitHub GUI is not really good here; I commented specifically on the line |
You are adding the |
f11c212
to
7323c5b
Compare
@cmarqu taking into account that Xcelium is the newest, and Incisive could be deprecated sometime in the future, I defined the CLI arguments for both at xcelium.py. I put a note into incisive.py. What do you think? |
Solved. |
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.
Looks good, however, I am unsure about duplicating the sim_if source. I think that the strategy from #684 about using some private fields (executable_name
, option_prefix
) might be applied here. As @LarsAsplund commented in #684 (comment), we want to support both simulators explicitly, meaning that users should be able to select incisive
or xcelium
. However, from an implementation point of view we should be able to use a single class or, at least, have a common class and two inheriting from it. Say:
from .cadence import IncisiveInterface, XceliumInterface
cc0674f
to
3804ced
Compare
I applied two fixes in VHDL packages (based on #325 (comment) and from #684) and now when I run under
Seems to be almost there :-D |
Well, we were testing with @eine and the context must be replaced by the use clauses, and it works. At least part of the fixup will be needed. |
f85ee19
to
a2774bd
Compare
@rodrigomelo9 we picked one of the commits in #731. Can you please rebase? |
Incisive and irun were replaced by Xcelium and xrun.
…ath not supported)
Added to ignore a too-many-branches complain of pylint due to the additions.
a2774bd
to
cfc070a
Compare
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.
return "-v200x -extv200x" | ||
|
||
if vhdl_standard == VHDL.STD_2008: | ||
return "-v200x -extv200x -inc_v200x_pkg" |
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.
I took it from #684 but I'm not sure it's necessary. If yes, must be also applied to VHDL.SDT_2002? Must be applied to Incisive?
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.
Ok, I found some answers...
To successfully compile the code with the VHDL protected types, a second option in addition to -v200x is needed. Please add -extv200x to your compiler command line. This switch is not documented.
To compile the latest VHDL 2008 constructs, a new command line option, -inc_v200x_pkg, must be used. This is in addition to the -v200x option.
So these additions seem ok.
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.
They are okay for Xcelium, but I'm not so sure about a "true" Incisive installation - after all, the last Incisive release was 2015.
Thinking about it, I wouldn't be surprised if such an old Incisive cannot run a modern vunit at all anymore, so maybe we can and should remove Incisive support completely.
I'll try to check what the status there is.
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.
Ok, so Incisive 15.20 doesn't accept -inc_v200x_pkg
and without it doesn't compile vunit/vhdl/logging/src/ansi_pkg.vhd
(which I'm sure is just the first file of many with problems).
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.
Ok. Taking this into account seems ok to maintain xcellium.py as parent class and incisive.py could heritage from it?
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.
Ok, so Incisive 15.20 doesn't accept
-inc_v200x_pkg
and without it doesn't compilevunit/vhdl/logging/src/ansi_pkg.vhd
(which I'm sure is just the first file of many with problems).
@cmarqu, did you try running Incisive or did you find it in the documentation?
In case you can run it, would you please try the user_guide for 1993 I proposed in #737? It might be desirable to support running the subset of tests that is compatible with 1993, for those simulators which support VHDL 1993 properly, but don't support VHDL 2008 (yet).
Until now, I believe we had not run into that use case. All other simulators support both 1993 and 2008, or don't support either of them (XSIM cannot handle VUnit's 1993 codebase). However, strictly it should be possible to use VUnit as a "project builder and test runner", despite many utility libraries not being available without 2008 support.
Trying the verilog user guide, I have this message:
Which disappears if I include the arg |
Hi @umarcor I perform the change to inherit Incisive from Xcelium, but I have pytest issues where I need help. I open rodrigomelo9#1 to work on that. |
@LarsAsplund @cmarqu @umarcor here is an update, to define possible next steps. In the current state of this PR:
|
Sorry, I can't test it for the next three weeks or so. |
Ok @cmarqu let me know if you take a look. I will be working into a unified Cadence interface to be inherited by Incisive and Xcelium. |
@rodrigomelo9 I'm wondering if you've had a chance to get this further down the path of working? I've rebased your branch on the latest master and fixed one decode issue in commits over on my branch here: https://github.com/Hardolaf/vunit/tree/add/xcelium-support |
Sorry, I did not further development on that. I'm not a VUnit user. It would be great if it reaches |
I made an attempt to run VUnit with Xcelium 21.09 but still some issues. I did the acceptance test and the current status is 28 failed, 13 passes and 8 skipped. I'm also running the examples. Two issues happens quite often:
There is also a problem when calling VUnit in batch mode : the test ends but Xcelium does not exit (simulation stops still). I had to add the '-exit' argument to xrun call to fix this. I will try in the coming weeks to work on that. |
I did a bit of progress since yesterday, I'm focusing on making the uart example working. The implicit conversion between std_logic_vector and std_ulogic_vector seems not to be well supported by Xcelium. For instance at many places there is this kind of construct:
And it triggers this error:
To fix this, we can add an explicit cast but it is a bit painful and I'm not sure it is the best way to proceed (even this is what I did as a quick fix). This affects many files in the OSVVM and verification components libraries. Another issue is the unconstrained type inside record, when the record type is used as a subprogram parameter. It is for instance the case in axi_stream_pkg.vhd at line 313. Based on Cadence support it is a known limitation... |
@VinieBerry I believe we can fix the OSVVM issues for some versions of Xcelium by checking to see if there is a Cadence provided version of OSVVM located at |
Concerning the
While:
This error happens 294 times in the acceptance test. I'm wondering if it is actually related to Xcelium simulator ? Does someone have already deal with this kind of issue ? |
The root cause of the
I don't know why it seems to only happen with mocked logger (to be confirmed). For now my fix is to force to True the attribute value. Would it be possible to condition this depending on the simulator ? |
Good news, I reported the issue to Cadence support and the answer is that it has been fixed and will be supported in a next release. This will be a great improvment since as of today we cannot compile all the VIP components (and therefore the associated testbenches). |
Doing more testing, I'm running into two issues:
|
This is partially unexpected... It is expected for multiple |
It does, via the |
I'll take a look at making that change. I've done it in the past but haven't had the bandwidth to do it yet due to other work obligations. |
I took a look at adding support for this and it appears to be a non-trivial change to VUnit due to Xcelium not supporting certain options within a
Now, I say that this is a non-trivial change because VUnit currently tracks compilation options on a per-file basis rather than on a per-invocation basis (see
I personally believe that option 1 is the best option in terms of a resolution and adding proper support for this (and it is lower runtime compared to option 2 by a lot) but in terms of implementing the change, I don't know if I have the requisite knowledge of VUnit's internals to safely make this change for all simulators especially as I only have one simulator currently able to run on my development machine. If I did go ahead with it, I could get one of our Python devs to at least review the code before I try to push it up, but even that will only have limited utility as they would be completely unfamiliar with the code base. So I'm really just looking for opinions as to what people think is the best path forward on adding this support. If it would help to see the code that I currently have, I can push it to my branch and link it here. |
I can't help with your real question but the first error could be circumvented by setting the environment variable |
I know that in theory that should work, but I have already opened an issue with Cadence about |
I’m digging on the communication library support after having many errors and/or crashes. After many attempts I figured out that an issue arises from the com_messenger_pkg. It seems that at some moments the name() procedure (line 312 of com_messenger.vhd) is called while the actors variable is null. Therefore, we have:
As a workaround I have added a condition to avoid this case:
This solves the problem, but now I get the same kind of issues when calling the to_string(msg) function... I'm not enough familiar with the internal cooking of the communication library to deeply understand the root cause. What I can say is that my actor is well created at startup (I suppose):
Any ideas ? |
On this side, I figured out that Xcelium is able to switch from std_logic/std_ulogic implicitely (procedures with std_logic or std_ulogic parameter have the same signature). Hovewer it is not the case for std_logic_vector/std_ulogic_vector. Thus, I have added the same functions/procedures for std_logic_vector than for std_ulogic_vector in com_types_pkg and queue_pkg (and associated files). The explicit casting is then hidden from the user. |
Hello! I have two simple questions:
|
My attempt to provide support to Xcelium, based on previous Incisive support.