From 3da3492e7f5b323a168fa175c1c80ea6b7ad7dac Mon Sep 17 00:00:00 2001 From: Mohamed Gaber Date: Mon, 16 Sep 2024 16:57:04 +0300 Subject: [PATCH 1/4] Fix regression in `2.1.1` (#548) * `Odb.SetPowerConnections` * Fixed an issue introduced in `2.1.1` where modules that are defined as part of hierarchical netlists would be considered macros and then cause a crash when they are inevitably not found in the design database. * Explicitly mention that macros that are not on the top level will not be connected, and emit warnings if a hierarchical netlist is detected. ## Documentation * Updated macro documentation to further clarify how instances should be named and how names should be added to the configuration. --- Changelog.md | 19 ++++++- docs/source/usage/using_macros.md | 48 ++++++++++++++++- openlane/scripts/odbpy/power_utils.py | 75 +++++++++++++++++++-------- openlane/scripts/odbpy/reader.py | 4 +- openlane/steps/odb.py | 8 ++- pyproject.toml | 2 +- test/steps/all | 2 +- test/steps/excluded_step_tests | 1 - 8 files changed, 129 insertions(+), 30 deletions(-) diff --git a/Changelog.md b/Changelog.md index edbc822e9..fdab89a33 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,23 @@ ## Documentation --> +# 2.1.5 + +## Steps + +* `Odb.SetPowerConnections` + + * Fixed an issue introduced in `2.1.1` where modules that are defined as part + of hierarchical netlists would be considered macros and then cause a crash + when they are inevitably not found in the design database. + * Explicitly mention that macros that are not on the top level will not be + connected, and emit warnings if a hierarchical netlist is detected. + +## Documentation + +* Updated macro documentation to further clarify how instances should be named + and how names should be added to the configuration. + # 2.1.4 ## Steps @@ -27,7 +44,7 @@ regardless the information in the SDC file. * For backwards compatibility, `STAPrePNR` unsets all propagated clocks and the rest set all propagated clocks IF the SDC file lacks the strings - `set_propagated_clock` or `unset_propagated_clock`. + `set_propagated_clock` or `unset_propagated_clock`. # 2.1.3 diff --git a/docs/source/usage/using_macros.md b/docs/source/usage/using_macros.md index 83036406d..37ffe5d20 100644 --- a/docs/source/usage/using_macros.md +++ b/docs/source/usage/using_macros.md @@ -154,7 +154,7 @@ would be declared as: } ``` -```{admonition} On Instance Names +````{admonition} On Instance Names :class: tip Instance names should match the name as instantiated in Verilog, i.e., @@ -163,7 +163,53 @@ without escaping any characters for layout formats. For example, if you instantiate an array of macros `spm spm_inst[1:0]`, the first instance's name should be `spm_inst[0]`, not `spm_inst\[0\]` or similar. + +--- + +If your macros are within other instances, the names get a bit more complicated +depending on whether you flatten the hierarchy during synthesis or not. It is +the designer's responsibility to use either notation in the Instances dictionary +depending on which hierarchy mode is utilized. + +**Hierarchy flattened during Synthesis (default, recommended)** + +When flattening the hierarchy, Yosys will rename instances to use the dot +notation, i.e, the name of an instance inside another instance will be +`instance_a.instance_b`. Note that they are the names of instances and not +the names of modules. + +**Hierarchy maintainted during Synthesis** + +Using macros with hierarchy is maintained during Synthesis is +**currently unsupported** in OpenLane, as the netlist is flattened upon moving +into OpenROAD as its hierarchical support is rather experimental. This will be +remedied in a future update to OpenLane. + +In the meantime, if you do choose to keep the hierarchy after Synthesis +regardless by using the attribute `(* keep_hierarchy = "TRUE" *)`, you're on +your own, but here are a couple tips. +* Naming will fall upon OpenROAD, which use the `/` notation regardless of your + technology LEF's divider characters property, i.e., `instance_a/instance_b`. + Note once again that are the names of instances and not the names of modules. +* The power and ground pins of the macro must match that of the standard cell + library as `Odb.SetPowerConnections` won't work for these macros. + +**Please do not do this:** + +Please do not use any of these characters in instance names by using the `\` +prefix, as the example below shows: + +* `/` +* `[]` +* `:` + +Using these is considered **undefined behavior** and the flow will likely crash +because of assumptions about the hierarchy involving these characters. + +```verilog +CellType \instance/name (…); ``` +```` ## STA diff --git a/openlane/scripts/odbpy/power_utils.py b/openlane/scripts/odbpy/power_utils.py index 1e96ca3b1..5e06541e7 100644 --- a/openlane/scripts/odbpy/power_utils.py +++ b/openlane/scripts/odbpy/power_utils.py @@ -33,7 +33,7 @@ class Design(object): @dataclass class Instance: name: str - module: str + module_name: str power_connections: Dict[str, str] ground_connections: Dict[str, str] @@ -41,16 +41,21 @@ def __init__(self, reader: OdbReader, yosys_dict: dict) -> None: self.reader = reader self.design_name = reader.block.getName() self.yosys_dict = yosys_dict - self.yosys_design_object = yosys_dict["modules"][self.design_name] self.pins_by_module_name: Dict[str, Dict[str, odb.dbMTerm]] = {} - self.verilog_net_name_by_bit: Dict[int, str] = functools.reduce( - lambda a, b: {**a, **{bit: b[0] for bit in b[1]["bits"]}}, - self.yosys_design_object["netnames"].items(), - {}, - ) + self.verilog_net_names_by_bit_by_module: Dict[str, Dict[int, str]] = {} self.nets_by_net_name = {net.getName(): net for net in reader.block.getNets()} + def get_verilog_net_name_by_bit(self, top_module: str, target_bit: int): + yosys_design_object = self.yosys_dict["modules"][top_module] + if top_module not in self.verilog_net_names_by_bit_by_module: + self.verilog_net_names_by_bit_by_module[top_module] = functools.reduce( + lambda a, b: {**a, **{bit: b[0] for bit in b[1]["bits"]}}, + yosys_design_object["netnames"].items(), + {}, + ) + return self.verilog_net_names_by_bit_by_module[top_module][target_bit] + def get_pins(self, module_name: str) -> Dict[str, odb.dbMTerm]: if module_name not in self.pins_by_module_name: master = self.reader.db.findMaster(module_name) @@ -83,10 +88,17 @@ def is_ground(self, module_name: str, pin_name: str) -> bool: return module_pins[pin_name].getSigType() == "GROUND" - def extract_pg_pins(self, cell_name: str) -> dict: - cells = self.yosys_design_object["cells"] - module = cells[cell_name]["type"] - master = self.reader.db.findMaster(module) + def extract_pg_pins(self, top_module: str, cell_name: str) -> dict: + yosys_design_object = self.yosys_dict["modules"][top_module] + cells = yosys_design_object["cells"] + module_name = cells[cell_name]["type"] + master = self.reader.db.findMaster(module_name) + if master is None: + print( + f"[ERROR] Could not find master for cell type '{module_name}' in the database." + ) + exit(-1) + lef_pg_pins = [] for pin in master.getMTerms(): if pin.getSigType() in ["POWER", "GROUND"]: @@ -102,32 +114,53 @@ def extract_pg_pins(self, cell_name: str) -> dict: connection_bits = connections[pin_name] if len(connection_bits) != 1: print( - f"[ERROR] Unexpectedly found more than one bit connected to {sigtype} pin {module}/{pin_name}." + f"[ERROR] Unexpectedly found more than one bit connected to {sigtype} pin {module_name}/{pin_name}." ) exit(-1) connection_bit = connection_bits[0] - connected_to_v = self.verilog_net_name_by_bit[connection_bit] + connected_to_v = self.get_verilog_net_name_by_bit( + top_module, + connection_bit, + ) (power_pins if sigtype == "POWER" else ground_pins)[ pin_name ] = connected_to_v return power_pins, ground_pins - def extract_instances(self) -> List["Design.Instance"]: + def extract_instances( + self, + top_module: str, + prefix: str = "", + ) -> List["Design.Instance"]: + yosys_design_object = self.yosys_dict["modules"][top_module] instances = [] - cells = self.yosys_design_object["cells"] + cells = yosys_design_object["cells"] for cell_name in cells.keys(): - module = cells[cell_name]["type"] - if module.startswith("$"): + module_name = cells[cell_name]["type"] + if module_name.startswith("$"): # yosys primitive continue - power_pins, ground_pins = self.extract_pg_pins(cell_name) + if module_name in self.yosys_dict["modules"]: + print( + f"[WARNING] Macros inside hierarchical netlists are not currently supported in OpenLane: skipping submodule '{cell_name}' of type '{module_name}'." + ) + continue + # sub_instances = self.extract_instances( + # self.yosys_dict["modules"][module_name], + # prefix=prefix + f"{cell_name}/", + # ) + # instances += sub_instances + power_pins, ground_pins = self.extract_pg_pins( + top_module, + cell_name, + ) instances.append( Design.Instance( - name=cell_name, + name=prefix + cell_name, ground_connections=ground_pins, power_connections=power_pins, - module=module, + module_name=module_name, ) ) @@ -244,7 +277,7 @@ def set_power_connections(input_json, reader: OdbReader): yosys_dict = json.loads(design_str) design = Design(reader, yosys_dict) - macro_instances = design.extract_instances() + macro_instances = design.extract_instances(design.design_name) for instance in macro_instances: for pin in instance.power_connections.keys(): net_name = instance.power_connections[pin] diff --git a/openlane/scripts/odbpy/reader.py b/openlane/scripts/odbpy/reader.py index 762b778e3..fb6b58f55 100644 --- a/openlane/scripts/odbpy/reader.py +++ b/openlane/scripts/odbpy/reader.py @@ -86,8 +86,8 @@ def __init__(self, *args, **kwargs): self.instances = self.block.getInsts() busbitchars = re.escape("[]") # TODO: Get alternatives from LEF parser - dividerchar = re.escape("/") # TODO: Get alternatives from LEF parser - self.escape_verilog_rx = re.compile(rf"([{dividerchar + busbitchars}])") + # dividerchar = re.escape("/") # TODO: Get alternatives from LEF parser + self.escape_verilog_rx = re.compile(rf"([{busbitchars}])") def add_lef(self, new_lef): self.ord_tech.readLef(new_lef) diff --git a/openlane/steps/odb.py b/openlane/steps/odb.py index 0060af08c..0f2a78844 100644 --- a/openlane/steps/odb.py +++ b/openlane/steps/odb.py @@ -265,8 +265,12 @@ def run(self, state_in, **kwargs) -> Tuple[ViewsUpdate, MetricsUpdate]: @Step.factory.register() class SetPowerConnections(OdbpyStep): """ - Uses JSON netlist and module information in Odb to add global power connections - for macros in a design. + Uses JSON netlist and module information in Odb to add global power + connections for macros at the top level of a design. + + If the JSON netlist is hierarchical (e.g. by using a keep hierarchy + attribute) this Step emits a warning and does not attempt to connect any + macros instantiated within submodules. """ id = "Odb.SetPowerConnections" diff --git a/pyproject.toml b/pyproject.toml index c02bdce43..d156ae581 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "openlane" -version = "2.1.4" +version = "2.1.5" description = "An infrastructure for implementing chip design flows" authors = ["Efabless Corporation and Contributors "] readme = "Readme.md" diff --git a/test/steps/all b/test/steps/all index c970ed0d2..920c54b1c 160000 --- a/test/steps/all +++ b/test/steps/all @@ -1 +1 @@ -Subproject commit c970ed0d2b8eeef1cdf40fc57b9b8f22a17b6faa +Subproject commit 920c54b1c7994cc1bfc05670c590fbda9f359b99 diff --git a/test/steps/excluded_step_tests b/test/steps/excluded_step_tests index ff09fc935..e69de29bb 100644 --- a/test/steps/excluded_step_tests +++ b/test/steps/excluded_step_tests @@ -1 +0,0 @@ -odb.setpowerconnections/003-fail_mismatched_view From f74e88d7a7ba11adaf96f719b0d788651a59ce2e Mon Sep 17 00:00:00 2001 From: htfab <91572108+htfab@users.noreply.github.com> Date: Sun, 22 Sep 2024 11:22:40 +0200 Subject: [PATCH 2/4] Fix arguments passed to Yosys hilomap (#552) * `Yosys.Synthesis` * Fixed bug where `hilomap` command was invoked incorrectly --- Changelog.md | 8 ++++++++ openlane/scripts/yosys/synthesize.tcl | 2 +- pyproject.toml | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index fdab89a33..a2f80cfd3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,14 @@ ## Documentation --> +# 2.1.6 + +## Steps + +* `Yosys.Synthesis` + + * Fixed bug where `hilomap` command was invoked incorrectly (thanks @htfab!) + # 2.1.5 ## Steps diff --git a/openlane/scripts/yosys/synthesize.tcl b/openlane/scripts/yosys/synthesize.tcl index 0324b11e1..294324a1d 100644 --- a/openlane/scripts/yosys/synthesize.tcl +++ b/openlane/scripts/yosys/synthesize.tcl @@ -180,7 +180,7 @@ proc run_strategy {output script strategy_name {postfix_with_strategy 0}} { setundef -zero - hilomap -hicell $::env(SYNTH_TIEHI_CELL) -locell $::env(SYNTH_TIELO_CELL) + hilomap -hicell {*}[split $::env(SYNTH_TIEHI_CELL) "/"] -locell {*}[split $::env(SYNTH_TIELO_CELL) "/"] if { $::env(SYNTH_SPLITNETS) } { splitnets diff --git a/pyproject.toml b/pyproject.toml index d156ae581..402fabfb0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "openlane" -version = "2.1.5" +version = "2.1.6" description = "An infrastructure for implementing chip design flows" authors = ["Efabless Corporation and Contributors "] readme = "Readme.md" From bc56ad4613f847805c4ebd9e152bd520876c329d Mon Sep 17 00:00:00 2001 From: Uri Shaked Date: Sun, 22 Sep 2024 14:04:46 +0300 Subject: [PATCH 3/4] Fix Odb.RemoveRoutingObstructions failing with non-integer coordinates (#553) * `Odb.Remove*Obstructions` * Rework obstruction matching code to not use IEEE 754 in any capacity * Fixed bug where non-integral obstructions would not be matched correctly (thanks @urish!) Co-authored-by: Mohamed Gaber --- Changelog.md | 10 ++++++++++ openlane/scripts/odbpy/defutil.py | 31 ++++++++++++++----------------- pyproject.toml | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/Changelog.md b/Changelog.md index a2f80cfd3..e49853a6b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,16 @@ ## Documentation --> +# 2.1.7 + +## Steps + +* `Odb.Remove*Obstructions` + + * Rework obstruction matching code to not use IEEE 754 in any capacity + * Fixed bug where non-integral obstructions would not be matched correctly + (thanks @urish!) + # 2.1.6 ## Steps diff --git a/openlane/scripts/odbpy/defutil.py b/openlane/scripts/odbpy/defutil.py index 70452f8eb..462611a71 100644 --- a/openlane/scripts/odbpy/defutil.py +++ b/openlane/scripts/odbpy/defutil.py @@ -17,6 +17,7 @@ import os import re import sys +from decimal import Decimal from reader import click_odb, click from typing import Tuple, List @@ -456,7 +457,7 @@ def replace_instance_prefixes(original_prefix, new_prefix, reader): cli.add_command(replace_instance_prefixes) -def parse_obstructions(obstructions): +def parse_obstructions(obstructions) -> List[Tuple[str, List[int]]]: RE_NUMBER = r"[\-]?[0-9]+(\.[0-9]+)?" RE_OBS = ( r"(?P\S+)\s+" @@ -483,7 +484,7 @@ def parse_obstructions(obstructions): sys.exit(FORMAT_ERROR) else: layer = m.group("layer") - bbox = [float(x) for x in m.group("bbox").split()] + bbox = [Decimal(x) for x in m.group("bbox").split()] obs_list.append((layer, bbox)) return obs_list @@ -526,12 +527,8 @@ def add_obstructions(reader, input_lefs, obstructions): ) @click_odb def remove_obstructions(reader, input_lefs, obstructions): - obs_list = parse_obstructions(obstructions) + dbu: int = reader.tech.getDbUnitsPerMicron() existing_obstructions: List[Tuple[str, List[int], odb.dbObstruction]] = [] - dbu = reader.tech.getDbUnitsPerMicron() - - def to_microns(x): - return int(x / dbu) for odb_obstruction in reader.block.getObstructions(): bbox = odb_obstruction.getBBox() @@ -539,28 +536,28 @@ def to_microns(x): ( bbox.getTechLayer().getName(), [ - to_microns(bbox.xMin()), - to_microns(bbox.yMin()), - to_microns(bbox.xMax()), - to_microns(bbox.yMax()), + bbox.xMin(), + bbox.yMin(), + bbox.xMax(), + bbox.yMax(), ], odb_obstruction, ) ) - for obs in obs_list: - layer = obs[0] - bbox = obs[1] - bbox = [int(x * dbu) for x in bbox] + for obs in parse_obstructions(obstructions): + layer, bbox = obs + bbox = [int(x * dbu) for x in bbox] # To dbus found = False if reader.tech.findLayer(layer) is None: print(f"[ERROR] layer {layer} doesn't exist.", file=sys.stderr) sys.exit(METAL_LAYER_ERROR) for odb_obstruction in existing_obstructions: - if odb_obstruction[0:2] == obs: + odb_layer, odb_bbox, odb_obj = odb_obstruction + if (odb_layer, odb_bbox) == (layer, bbox): print(f"Removing obstruction on {layer} at {bbox} (DBU)…") found = True - odb.dbObstruction_destroy(odb_obstruction[2]) + odb.dbObstruction_destroy(odb_obj) if found: break if not found: diff --git a/pyproject.toml b/pyproject.toml index 402fabfb0..10165b4ea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "openlane" -version = "2.1.6" +version = "2.1.7" description = "An infrastructure for implementing chip design flows" authors = ["Efabless Corporation and Contributors "] readme = "Readme.md" From 92c2a552a846d4c340a8abc24d3d20732b3b6877 Mon Sep 17 00:00:00 2001 From: Kareem Farid Date: Mon, 23 Sep 2024 14:03:51 +0300 Subject: [PATCH 4/4] Fix Zero-Slack Paths Being Counted As Violations (#556) * `OpenROAD.STA*PnR` * Fixed a bug in STA metrics where paths with exactly zero slack are counted as violations --- Changelog.md | 11 ++++++++++- openlane/scripts/openroad/sta/corner.tcl | 20 ++++++++++++++++++++ pyproject.toml | 2 +- test/steps/all | 2 +- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/Changelog.md b/Changelog.md index e49853a6b..022ea4666 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,15 @@ ## Documentation --> +# 2.1.8 + +## Steps + +* `OpenROAD.STA*PnR` + + * Fixed a bug in STA metrics where paths with exactly zero slack are counted + as violations + # 2.1.7 ## Steps @@ -29,7 +38,7 @@ ## Steps * `Yosys.Synthesis` - + * Fixed bug where `hilomap` command was invoked incorrectly (thanks @htfab!) # 2.1.5 diff --git a/openlane/scripts/openroad/sta/corner.tcl b/openlane/scripts/openroad/sta/corner.tcl index 5a85b5edf..d9352205e 100644 --- a/openlane/scripts/openroad/sta/corner.tcl +++ b/openlane/scripts/openroad/sta/corner.tcl @@ -265,6 +265,11 @@ foreach path $hold_violating_paths { set start_pin [get_property $path startpoint] set end_pin [get_property $path endpoint] set kind "[get_path_kind $start_pin $end_pin]" + set slack [get_property $path slack] + + if { $slack >= 0 } { + continue + } incr total_hold_vios if { "$kind" == "reg-reg" } { @@ -279,6 +284,11 @@ foreach path $hold_paths { set start_pin [get_property $path startpoint] set end_pin [get_property $path endpoint] set kind "[get_path_kind $start_pin $end_pin]" + set slack [get_property $path slack] + + if { $slack >= 0 } { + continue + } if { "$kind" == "reg-reg" } { set slack [get_property $path slack] @@ -293,6 +303,11 @@ foreach path $setup_violating_paths { set start_pin [get_property $path startpoint] set end_pin [get_property $path endpoint] set kind "[get_path_kind $start_pin $end_pin]" + set slack [get_property $path slack] + + if { $slack >= 0 } { + continue + } incr total_setup_vios if { "$kind" == "reg-reg" } { @@ -307,6 +322,11 @@ foreach path $setup_paths { set start_pin [get_property $path startpoint] set end_pin [get_property $path endpoint] set kind "[get_path_kind $start_pin $end_pin]" + set slack [get_property $path slack] + + if { $slack >= 0 } { + continue + } if { "$kind" == "reg-reg" } { set slack [get_property $path slack] diff --git a/pyproject.toml b/pyproject.toml index 10165b4ea..81e1b25b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "openlane" -version = "2.1.7" +version = "2.1.8" description = "An infrastructure for implementing chip design flows" authors = ["Efabless Corporation and Contributors "] readme = "Readme.md" diff --git a/test/steps/all b/test/steps/all index 920c54b1c..d2ce1465d 160000 --- a/test/steps/all +++ b/test/steps/all @@ -1 +1 @@ -Subproject commit 920c54b1c7994cc1bfc05670c590fbda9f359b99 +Subproject commit d2ce1465d11574bd0c371c454c280baa39a37d79