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

Diode Insertion Overhaul #1686

Merged
merged 44 commits into from
Mar 27, 2023
Merged

Conversation

kareefardi
Copy link
Collaborator

@kareefardi kareefardi commented Mar 20, 2023

- Deprecate DIODE_INSERTION_STRATEGY.
- Remove DIODE_INSERTION_STRATEGY 2, 1, and 5
+ Add GRT_REPAIR_ANTENNAS
+ Add HEURISTIC_ANTENNA_THRESHOLD
+ Add RUN_HEURISTIC_DIODE_INSERTION
+ Add DIODE_ON_PORTS
+ Add HEURISITIC_ANTENNA_INSERTION_MODE
~ Update benchmark results for SW_HD
~ Apply DIODE_PADDING in dpl_cell_pad which also runs after RUN_HEURISTIC_DIODE_INSERTION

run_designs.py:
  ~ Change default config to `config` instead of `config.json` to allow for designs with
  tcl default config
  ~ Change logging format
  + Print SUCCESS when a design is finished
  ~ Use extra parameters `params.keys()` instead of `ConfigHandler.get_header()` to build
   report csv header. This fixes inconsistencies between csv header and values reported

compare_regression_design.py:
  ~ Change metric name antenna_violations -> pin_antenna_violations
  ~ Handle "bad" encoding of csv report files
  ~ Quit when a report is perceived as invalid
  ~ Don't print output file name to stderr
  
compare_regression_reports.py:
  ~ Change metric name antenna_violations -> pin_antenna_violation
  ~ Handle "bad" encoding of csv report files

config.py:
  ~ Sort result from get_config_for_run and configuration_values for consistency
  ~ All get_config_for_run to get the full config
  
~ Fix antenna violations net extraction in `extract_antenna_violators.py`
~ Fix fetching antenna violation count in `generate_reports.py`

report.py:
  ~ Split "metric" antenna_violations to pin_antenna_violations and
  net_antenna_violations as reported by openroad antenna checker
  ~ Add Non-phyCells 
  ~ Add TotalCells
  ~ Rename cell_count to synth_cell_count to avoid confusion with TotalCells
  ~ Calculate cells_per_mm based on Non-phyCells instead of synth_cell_count
  ~ Rename 

scripts/report/report.py Outdated Show resolved Hide resolved
scripts/report/report.py Outdated Show resolved Hide resolved
scripts/config/config.py Outdated Show resolved Hide resolved
@donn
Copy link
Collaborator

donn commented Mar 22, 2023

Just wanna say, it's an incredible power move to review your own PR and then fix it, absolutely mental

Copy link
Collaborator

@donn donn left a comment

Choose a reason for hiding this comment

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

I'm also wondering if adding diodes to I/Os should be a separate step from the heuristic inserter altogether. Since we're not in THAT much of a hurry, might as well do this right?

docs/source/reference/configuration.md Outdated Show resolved Hide resolved
docs/source/reference/configuration.md Outdated Show resolved Hide resolved
run_designs.py Show resolved Hide resolved
@kareefardi kareefardi marked this pull request as ready for review March 26, 2023 19:29
@donn donn changed the title Antenna fixes Diode Insertion Overhaul Mar 26, 2023
@donn donn self-requested a review March 26, 2023 19:36
donn
donn previously approved these changes Mar 26, 2023
Copy link
Collaborator

@donn donn left a comment

Choose a reason for hiding this comment

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

LGTM- will merge after CI

Copy link
Collaborator

@donn donn left a comment

Choose a reason for hiding this comment

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

Let the sky falllll

@donn donn merged commit 2e09573 into The-OpenROAD-Project:master Mar 27, 2023
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