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

Added UPF Support #775

Merged
merged 5 commits into from
Apr 5, 2023
Merged

Added UPF Support #775

merged 5 commits into from
Apr 5, 2023

Conversation

derrickqi2003
Copy link
Contributor

@derrickqi2003 derrickqi2003 commented Mar 30, 2023

Added UPF support to hammer. Generates a single power domain UPF. It should do exactly what hasCPFsupport does. Tested it by running PAR on the tapeout vlsi lab and manually feeding into innovus with the innovus command below:
read_power_intent -1801 power_spec.upf
power_spec.pdf
.

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?

Copy link
Contributor

@harrisonliew harrisonliew left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! A couple comments:

  • It seems like these are UPF 2.0 commands. You might want to include a upf_version command in the header. Similarly, this comment may need to reflect that
  • Building on previous point, supporting UPF 3+ could be a switch? That option there could be like upf_2.0 or upf_3.1, etc. if you desire.
  • Is there any corresponding updates needed for the Cadence plugins?

for g_net in ground_nets:
if(g_net.pin != None):
output.append("add_port_state {g} \\".format(g=g_net.name))
output.append("\t-state {default off}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct according to my understanding of the command... instead it should be a voltage of 0. "off" is distinctly a different state?

Copy link
Contributor Author

@derrickqi2003 derrickqi2003 Mar 30, 2023

Choose a reason for hiding this comment

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

Was mistaken. Will fix that as soon as I get to it.

hammer/vlsi/hammer_vlsi_impl.py Outdated Show resolved Hide resolved
@kenhoberkeley
Copy link
Contributor

kenhoberkeley commented Mar 30, 2023

Really nitpicky here but could we switch to f-strings. Might just be nice to stick to for future code.

@derrickqi2003
Copy link
Contributor Author

@kenhoberkeley Added fstrings
@harrisonliew Made the voltage change from "off" to 0.0 & updated the documentation to say IEEE 1801-2009

@harrisonliew harrisonliew added this pull request to the merge queue Apr 5, 2023
Merged via the queue into master with commit 34d2866 Apr 5, 2023
@harrisonliew harrisonliew deleted the power-intent branch April 5, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UPF_CPF power spec related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants