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

MXHammer experimental version #859

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

MXHammer experimental version #859

wants to merge 13 commits into from

Conversation

Miguel4141
Copy link

Added AMS simulation support through the Xcelium plugin, with hard-coded additions to arguments and constraints as of now.

Related PRs / Issues

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • Change to core Hammer
  • Change to a Hammer plugin
  • Other

Contributor Checklist:

  • Did you set master as the base branch?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you update the poetry.lock file if you updated the requirements in pyproject.toml?
  • (If applicable) Did you add a unit test demonstrating the PR?
  • (If applicable) Did you run this through the e2e integration tests?
  • (If applicable) Did you update the submodules in e2e/ if this feature depends on updated plugins?

hammer/sim/xcelium/__init__.py Outdated Show resolved Hide resolved
hammer/sim/xcelium/__init__.py Outdated Show resolved Hide resolved
hammer/sim/xcelium/__init__.py Outdated Show resolved Hide resolved
hammer/sim/xcelium/__init__.py Outdated Show resolved Hide resolved
@Miguel4141 Miguel4141 marked this pull request as ready for review May 22, 2024 22:44
@@ -3,9 +3,9 @@ vlsi_dir=$(abspath .)


# minimal flow configuration variables
design ?= pass
design ?= level_shifter
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the design you tested on? If so, either add the sources to e2e/ or change this back.

Also, below in this file, please keep the clean target.

Copy link
Contributor

Choose a reason for hiding this comment

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

General comment on this file: you can put most of this info (starting at section 2, since the setup is common) in hammer/sim/xcelium/README.md and then link that file to the main Hammer documentation with a symbolic link like this: https://github.com/ucb-bar/hammer/blob/master/doc/CAD-Tools/Joules.md.

When doing so, remove references to your experimental branch (this is getting merged), the term "MXHammer" (doesn't mean anything), and references to your workspace path. If possible, delineate regular Xcelium (digital sim) setup vs. MX setup when you update this documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is in e2e, it would be OK to put BWRC-specific paths into this file (then link to it from the README document).

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty?

@@ -377,6 +390,80 @@ def generate_sim_tcl(self) -> bool:
f.close()
return True

"""def generate_amscf(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented out?

"""

# Get analog models, schematics from directories specified in extralibs
extralib = self.get_setting("vlsi.technologies.extra_libraries")
Copy link
Contributor

Choose a reason for hiding this comment

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

This key doesn't exist... what are you trying to do/specify here? Do you need fields added to the normal vlsi.technology.extra_libraries key?

class ExtraLibrary(BaseModel):

f = open(runpath, "w+")

# Write Shebang + xrun clean
f.write("#!/bin/csh -f\n#\nxrun -clean \\\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this only work in csh? Generally we want users to use bash family terminals.

@@ -1,11 +1,18 @@
sim.xcelium:
# Tool version (e.g., "XCELIUM2103")
# Tool version (e.g., "XCELIUM2103")
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert extra space

version: "XCELIUM2103"

# Spectre version (e.g., "SPECTRE211")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would note that this is only required for AMS.

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.

2 participants