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.

Sign up for GitHub

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

Jump to bottom

xcelium support #1386

Merged
merged 8 commits into from
Mar 16, 2023
Merged

xcelium support #1386

merged 8 commits into from
Mar 16, 2023

Conversation

sagark
Copy link
Member

@sagark sagark commented Mar 8, 2023 •
edited
Loading

Related PRs / Issues:

N/A

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

@sagark sagark changed the title [DO NOT MERGE, WIP] xcelium support xcelium support Mar 9, 2023
abejgonzalez
abejgonzalez reviewed Mar 9, 2023
View reviewed changes
sims/xcelium/Makefile Outdated Show resolved Hide resolved
sims/xcelium/Makefile Show resolved Hide resolved
sims/xcelium/Makefile

$(sim_workdir): $(sim_common_files) $(dramsim_lib) $(EXTRA_SIM_REQS)
rm -rf $(model_dir)
$(XCELIUM) -elaborate $(XCELIUM_OPTS) $(EXTRA_SIM_SOURCES) $(XCELIUM_COMMON_ARGS)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought elaboration and running all happened in 1 step. What is this doing?

Copy link

Choose a reason for hiding this comment

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

Could be useful to use xrun in multi-step like this if you wanted to save a snapshot.

sims/xcelium/Makefile Outdated Show resolved Hide resolved
sims/xcelium/Makefile Outdated Show resolved Hide resolved
sims/xcelium/Makefile
rm -rf $(model_dir)
$(XCELIUM) -elaborate $(XCELIUM_OPTS) $(EXTRA_SIM_SOURCES) $(XCELIUM_COMMON_ARGS)

$(sim_run_tcl): $(sim_workdir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you somehow inline the tcl so that you don't create an extra run tcl script?

Copy link
Contributor

Choose a reason for hiding this comment

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

Track this issue in #1405

harrisonliew
harrisonliew reviewed Mar 9, 2023
View reviewed changes
Copy link
Contributor

@harrisonliew harrisonliew left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems unwise to use VCD for simulations on Chipyard-sized designs. Can you use SHM instead? Perhaps repurpose the USE_VPD flag for fallback to VCD...

@kenhoberkeley
Copy link

Two useful keys when used in conjunction with SHM outputs, -incsize (incremental file size for splitting waveform outputs) and -compress.

@jerryz123
Copy link
Contributor

Seems unwise to use VCD for simulations on Chipyard-sized designs. Can you use SHM instead? Perhaps repurpose the USE_VPD flag for fallback to VCD...

Can we just define a new USE_VCD fallback?

@jerryz123 jerryz123 mentioned this pull request Mar 16, 2023
Open
4 tasks
@jerryz123
Copy link
Contributor

Handle remaining issues in #1405

jerryz123
jerryz123 approved these changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

Fixed the last few comment-related issues. Improving the overall flow will be left to future PRs.

@jerryz123 jerryz123 merged commit ae730db into main Mar 16, 2023
@abejgonzalez abejgonzalez mentioned this pull request Mar 17, 2023
Merged
16 tasks
@jerryz123 jerryz123 deleted the xcelium-support branch March 20, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abejgonzalez abejgonzalez abejgonzalez left review comments

@harrisonliew harrisonliew harrisonliew left review comments

@jerryz123 jerryz123 jerryz123 approved these changes

@kenhoberkeley kenhoberkeley Awaiting requested review from kenhoberkeley

Assignees
No one assigned
Labels
changelog:added
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sagark @kenhoberkeley @jerryz123 @abejgonzalez @harrisonliew