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

[xcelium] updated xcelium flow to a working state #273

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rasmus-madsen
Copy link

the previous state of the xcelium flow was not working correctly.
I have created a new flow based on the existing flow - adding a lot more freedom for the user.

before there was a onestop for all flow - where compile and simulation was always executed.
the new flow breaks this into the natural steps of compile/elaborate
and another step for the simulation which is very useful if anywant is running a regression sweep or just want to update a c library or similar and don't want to be forced to recompile the full RTL code.

I also created the option of adding a prefix to the run command.
which is useful when using submission queues.

let me know what you guys think.

@rasmus-madsen
Copy link
Author

@imphil @olofk
any feed back on this?

Copy link
Owner

@olofk olofk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks mostly fine except for a few smaller things. With that said however, I'm closing in on a larger refactoring (as described somewhat in https://github.com/olofk/edalize/wiki/Edalize-(Slight-return) and which you can find a PoC in https://github.com/olofk/edalize/tree/esr2. This means that depending on how fast I will land this refactoring, I might need a larger redesign of this code.


(src_files, incdirs) = self._get_fileset_files()
vlog_include_dirs = ['+incdir+'+d.replace('\\','/') for d in incdirs]

libs = []

print("\n DEFINES::", file=sys.stderr)
print(self.vlogdefine, file=sys.stderr)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the logger instead of printing to be inline with other backends

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank I will do that.

{'name' : 'xrun_options',
'type' : 'String',
'desc' : 'Additional run options for xrun'},
'desc' : 'Additional run options for xrun -R'},
]}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new options look cleaner, but I'm worried about current users of the xcelium backend because removing the old options will break things for them. Or do you mean that the xcelium backend didn't work at all before? In that case I guess there is less risk anyone will complain

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless someone would use the generated make as standalone without the generated file list - the previous backend did not work.
so I think it is safe to assume that no one used it.

'type' : 'String',
'desc' : 'Additional options for compilation with xmvlog'},
{'name' : 'xmsim_options',
'desc' : 'prefix for using submission servers like bjob or nc'},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this and use the EDALIZE_LAUNCHER environment variable instead. There is an upcoming refactoring that, among other things, will standardize this for all the backend flows

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants