From ed5647b8c8a83946bb040df4a84c7e6d5e16ce05 Mon Sep 17 00:00:00 2001 From: Kareem Farid Date: Wed, 12 Jul 2023 14:38:52 +0300 Subject: [PATCH] Undocumented Variable CI Workflow Script uses regular expressions to create a set of documents variables and a set of used variables and compare them against each other. Some variables are internal, unexposed and others. These variables are whitelisted. See https://github.com/The-OpenROAD-Project/OpenLane/issues/1889 \+ Add documentation for `QUIT_ON_XOR_ERROR` --- .github/scripts/variables_documentation.py | 227 ++++++++++++++++++ .../variables_documentation_check.yml | 20 ++ configuration/general.tcl | 5 +- configuration/routing.tcl | 2 +- docs/source/reference/configuration.md | 1 + flow.tcl | 3 - scripts/odbpy/remove_buffers.py | 176 -------------- scripts/openroad/common/resizer.tcl | 7 +- scripts/tcl_commands/placement.tcl | 31 --- scripts/tcl_commands/routing.tcl | 3 - 10 files changed, 253 insertions(+), 222 deletions(-) create mode 100644 .github/scripts/variables_documentation.py create mode 100644 .github/workflows/variables_documentation_check.yml delete mode 100644 scripts/odbpy/remove_buffers.py diff --git a/.github/scripts/variables_documentation.py b/.github/scripts/variables_documentation.py new file mode 100644 index 00000000..8874bf54 --- /dev/null +++ b/.github/scripts/variables_documentation.py @@ -0,0 +1,227 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +# Copyright 2023 Efabless Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import subprocess + +documented_elsewhere = """ +CREATE_REPRODUCIBLE_FROM_SCRIPT +""" +internal_variables = """ +CONFIGS +CORE_HEIGHT +CORE_WIDTH +CURRENT_DEF +CURRENT_GDS +CURRENT_GUIDE +CURRENT_INDEX +CURRENT_LIB +CURRENT_NETLIST +CURRENT_ODB +CURRENT_POWERED_NETLIST +CURRENT_SDC +CURRENT_SDF +CURRENT_SPEF +CURRENT_STEP +DEBUG +DESIGN_CONFIG +DESIGN_DIR +ESTIMATE_PARASITICS +EXIT_ON_ERROR +EXT_NETLIST +FLOW_FAILED +GDS_INPUT +GLB_CFG_FILE +GND_NET +GRT_CONGESTION_REPORT_FILE +INSERT_BUFFER_COMMAND +INSERT_BUFFER_COUNTER +IO_READ_DEF +LAST_TIMING_REPORT_TAG +LEC_LHS_NETLIST +LEC_RHS_NETLIST +LOGS_DIR +MAGIC_GDS +MAGIC_SCRIPT +MAX_METAL_LAYER +MC_SDF_DIR +MC_SPEF_DIR +METAL_LAYER_NAMES +OPENLANE_MOUNTED_SCRIPTS_VERSION +OPENLANE_ROOT +OPENLANE_VERSION +OPENROAD_BIN +PACKAGED_SCRIPT_0 +PDKPATH +PROCESS_CORNER +PWD +RCX_DEF +RCX_LEF +RCX_LIB +RCX_RULESET +REPORTS_DIR +REPORT_OUTPUT +RESULTS_DIR +RUN_DIR +RUN_TAG +SAVE_DEF +SAVE_GDS +SAVE_GUIDE +SAVE_LIB +SAVE_MAG +SAVE_NETLIST +SAVE_ODB +SAVE_POWERED_NETLIST +SAVE_SDC +SAVE_SDF +SAVE_SPEF +SCRIPTS_DIR +SCRIPT_DIR +START_TIME +STA_MULTICORNER +STA_PRE_CTS +SYNTH_EXPLORE +SYNTH_SCRIPT +TECH +TERM +TERMINAL_OUTPUT +TMP_DIR +TRACKS_INFO_FILE_PROCESSED +VCHECK_OUTPUT +VDD_NET +WRITE_VIEWS_NO_GLOBAL_CONNECT +TECH_METAL_LAYERS +LIB_SYNTH_COMPLETE +LIB_SYNTH_COMPLETE_NO_PG +LIB_SYNTH_MERGED +LIB_SYNTH_NO_PG +""" +gpio_variables = """ +USE_GPIO_ROUTING_LEF +GPIO_PADS_LEF_CORE_SIDE +GPIO_PADS_VERILOG +""" +to_be_removed = """ +ANTENNA_CHECK_CURRENT_DEF +CTS_CURRENT_DEF +DRC_CURRENT_DEF +LVS_CURRENT_DEF +PARSITICS_CURRENT_DEF +PLACEMENT_CURRENT_DEF +ROUTING_CURRENT_DEF +FAKEDIODE_CELL +VERILOG_STA_NETLISTS +""" +unexposed = """ +HEURISTIC_ANTENNA_INSERTION_MODE +STA_MULTICORNER_READ_LIBS +""" +opts = """ +CELLS_LEF_OPT +DRC_EXCLUDE_CELL_LIST_OPT +GDS_FILES_OPT +NO_SYNTH_CELL_LIST_OPT +STD_CELL_LIBRARY_OPT_CDL +TECH_LEF_OPT +LIB_SYNTH_OPT +""" +untested = """ +LVS_EXTRA_GATE_LEVEL_VERILOG +LVS_EXTRA_STD_CELL_LIBRARY +KLAYOUT_DRC_TECH_SCRIPT +""" + +white_list = set( + ( + internal_variables + + gpio_variables + + to_be_removed + + documented_elsewhere + + unexposed + + opts + + untested + ).split() +) +docs_variables = ( + subprocess.check_output( + [ + "rg", + "\\| *`([A-Z]\\S+) *` *‡* *\\|", + "-r", + "$1", + "-o", + "-N", + "-I", + "--no-ignore", + "docs", + ] + ) + .decode("utf-8") + .split() +) +docs_variables = [var for var in docs_variables if var.isupper()] +docs_variables_set = set(docs_variables) + +deprecated_docs_variables = ( + subprocess.check_output( + [ + "rg", + "\\| *`([A-Z]\\S+) *` *‡* *\\|.*(Deprecated|Removed)", + "-r", + "$1", + "-o", + "-N", + "-I", + "--no-ignore", + "docs", + ] + ) + .decode("utf-8") + .split() +) +depreacted_docs_variables = [var for var in deprecated_docs_variables if var.isupper()] +deprecated_docs_variables_set = set(deprecated_docs_variables) + + +used_variables = ( + subprocess.check_output( + [ + "rg", + "\\$::env\\(([A-Z]\\S+?)\\)", + "-r", + "$1", + "-o", + "-N", + "-I", + "--no-ignore", + "scripts", + "flow.tcl", + ] + ) + .decode("utf-8") + .split() +) +used_variables_set = set(used_variables) + +undocumented = sorted( + used_variables_set - docs_variables_set - deprecated_docs_variables_set - white_list +) +if undocumented: + print("[ERROR]: found the following undocumented variables.") + for var in undocumented: + print(var) + exit(1) +else: + print("Pass") diff --git a/.github/workflows/variables_documentation_check.yml b/.github/workflows/variables_documentation_check.yml new file mode 100644 index 00000000..06ddefd3 --- /dev/null +++ b/.github/workflows/variables_documentation_check.yml @@ -0,0 +1,20 @@ +name: Documentation +on: + # Runs on all pushes to branches + push: + # Runs on all PRs + pull_request: + # Manual Dispatch + workflow_dispatch: + +jobs: + check_variables: + name: Check Variables + runs-on: ubuntu-20.04 + steps: + - name: Check out Git repository + uses: actions/checkout@v2 + - name: Install Ripgrep + run: sudo apt install -y ripgrep + - name: Check for missing documentation + run: cd ${GITHUB_WORKSPACE}/ && python3 ${GITHUB_WORKSPACE}/.github/scripts/variables_documentation.py diff --git a/configuration/general.tcl b/configuration/general.tcl index cc68d651..525df557 100755 --- a/configuration/general.tcl +++ b/configuration/general.tcl @@ -26,12 +26,11 @@ set ::env(RUN_FILL_INSERTION) 1 set ::env(RUN_TAP_DECAP_INSERTION) 1 set ::env(RUN_LINTER) 1 -## Intentionally Undocumented -set ::env(RSZ_USE_OLD_REMOVER) 0 - ## STA set ::env(STA_REPORT_POWER) {1} set ::env(STA_WRITE_LIB) {1} + +### Private: Not granular enough, not going to be compatible with OL2 set ::env(STA_MULTICORNER_READ_LIBS) 0 ## Routing diff --git a/configuration/routing.tcl b/configuration/routing.tcl index 8598ce5d..3b5b6cfe 100755 --- a/configuration/routing.tcl +++ b/configuration/routing.tcl @@ -20,7 +20,7 @@ if { ![info exists ::env(ROUTING_CORES)] } { set ::env(RUN_HEURISTIC_DIODE_INSERTION) 0 set ::env(HEURISTIC_ANTENNA_THRESHOLD) 90 -# Privbate: Strategy for placement of the diodes. Possible values `source`, `pin`, `balanced` and `random`. Only applicable when `RUN_HEURISTIC_DIODE_INSERTION` is enabled. +# Private: Strategy for placement of the diodes. Possible values `source`, `pin`, `balanced` and `random`. Only applicable when `RUN_HEURISTIC_DIODE_INSERTION` is enabled. set ::env(HEURISTIC_ANTENNA_INSERTION_MODE) "source" set ::env(DIODE_PADDING) 2 diff --git a/docs/source/reference/configuration.md b/docs/source/reference/configuration.md index 357601e8..33c0edb5 100644 --- a/docs/source/reference/configuration.md +++ b/docs/source/reference/configuration.md @@ -386,6 +386,7 @@ These variables worked initially, but they were too sky130 specific and will be | `QUIT_ON_TIMING_VIOLATIONS ` | Controls `QUIT_ON_HOLD_VIOLATIONS` and `QUIT_ON_SETUP_VIOLATIONS`
(Default: `1`)| | `QUIT_ON_LINTER_WARNINGS` | Quit on warnings generated by linter (currently Verilator)
(Default: `0`)| | `QUIT_ON_LINTER_ERRORS` | Quit on errors generated by linter (currently Verilator)
(Default: `1`)| +| `QUIT_ON_XOR_ERROR` | Quit on XOR differences between GDSII generated by Magic and KLayout
(Default: `1`)| ### On comma-delimited variables :::{warning} diff --git a/flow.tcl b/flow.tcl index a118519a..05739b42 100755 --- a/flow.tcl +++ b/flow.tcl @@ -43,9 +43,6 @@ proc run_cts_step {args} { run_cts run_resizer_timing - if { $::env(RSZ_USE_OLD_REMOVER) == 1} { - remove_buffers_from_nets - } } proc run_routing_step {args} { diff --git a/scripts/odbpy/remove_buffers.py b/scripts/odbpy/remove_buffers.py deleted file mode 100644 index 5e1705df..00000000 --- a/scripts/odbpy/remove_buffers.py +++ /dev/null @@ -1,176 +0,0 @@ -# Copyright 2021-2022 Efabless Corporation -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import odb - -import os -import re -from typing import List - -import click - -from reader import click_odb - - -def get_pin_name(pin: odb.dbITerm): - cell = pin.getInst() - cell_name = cell.getName() - master_pin = pin.getMTerm() - master_pin_name = master_pin.getConstName() - return f"{cell_name}/{master_pin_name}" - - -def get_sinks_terms(net: odb.dbNet) -> List[odb.dbITerm]: - sinks = [] - for it in net.getITerms(): - cell = it.getInst() - cell_pin = it.getMTerm() - - master_instance = cell.getMaster() - master_name = master_instance.getConstName() - - if cell_pin.getIoType() == "INPUT": - print( - f" * Net {net.getConstName()} sinks into {get_pin_name(it)} ({master_name})..." - ) - sinks.append(it) - - return sinks - - -def get_drivers(net: odb.dbNet) -> List[odb.dbInst]: - drivers = [] - for it in net.getITerms(): - cell = it.getInst() - cell_pin = it.getMTerm() - - master_instance = cell.getMaster() - master_name = master_instance.getConstName() - - if cell_pin.getIoType() == "OUTPUT": - print( - f" * Net {net.getConstName()} is driven by {get_pin_name(it)} ({master_name})..." - ) - drivers.append(cell) - - return drivers - - -def get_io(cell: odb.dbInst): - inputs = [] - outputs = [] - for iterm in cell.getITerms(): - if iterm.isSpecial(): # Skip special nets - continue - - if iterm.isInputSignal(): - input_pin_net = iterm.getNet() - inputs.append((iterm, input_pin_net)) - - if iterm.isOutputSignal(): - output_pin_net = iterm.getNet() - outputs.append((iterm, output_pin_net)) - - return inputs, outputs - - -@click.command() -@click.option( - "-m", - "--match", - "rx_str", - required=True, - help="A regular expression matching all nets to remove.", -) -@click_odb -def remove_buffers(reader, rx_str): - if rx_str != "^$": - # Save some compute time :) - - rx = re.compile(rx_str) - - design_nets = reader.block.getNets() - dont_buffer_nets = [ - net for net in design_nets if rx.match(net.getConstName()) is not None - ] - - for net in dont_buffer_nets: - net_name = net.getConstName() - print(f"* Attempting to unbuffer {net_name}...") - # get the cells driving the dont buffer net - drivers = get_drivers(net) - - if len(drivers) > 1: - print(f"Net {net_name} is driven by multiple cells.") - exit(os.EX_DATAERR) - elif len(drivers) == 0: - print(f"Net {net_name} is not driven by any cell..") - exit(os.EX_DATAERR) - - buffer = drivers[0] - buffer_name = drivers[0].getName() - master = buffer.getMaster() - master_name = master.getConstName() - - if "buf" not in master_name: - print( - f"* {net_name} isn't driven by a buffer cell. It is driven by {buffer_name} ({master_name}). Skipping..." - ) - continue - - # get the net connected to the input pin of this buffer - inputs, outputs = get_io(buffer) - - if len(inputs) != 1: - print( - f"* {master_name} has more than one output port. Doesn't appear to actually be a buffer. Skipping..." - ) - continue - if len(outputs) != 1: - print( - f"{master_name} has more than one output port. Doesn't appear to actually be a buffer. Skipping..." - ) - continue - - _, input_net = inputs[0] - _, output_net = outputs[0] - - print(" * Reconnecting IO...") - # We connect the driver's output to the output_net: there may not be a - # sink with ITerms in case of things like output ports for example. - buffer_input_driver = get_drivers(input_net)[0] - _, bid_outputs = get_io(buffer_input_driver) - (bid_iterm, _) = bid_outputs[0] - bid_iterm.connect(output_net) - print( - f" * Connected buffer output({output_net.getConstName()}) to {get_pin_name(bid_iterm)}." - ) - - print(f" * Removing buffer {buffer_name} ({master_name})...") - odb.dbInst.destroy(buffer) - - input_net_sinks = get_sinks_terms(input_net) - for iterm in input_net_sinks: - iterm.connect(output_net) - print( - f" * Connected buffer output({output_net.getConstName()}) to {get_pin_name(iterm)}." - ) - - print(f" * Removing net {input_net.getName()}...") - odb.dbNet.destroy(input_net) - - print(" * Done.") - - -if __name__ == "__main__": - remove_buffers() diff --git a/scripts/openroad/common/resizer.tcl b/scripts/openroad/common/resizer.tcl index 1c059e89..e4e9b068 100644 --- a/scripts/openroad/common/resizer.tcl +++ b/scripts/openroad/common/resizer.tcl @@ -12,8 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. proc set_dont_touch_wrapper {} { - if { [info exists ::env(RSZ_DONT_TOUCH_RX)] && \ - ($::env(RSZ_USE_OLD_REMOVER) != 1 || $::env(RSZ_DONT_TOUCH_RX) != {^$}) } { + if { [info exists ::env(RSZ_DONT_TOUCH_RX)] && $::env(RSZ_DONT_TOUCH_RX) != {^$} } { set pattern $::env(RSZ_DONT_TOUCH_RX) variable odb_block [[[::ord::get_db] getChip] getBlock] @@ -41,9 +40,7 @@ proc set_dont_touch_wrapper {} { } proc unset_dont_touch_wrapper {} { - if { [info exists ::env(RSZ_DONT_TOUCH_RX)] && \ - ($::env(RSZ_USE_OLD_REMOVER) != 1 || $::env(RSZ_DONT_TOUCH_RX) != {^$}) } { - + if { [info exists ::env(RSZ_DONT_TOUCH_RX)] && $::env(RSZ_DONT_TOUCH_RX) != {^$} } { set pattern $::env(RSZ_DONT_TOUCH_RX) variable odb_block [[[::ord::get_db] getChip] getBlock] set odb_nets [odb::dbBlock_getNets $::odb_block] diff --git a/scripts/tcl_commands/placement.tcl b/scripts/tcl_commands/placement.tcl index f69460e1..1d25a552 100755 --- a/scripts/tcl_commands/placement.tcl +++ b/scripts/tcl_commands/placement.tcl @@ -175,9 +175,6 @@ proc run_placement {args} { } run_resizer_design - if { $::env(RSZ_USE_OLD_REMOVER) == 1} { - remove_buffers_from_nets - } detailed_placement_or @@ -203,32 +200,4 @@ proc run_resizer_design {args} { } } -proc remove_buffers_from_nets {args} { - # This is a workaround for some situations where the resizer would buffer - # analog ports. - # - # Though to be clear- it works on all nets. - increment_index - TIMER::timer_start - set log [index_file $::env(placement_logs)/remove_buffers.log] - puts_info "Removing Buffers from Nets (If Applicable) (log: [relpath . $log])..." - - set fbasename [file rootname $::env(CURRENT_DEF)] - set save_def ${fbasename}.buffers_removed.def - set save_odb ${fbasename}.buffers_removed.odb - - manipulate_layout $::env(SCRIPTS_DIR)/odbpy/remove_buffers.py\ - -indexed_log $log\ - -output $save_odb\ - -output_def $save_def\ - -input $::env(CURRENT_ODB)\ - --match $::env(RSZ_DONT_TOUCH_RX) - - set_def $save_def - set_odb $save_odb - - TIMER::timer_stop - exec echo "[TIMER::get_runtime]" | python3 $::env(SCRIPTS_DIR)/write_runtime.py "remove buffers from nets - openlane" -} - package provide openlane 0.9 diff --git a/scripts/tcl_commands/routing.tcl b/scripts/tcl_commands/routing.tcl index 989b6ca8..aa117edb 100755 --- a/scripts/tcl_commands/routing.tcl +++ b/scripts/tcl_commands/routing.tcl @@ -401,9 +401,6 @@ proc run_routing {args} { run_resizer_design_routing run_resizer_timing_routing - if { $::env(RSZ_USE_OLD_REMOVER) == 1} { - remove_buffers_from_nets - } if { [info exists ::env(DIODE_CELL)] && ($::env(DIODE_CELL) ne "") } { if { $::env(DIODE_ON_PORTS) ne "none" } {