From dcaa670b62f819c3cc3eaf71838fc7a238333857 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sun, 13 Oct 2024 10:41:47 +0000 Subject: [PATCH 1/9] add yamllint scaffold, and prettify variables.yaml Signed-off-by: Jack Luar --- .../workflows/github-actions-yaml-test.yml | 67 +++++++ .yamllint | 37 ++++ etc/DependencyInstaller.sh | 2 +- flow/scripts/variables.yaml | 181 +++++++++--------- 4 files changed, 195 insertions(+), 92 deletions(-) create mode 100644 .github/workflows/github-actions-yaml-test.yml create mode 100644 .yamllint diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml new file mode 100644 index 0000000000..3a55cea0e1 --- /dev/null +++ b/.github/workflows/github-actions-yaml-test.yml @@ -0,0 +1,67 @@ +name: ORFS variables.yaml tester and linter + +on: + push: + +jobs: + docs-test-job: + name: 'Tests for variables.yaml' + runs-on: ubuntu-latest + container: + image: openroad/ubuntu-cpp20 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: 'recursive' + - name: Run YAML script + run: | + python3 flow/scripts/generate-variable-docs.py + - name: Run YAML Lint + run: | + pip install yamllint==1.35.1 + yamllint flow/scripts/variables.yaml + + docs-pr-update: + name: 'Create PR to update ORFS FlowVariables.md' + needs: + - docs-test-job + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Run generate-variable-docs.py + run: | + python3 flow/scripts/generate-variable-docs.py + - name: Create branch if diff exists + id: docs-update + run: | + git config --local user.email "github-actions[bot]@users.noreply.github.com" + git config --local user.name "github-actions[bot]" + if [ -n "$(git status --porcelain)" ]; then + echo "has_update=true" >> "$GITHUB_OUTPUT" + else + echo "has_update=false" >> "$GITHUB_OUTPUT" + fi + git add . + git commit --signoff -m "[Docs]: Update ORFS FlowVariables.md" + - name: Only push if not master + if: "github.event.client_payload.branch != 'master'" + id: remote-update-pr + run: | + git push origin "HEAD:refs/pull/${{ github.event.client_payload.branch }}/head" + - name: Create docs update PR + if: "steps.remote-update.outputs.has_update == 'true' && github.event.client_payload.branch == 'master'" + uses: peter-evans/create-pull-request@v5 + with: + token: ${{ github.token }} + signoff: true + delete-branch: true + title: "[BOT] Update ORFS variables" + reviewers: | + vvbandeira + maliberty + draft: true + branch: bot-update-variables + commit-message: | + [BOT] Update ORFS variables diff --git a/.yamllint b/.yamllint new file mode 100644 index 0000000000..524db4c8e8 --- /dev/null +++ b/.yamllint @@ -0,0 +1,37 @@ +extends: default + +rules: + # General formatting + indentation: + level: error # options: 'error' (strict), 'warning' (less strict) + spaces: 2 # number of spaces per indentation level + line-length: + max: 120 # maximum allowed length for lines + level: warning + allow-non-breakable-words: false # non-breakable words exceeding line-length + + # Document structure + document-start: + level: error # documents should start with '---' + + # Trailing spaces + trailing-spaces: + level: error # no trailing spaces allowed + + # Comments + comments: + level: warning + require-starting-space: true # comments should have a space after '#' + + # Special characters + key-duplicates: + level: error # duplicate keys are not allowed + truthy: + level: warning # warn on 'yes/no' and 'on/off'; prefer 'true/false' + + # File properties + empty-lines: + max: 2 # no more than 2 consecutive empty lines + level: warning + new-line-at-end-of-file: + level: warning # files should end with a newline diff --git a/etc/DependencyInstaller.sh b/etc/DependencyInstaller.sh index 53a50aba0d..a92b45d5a9 100755 --- a/etc/DependencyInstaller.sh +++ b/etc/DependencyInstaller.sh @@ -29,7 +29,7 @@ _installCommon() { source /opt/rh/rh-python38/enable set -u fi - local pkgs="pandas numpy firebase_admin click pyyaml" + local pkgs="pandas numpy firebase_admin click pyyaml yamllint" if [[ $(id -u) == 0 ]]; then pip3 install --no-cache-dir -U $pkgs else diff --git a/flow/scripts/variables.yaml b/flow/scripts/variables.yaml index 39a1893d66..c9c18eff9c 100644 --- a/flow/scripts/variables.yaml +++ b/flow/scripts/variables.yaml @@ -2,13 +2,13 @@ GENERATE_ARTIFACTS_ON_FAILURE: description: > For instance Bazel needs artifacts (.odb and .rpt files) on a failure to - allow the user to save hours on re-running the failed step locally, but - when working with a Makefile flow, it is more natural to fail the step - and leave the user to manually inspect the logs and artifacts directly via - the file system. + allow the user to save hours on re-running the failed step locally, but when + working with a Makefile flow, it is more natural to fail the step and leave + the user to manually inspect the logs and artifacts directly via the file + system. - Set to 1 to change the behavior to generate artifacts upon failure to - e.g. do a global route. The exit code will still be non-zero on all other + Set to 1 to change the behavior to generate artifacts upon failure to e.g. + do a global route. The exit code will still be non-zero on all other failures that aren't covered by the "useful to inspect the artifacts on failure" use-case. @@ -19,22 +19,21 @@ GENERATE_ARTIFACTS_ON_FAILURE: Detailed route will not proceed, if there is global routing congestion - This allows build systems, such as bazel, to create artifacts for global - and detailed route, even if the operation had problems, without having - know about the semantics between global and detailed route. + This allows build systems, such as bazel, to create artifacts for global and + detailed route, even if the operation had problems, without having know + about the semantics between global and detailed route. - Considering that global and detailed route can run for a long time and - use a lot of memory, this allows inspecting results on a laptop for - a build that ran on a server. + Considering that global and detailed route can run for a long time and use a + lot of memory, this allows inspecting results on a laptop for a build that + ran on a server. default: 0 - TNS_END_PERCENT: description: > - Default TNS_END_PERCENT value for post CTS timing repair. - Try fixing all violating endpoints by default (reduce to 5% for runtime). + Default TNS_END_PERCENT value for post CTS timing repair. Try fixing all + violating endpoints by default (reduce to 5% for runtime). - Specifies how many percent of violating paths to fix [0-100]. - Worst path will always be fixed. + Specifies how many percent of violating paths to fix [0-100]. Worst path + will always be fixed. default: 100 stages: - cts @@ -100,7 +99,7 @@ EQUIVALENCE_CHECK: stages: - cts CORE_UTILIZATION: - description: > + description: | The core utilization percentage (0-100). stages: - floorplan @@ -135,7 +134,7 @@ SKIP_REPORT_METRICS: - route - final PROCESS: - description: > + description: | Technology node or process in use. CORNER: description: > @@ -145,10 +144,10 @@ TECH_LEF: A technology LEF file of the PDK that includes all relevant information regarding metal layers, vias, and spacing requirements. SC_LEF: - description: > + description: | Path to technology standard cell LEF file. GDS_FILES: - description: > + description: | Path to platform GDS files. LIB_FILES: description: > @@ -156,34 +155,34 @@ LIB_FILES: input and output characteristics, timing and power definitions for each cell. DONT_USE_CELLS: - description: > + description: | Dont use cells eases pin access in detailed routing. SYNTH_GUT: description: > - Load design and remove all internal logic before doing synthesis. This - is useful when creating a mock .lef abstract that has a smaller area - than the amount of logic would allow. bazel-orfs uses this to mock - SRAMs, for instance. + Load design and remove all internal logic before doing synthesis. This is + useful when creating a mock .lef abstract that has a smaller area than the + amount of logic would allow. bazel-orfs uses this to mock SRAMs, for + instance. stages: - synth SYNTH_HIERARCHICAL: - description: > + description: | Enable to Synthesis hierarchically, otherwise considered flat synthesis. stages: - synth default: 0 LATCH_MAP_FILE: - description: > + description: | List of latches treated as a black box by Yosys. stages: - synth CLKGATE_MAP_FILE: - description: > + description: | List of cells for gating clock treated as a black box by Yosys. stages: - synth ADDER_MAP_FILE: - description: > + description: | List of adders treated as a black box by Yosys. stages: - synth @@ -195,13 +194,13 @@ TIEHI_CELL_AND_PORT: - synth - place TIELO_CELL_AND_PORT: - description: > + description: | Tie low cells used in Yosys synthesis to replace a logical 0 in the Netlist. stages: - synth - place MIN_BUF_CELL_AND_PORTS: - description: > + description: | Used to insert a buffer cell to pass through wires. Used in synthesis. stages: - synth @@ -212,12 +211,12 @@ ABC_CLOCK_PERIOD_IN_PS: stages: - synth ABC_DRIVER_CELL: - description: > + description: | Default driver cell used during ABC synthesis. stages: - synth ABC_LOAD_IN_FF: - description: > + description: | During synthesis set_load value used. stages: - synth @@ -228,7 +227,7 @@ MAX_UNGROUP_SIZE: stages: - synth FLOORPLAN_DEF: - description: > + description: | Use the DEF file to initialize floorplan. stages: - floorplan @@ -242,12 +241,12 @@ REMOVE_ABC_BUFFERS: - floorplan deprecated: 1 PLACE_SITE: - description: > + description: | Placement site for core cells defined in the technology LEF file. stages: - floorplan TAPCELL_TCL: - description: > + description: | Path to Endcap and Welltie cells file. stages: - floorplan @@ -258,7 +257,7 @@ MACRO_PLACEMENT: stages: - floorplan MACRO_PLACEMENT_TCL: - description: > + description: | Specifies the path of a TCL file on how to place certain macros manually. stages: - floorplan @@ -276,7 +275,7 @@ MACRO_PLACE_CHANNEL: stages: - floorplan MACRO_BLOCKAGE_HALO: - description: > + description: | Blockage width overridden from default calculation. stages: - floorplan @@ -288,12 +287,12 @@ PDN_TCL: stages: - floorplan MAKE_TRACKS: - description: > + description: | Tcl file that defines add routing tracks to a floorplan. stages: - floorplan IO_CONSTRAINTS: - description: > + description: | File path to the IO constraints .tcl file. stages: - floorplan @@ -314,8 +313,8 @@ IO_PLACER_V: - place GUI_TIMING: description: > - Load timing information when opening GUI. For large designs, this can - be quite time consuming. Useful to disable when investigating non-timing + Load timing information when opening GUI. For large designs, this can be + quite time consuming. Useful to disable when investigating non-timing aspects like floorplan, placement, routing, etc. default: 1 FILL_CELLS: @@ -325,7 +324,7 @@ FILL_CELLS: stages: - route TAP_CELL_NAME: - description: > + description: | Name of the cell to use in tap cell insertion. CELL_PAD_IN_SITES_GLOBAL_PLACEMENT: description: > @@ -345,7 +344,7 @@ CELL_PAD_IN_SITES_DETAIL_PLACEMENT: - grt default: 0 PLACE_PINS_ARGS: - description: > + description: | Arguments to place_pins stages: - place @@ -363,7 +362,7 @@ PLACE_DENSITY_LB_ADDON: Check the lower boundary of the PLACE_DENSITY and add PLACE_DENSITY_LB_ADDON if it exists. REPAIR_PDN_VIA_LAYER: - description: > + description: | Remove power grid vias which generate DRC violations after detailed routing. GLOBAL_PLACEMENT_ARGS: description: > @@ -371,21 +370,21 @@ GLOBAL_PLACEMENT_ARGS: args defined in global_place.tcl. default: "" ENABLE_DPO: - description: > + description: | Enable detail placement with improve_placement feature. default: 1 DPO_MAX_DISPLACEMENT: - description: > + description: | Specifies how far an instance can be moved when optimizing. default: 5 1 GPL_TIMING_DRIVEN: - description: > + description: | Specifies whether the placer should use timing driven placement. stages: - place default: 1 GPL_ROUTABILITY_DRIVEN: - description: > + description: | Specifies whether the placer should use routability driven placement. stages: - place @@ -399,7 +398,7 @@ SLEW_MARGIN: Specifies a slew margin when fixing max slew violations. This option allows you to overfix. CTS_ARGS: - description: > + description: | Override `clock_tree_synthesis` arguments. stages: - cts @@ -454,7 +453,7 @@ HOLD_SLACK_MARGIN: - grt default: 0 SETUP_SLACK_MARGIN: - description: > + description: | Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). @@ -502,7 +501,7 @@ SKIP_CTS_REPAIR_TIMING: stages: - cts MIN_ROUTING_LAYER: - description: > + description: | The lowest metal layer name to be used in routing. stages: - place @@ -510,7 +509,7 @@ MIN_ROUTING_LAYER: - route - final MAX_ROUTING_LAYER: - description: > + description: | The highest metal layer name to be used in routing. stages: - place @@ -518,12 +517,12 @@ MAX_ROUTING_LAYER: - route - final DETAILED_ROUTE_ARGS: - description: > + description: | Add additional arguments for debugging purposes during detail route. stages: - route MACRO_EXTENSION: - description: > + description: | Sets the number of GCells added to the blockages boundaries from macros. DETAILED_ROUTE_END_ITERATION: description: > @@ -532,27 +531,27 @@ DETAILED_ROUTE_END_ITERATION: stages: - route RCX_RULES: - description: > + description: | RC Extraction rules file path. SET_RC_TCL: - description: > + description: | Metal & Via RC definition file path. FILL_CONFIG: - description: > + description: | JSON rule file for metal fill during chip finishing. KLAYOUT_TECH_FILE: - description: > + description: | A mapping from LEF/DEF to GDS using the KLayout tool. IR_DROP_LAYER: - description: > + description: | Default metal layer to report IR drop. PLATFORM: required: true - description: > + description: | Specifies process design kit or technology node to be used. DESIGN_NAME: required: true - description: > + description: | The name of the top-level module of the design. VERILOG_FILES: required: true @@ -563,20 +562,20 @@ VERILOG_FILES: - synth SDC_FILE: required: true - description: > + description: | The path to design constraint (SDC) file. stages: - synth SDC_GUT: description: > - Load design and remove all internal logic before doing synthesis. This - is useful when creating a mock .lef abstract that has a smaller area - than the amount of logic would allow. bazel-orfs uses this to mock - SRAMs, for instance. + Load design and remove all internal logic before doing synthesis. This is + useful when creating a mock .lef abstract that has a smaller area than the + amount of logic would allow. bazel-orfs uses this to mock SRAMs, for + instance. stages: - synth ADDITIONAL_FILES: - description: > + description: | Additional files to be added to `make issue` archive. ADDITIONAL_LEFS: description: > @@ -587,12 +586,12 @@ ADDITIONAL_LIBS: Hardened macro library files listed here. The library information is immutable and used throughout all stages. Not stored in the .odb file. ADDITIONAL_GDS: - description: > + description: | Hardened macro GDS files listed here. stages: - final VERILOG_INCLUDE_DIRS: - description: > + description: | Specifies the include directories for the Verilog input files. stages: - synth @@ -602,18 +601,18 @@ DESIGN_NICKNAME: DESIGN_NICKNAME instead of DESIGN_NAME in case DESIGN_NAME is unwieldy or conflicts with a different design. ABC_AREA: - description: > + description: | Strategies for Yosys ABC synthesis: Area/Speed. Default ABC_SPEED. stages: - synth default: 0 PWR_NETS_VOLTAGES: - description: > + description: | Used for IR Drop calculation. stages: - final GND_NETS_VOLTAGES: - description: > + description: | Used for IR Drop calculation. stages: - final @@ -622,23 +621,23 @@ BLOCKS: Blocks used as hard macros in a hierarchical flow. Do note that you have to specify block-specific inputs file in the directory mentioned by Makefile. CDL_FILES: - description: > + description: | Insert additional Circuit Description Language (`.cdl`) netlist files. DFF_LIB_FILES: - description: > + description: | Technology mapping liberty files for flip-flops. DONT_USE_LIBS: - description: > + description: | Set liberty files as `dont_use`. PRESERVE_CELLS: - description: > + description: | Mark modules to keep from getting removed in flattening. SYNTH_ARGS: - description: > + description: | Optional synthesis variables for yosys. default: -flatten VERILOG_TOP_PARAMS: - description: > + description: | Apply toplevel params (if exist). stages: - synth @@ -665,34 +664,34 @@ DIE_AREA: stages: - floorplan RESYNTH_AREA_RECOVER: - description: > + description: | Enable re-synthesis for area reclaim. stages: - synth default: 0 RESYNTH_TIMING_RECOVER: - description: > + description: | Enable re-synthesis for timing optimization. stages: - synth default: 0 MACRO_HALO_X: - description: > + description: | Set macro halo for x-direction. Only available for ASAP7 PDK. stages: - floorplan MACRO_HALO_Y: - description: > + description: | Set macro halo for y-direction. Only available for ASAP7 PDK. stages: - floorplan MACRO_WRAPPERS: - description: > + description: | The wrapper file that replaces existing macros with their wrapped version. stages: - floorplan CTS_BUF_DISTANCE: - description: > + description: | Distance (in microns) between buffers. stages: - cts @@ -709,27 +708,27 @@ CTS_CLUSTER_SIZE: stages: - cts CTS_SNAPSHOT: - description: > + description: | Creates ODB/SDC files prior to clock net and setup/hold repair. stages: - cts POST_CTS_TCL: - description: > + description: | Specifies a Tcl script with commands to run after CTS is completed. stages: - cts FASTROUTE_TCL: - description: > + description: | Specifies a Tcl script with commands to run before FastRoute. USE_FILL: description: > Whether to perform metal density filling. default: 0 SEAL_GDS: - description: > + description: | Seal macro to place around the design. ABSTRACT_SOURCE: - description: > + description: | Which .odb file to use to create abstract stages: - generate_abstract From a18b283858360ca3293b554c61c2a8c35e8b7b49 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sun, 13 Oct 2024 13:38:28 +0000 Subject: [PATCH 2/9] replace yamllint with yamlfix, add sanity check before commit in gh action Signed-off-by: Jack Luar --- .../workflows/github-actions-yaml-test.yml | 30 ++++++++++----- .yamllint | 37 ------------------- etc/DependencyInstaller.sh | 2 +- flow/scripts/variables.yaml | 5 --- yamlfix.toml | 11 ++++++ 5 files changed, 32 insertions(+), 53 deletions(-) delete mode 100644 .yamllint create mode 100644 yamlfix.toml diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index 3a55cea0e1..61a83e1d14 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -14,16 +14,15 @@ jobs: uses: actions/checkout@v4 with: submodules: 'recursive' - - name: Run YAML script + - name: Run generate-variable-docs.py run: | python3 flow/scripts/generate-variable-docs.py - - name: Run YAML Lint + - name: Run yamlfix check run: | - pip install yamllint==1.35.1 - yamllint flow/scripts/variables.yaml + yamlfix -c yamlfix.toml flow/scripts/variables.yaml --check docs-pr-update: - name: 'Create PR to update ORFS FlowVariables.md' + name: 'Create PR to update ORFS FlowVariables.md and variables.yaml' needs: - docs-test-job runs-on: ubuntu-latest @@ -33,8 +32,12 @@ jobs: - name: Run generate-variable-docs.py run: | python3 flow/scripts/generate-variable-docs.py + - name: Run yamlfix + run: | + pip install yamlfix==1.17.0 + yamlfix -c yamlfix.toml flow/scripts/variables.yaml - name: Create branch if diff exists - id: docs-update + id: variables-update run: | git config --local user.email "github-actions[bot]@users.noreply.github.com" git config --local user.name "github-actions[bot]" @@ -43,15 +46,22 @@ jobs: else echo "has_update=false" >> "$GITHUB_OUTPUT" fi - git add . + git add flow/scripts/variables.yaml + git add docs/user/FlowVariables.md git commit --signoff -m "[Docs]: Update ORFS FlowVariables.md" + + # Sanity check that no other files are unstaged + if [ -n "$(git status --porcelain)" ]; then + echo "Error: Unstaged changes after commit." + exit 1 + fi - name: Only push if not master if: "github.event.client_payload.branch != 'master'" - id: remote-update-pr + id: variables-update-pr run: | git push origin "HEAD:refs/pull/${{ github.event.client_payload.branch }}/head" - - name: Create docs update PR - if: "steps.remote-update.outputs.has_update == 'true' && github.event.client_payload.branch == 'master'" + - name: Create variables update PR + if: "steps.variables-update.outputs.has_update == 'true' && github.event.client_payload.branch == 'master'" uses: peter-evans/create-pull-request@v5 with: token: ${{ github.token }} diff --git a/.yamllint b/.yamllint deleted file mode 100644 index 524db4c8e8..0000000000 --- a/.yamllint +++ /dev/null @@ -1,37 +0,0 @@ -extends: default - -rules: - # General formatting - indentation: - level: error # options: 'error' (strict), 'warning' (less strict) - spaces: 2 # number of spaces per indentation level - line-length: - max: 120 # maximum allowed length for lines - level: warning - allow-non-breakable-words: false # non-breakable words exceeding line-length - - # Document structure - document-start: - level: error # documents should start with '---' - - # Trailing spaces - trailing-spaces: - level: error # no trailing spaces allowed - - # Comments - comments: - level: warning - require-starting-space: true # comments should have a space after '#' - - # Special characters - key-duplicates: - level: error # duplicate keys are not allowed - truthy: - level: warning # warn on 'yes/no' and 'on/off'; prefer 'true/false' - - # File properties - empty-lines: - max: 2 # no more than 2 consecutive empty lines - level: warning - new-line-at-end-of-file: - level: warning # files should end with a newline diff --git a/etc/DependencyInstaller.sh b/etc/DependencyInstaller.sh index a92b45d5a9..82b91f916d 100755 --- a/etc/DependencyInstaller.sh +++ b/etc/DependencyInstaller.sh @@ -29,7 +29,7 @@ _installCommon() { source /opt/rh/rh-python38/enable set -u fi - local pkgs="pandas numpy firebase_admin click pyyaml yamllint" + local pkgs="pandas numpy firebase_admin click pyyaml yamlfix" if [[ $(id -u) == 0 ]]; then pip3 install --no-cache-dir -U $pkgs else diff --git a/flow/scripts/variables.yaml b/flow/scripts/variables.yaml index c9c18eff9c..2acd52398c 100644 --- a/flow/scripts/variables.yaml +++ b/flow/scripts/variables.yaml @@ -6,23 +6,19 @@ GENERATE_ARTIFACTS_ON_FAILURE: working with a Makefile flow, it is more natural to fail the step and leave the user to manually inspect the logs and artifacts directly via the file system. - Set to 1 to change the behavior to generate artifacts upon failure to e.g. do a global route. The exit code will still be non-zero on all other failures that aren't covered by the "useful to inspect the artifacts on failure" use-case. - Example: just like detailed routing, a global route that fails with congestion, is not a build failure(as in exit code non-zero), it is a successful(as in zero exit code) global route that produce reports detailing the problem. Detailed route will not proceed, if there is global routing congestion - This allows build systems, such as bazel, to create artifacts for global and detailed route, even if the operation had problems, without having know about the semantics between global and detailed route. - Considering that global and detailed route can run for a long time and use a lot of memory, this allows inspecting results on a laptop for a build that ran on a server. @@ -31,7 +27,6 @@ TNS_END_PERCENT: description: > Default TNS_END_PERCENT value for post CTS timing repair. Try fixing all violating endpoints by default (reduce to 5% for runtime). - Specifies how many percent of violating paths to fix [0-100]. Worst path will always be fixed. default: 100 diff --git a/yamlfix.toml b/yamlfix.toml new file mode 100644 index 0000000000..7226384a44 --- /dev/null +++ b/yamlfix.toml @@ -0,0 +1,11 @@ +explicit_start = true +line_length = 120 +preserve_quotes = true +quote_representation = "'" +section_whitelines = 0 +sequence_style = "block_style" +comments_min_spaces_from_content = 2 +allow_duplicate_keys = false +indent_mapping = 2 +indent_offset = 2 +indent_sequence = 4 From 1e9929e09e0827455ec28f83d712be87634ce5a8 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sat, 9 Nov 2024 14:24:01 +0000 Subject: [PATCH 3/9] only run pr update on push, lint on pr/push Signed-off-by: Jack Luar --- .github/workflows/github-actions-yaml-test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index 61a83e1d14..2fb8d71a3f 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -2,10 +2,12 @@ name: ORFS variables.yaml tester and linter on: push: + pull_request: jobs: docs-test-job: name: 'Tests for variables.yaml' + if: "github.event_name == 'pull_request' || github.event_name == 'push'" runs-on: ubuntu-latest container: image: openroad/ubuntu-cpp20 @@ -23,6 +25,7 @@ jobs: docs-pr-update: name: 'Create PR to update ORFS FlowVariables.md and variables.yaml' + if: "github.event_name == 'push'" needs: - docs-test-job runs-on: ubuntu-latest From d19aafd7049f993b03df35629db107ef698a3531 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sat, 9 Nov 2024 15:00:38 +0000 Subject: [PATCH 4/9] fix ci Signed-off-by: Jack Luar --- .github/workflows/github-actions-yaml-test.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index 2fb8d71a3f..832a6fdb92 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -16,25 +16,25 @@ jobs: uses: actions/checkout@v4 with: submodules: 'recursive' - - name: Run generate-variable-docs.py + - name: Run generate-variables-docs.py run: | - python3 flow/scripts/generate-variable-docs.py + python3 flow/scripts/generate-variables-docs.py - name: Run yamlfix check run: | yamlfix -c yamlfix.toml flow/scripts/variables.yaml --check docs-pr-update: name: 'Create PR to update ORFS FlowVariables.md and variables.yaml' - if: "github.event_name == 'push'" + if: "github.event_name == 'pull_request' || github.event_name == 'push'" needs: - docs-test-job runs-on: ubuntu-latest steps: - name: Checkout repository uses: actions/checkout@v4 - - name: Run generate-variable-docs.py + - name: Run generate-variables-docs.py run: | - python3 flow/scripts/generate-variable-docs.py + python3 flow/scripts/generate-variables-docs.py - name: Run yamlfix run: | pip install yamlfix==1.17.0 From 9222f9c23bbd1316fcd04a95b6c8c0baf5e5a8cb Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sat, 9 Nov 2024 16:08:32 +0000 Subject: [PATCH 5/9] shift flowvar check to above. fix pr-update to run only on push Signed-off-by: Jack Luar --- .github/workflows/github-actions-yaml-test.yml | 8 ++++++-- docs/user/FlowVariables.md | 10 ++++++---- flow/scripts/variables.yaml | 7 +++---- yamlfix.toml | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index 832a6fdb92..e19cca9761 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -7,7 +7,7 @@ on: jobs: docs-test-job: name: 'Tests for variables.yaml' - if: "github.event_name == 'pull_request' || github.event_name == 'push'" + if: github.event_name == 'pull_request' || github.event_name == 'push' runs-on: ubuntu-latest container: image: openroad/ubuntu-cpp20 @@ -19,13 +19,17 @@ jobs: - name: Run generate-variables-docs.py run: | python3 flow/scripts/generate-variables-docs.py + - name: Check if FlowVariables.md is up to date + run: | + git diff --exit-code docs/user/FlowVariables.md - name: Run yamlfix check run: | + pip install yamlfix==1.17.0 yamlfix -c yamlfix.toml flow/scripts/variables.yaml --check docs-pr-update: name: 'Create PR to update ORFS FlowVariables.md and variables.yaml' - if: "github.event_name == 'pull_request' || github.event_name == 'push'" + if: github.event_name == 'push' needs: - docs-test-job runs-on: ubuntu-latest diff --git a/docs/user/FlowVariables.md b/docs/user/FlowVariables.md index 5d84f396df..18d768ef8b 100644 --- a/docs/user/FlowVariables.md +++ b/docs/user/FlowVariables.md @@ -94,6 +94,7 @@ configuration file. | FILL_CELLS| Fill cells are used to fill empty sites. If not set or empty, fill cell insertion is skipped.| | | | FILL_CONFIG| JSON rule file for metal fill during chip finishing.| | | | FLOORPLAN_DEF| Use the DEF file to initialize floorplan.| | | +| GDS_ALLOW_EMPTY| Regular expression of module names of macros that have no .gds file| | | | GDS_FILES| Path to platform GDS files.| | | | GENERATE_ARTIFACTS_ON_FAILURE| For instance Bazel needs artifacts (.odb and .rpt files) on a failure to allow the user to save hours on re-running the failed step locally, but when working with a Makefile flow, it is more natural to fail the step and leave the user to manually inspect the logs and artifacts directly via the file system. Set to 1 to change the behavior to generate artifacts upon failure to e.g. do a global route. The exit code will still be non-zero on all other failures that aren't covered by the "useful to inspect the artifacts on failure" use-case. Example: just like detailed routing, a global route that fails with congestion, is not a build failure(as in exit code non-zero), it is a successful(as in zero exit code) global route that produce reports detailing the problem. Detailed route will not proceed, if there is global routing congestion This allows build systems, such as bazel, to create artifacts for global and detailed route, even if the operation had problems, without having know about the semantics between global and detailed route. Considering that global and detailed route can run for a long time and use a lot of memory, this allows inspecting results on a laptop for a build that ran on a server.| 0| | | GLOBAL_PLACEMENT_ARGS| Use additional tuning parameters during global placement other than default args defined in global_place.tcl.| | | @@ -102,7 +103,7 @@ configuration file. | GPL_ROUTABILITY_DRIVEN| Specifies whether the placer should use routability driven placement.| 1| | | GPL_TIMING_DRIVEN| Specifies whether the placer should use timing driven placement.| 1| | | GUI_TIMING| Load timing information when opening GUI. For large designs, this can be quite time consuming. Useful to disable when investigating non-timing aspects like floorplan, placement, routing, etc.| 1| | -| HOLD_SLACK_MARGIN| Specifies a time margin for the slack when fixing hold violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). Use min of HOLD_SLACK_MARGIN and 0(default hold slack margin) in floorplan. This avoids overrepair in floorplan for hold by default, but allows skipping hold repair using a negative HOLD_SLACK_MARGIN. Exiting timing repair early is useful in exploration where the .sdc has a fixed clock period at designs target clock period and where HOLD/SETUP_SLACK_MARGIN is used to avoid overrepair(extremely long running times) when exploring different parameter settings.| 0| | +| HOLD_SLACK_MARGIN| Specifies a time margin for the slack when fixing hold violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). floorplan.tcl uses min of HOLD_SLACK_MARGIN and 0(default hold slack margin). This avoids overrepair in floorplan for hold by default, but allows skipping hold repair using a negative HOLD_SLACK_MARGIN. Exiting timing repair early is useful in exploration where the .sdc has a fixed clock period at the design's target clock period and where HOLD/SETUP_SLACK_MARGIN is used to avoid overrepair(extremely long running times) when exploring different parameter settings. When an ideal clock is used, that is before CTS, a clock insertion delay of 0 is used in timing paths. This creates a mismatch between macros that have a .lib file from after CTS, when the clock is propagated. To mitigate this, OpenSTA will use subtract the clock insertion delay of macros when calculating timing with ideal clock. Provided that min_clock_tree_path and max_clock_tree_path are in the .lib file, which is the case for macros built with OpenROAD. This is less accurate than if OpenROAD had created a placeholder clock tree for timing estimation purposes prior to CTS. There will inevitably be inaccuracies in the timing calculation prior to CTS. Use a slack margin that is low enough, even negative, to avoid overrepair. Inaccuracies in the timing prior to CTS can also lead to underrepair, but there no obvious and simple way to avoid underrapir in these cases. Overrepair can lead to excessive runtimes in repair or too much buffering being added, which can present itself as congestion of hold cells or buffer cells. Another use of SETUP/HOLD_SLACK_MARGIN is design parameter exploration when trying to find the minimum clock period for a design. The SDC_FILE for a design can be quite complicated and instead of modifying the clock period in the SDC_FILE, which can be non-trivial, the clock period can be fixed at the target frequency and the SETUP/HOLD_SLACK_MARGIN can be swept to find a plausible current minimum clock period.| 0| | | IO_CONSTRAINTS| File path to the IO constraints .tcl file.| | | | IO_PLACER_H| The metal layer on which to place the I/O pins horizontally (top and bottom of the die).| | | | IO_PLACER_V| The metal layer on which to place the I/O pins vertically (sides of the die).| | | @@ -137,13 +138,13 @@ configuration file. | PWR_NETS_VOLTAGES| Used for IR Drop calculation.| | | | RCX_RULES| RC Extraction rules file path.| | | | RECOVER_POWER| Specifies how many percent of paths with positive slacks can be slowed for power savings [0-100].| 0| | -| REMOVE_ABC_BUFFERS| Remove abc buffers from the netlist. If timing repair in floorplanning is taking too long, use a SETUP_HOLD_MARGIN to terminate timing repair early instead of using REMOVE_ABC_BUFFERS or set SKIP_LAST_GAST=1.| | yes| +| REMOVE_ABC_BUFFERS| Remove abc buffers from the netlist. If timing repair in floorplanning is taking too long, use a SETUP/HOLD_SLACK_MARGIN to terminate timing repair early instead of using REMOVE_ABC_BUFFERS or set SKIP_LAST_GASP=1.| | yes| | REMOVE_CELLS_FOR_EQY| String patterns directly passed to write_verilog -remove_cells <> for equivalence checks.| | | | REPAIR_PDN_VIA_LAYER| Remove power grid vias which generate DRC violations after detailed routing.| | | | REPORT_CLOCK_SKEW| Report clock skew as part of reporting metrics, starting at CTS, before which there is no clock skew. This metric can be quite time-consuming, so it can be useful to disable.| 1| | | RESYNTH_AREA_RECOVER| Enable re-synthesis for area reclaim.| 0| | | RESYNTH_TIMING_RECOVER| Enable re-synthesis for timing optimization.| 0| | -| ROUTING_LAYER_ADJUSTMENT| Default routing layer adjustment| 0.5| | +| ROUTING_LAYER_ADJUSTMENT| Adjusts routing layer capacities to manage congestion and improve detailed routing. High values ease detailed routing but risk excessive detours and long global routing times, while low values reduce global routing failure but can complicate detailed routing. The global routing running time normally reduces dramatically(entirely design specific, but going from hours to minutes has been observed) when the value is low(such as 0.10). Sometimes, global routing will succeed with lower values and fail with higher values. Exploring results with different values can help shed light on the problem. Start with a too low value, such as 0.10, and bisect to value that works by doing multiple global routing runs. As a last resort, `make global_route_issue` and using the tools/OpenROAD/etc/deltaDebug.py can be useful to debug global routing errors. If there is something specific that is impossible to route, such as a clock line over a macro, global routing will terminate with DRC errors routes that could have been routed were it not for the specific impossible routes. deltaDebug.py should weed out the possible routes and leave a minimal failing case that pinpoints the problem.| 0.5| | | RTLMP_AREA_WT| Weight for the area of the current floorplan.| 0.1| | | RTLMP_ARGS| Overrides all other RTL macro placer arguments.| | | | RTLMP_BOUNDARY_WT| Weight for the boundary or how far the hard macro clusters are from boundaries.| 50.0| | @@ -167,7 +168,7 @@ configuration file. | SDC_FILE| The path to design constraint (SDC) file.| | | | SDC_GUT| Load design and remove all internal logic before doing synthesis. This is useful when creating a mock .lef abstract that has a smaller area than the amount of logic would allow. bazel-orfs uses this to mock SRAMs, for instance.| | | | SEAL_GDS| Seal macro to place around the design.| | | -| SETUP_SLACK_MARGIN| Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack).| 0| | +| SETUP_SLACK_MARGIN| Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). See HOLD_SLACK_MARGIN for more details.| 0| | | SET_RC_TCL| Metal & Via RC definition file path.| | | | SKIP_CTS_REPAIR_TIMING| Skipping CTS repair, which can take a long time, can be useful in architectural exploration or when getting CI up and running.| | | | SKIP_GATE_CLONING| Do not use gate cloning transform to fix timing violations (default: use gate cloning).| | | @@ -343,6 +344,7 @@ configuration file. ## final variables - [ADDITIONAL_GDS](#ADDITIONAL_GDS) +- [GDS_ALLOW_EMPTY](#GDS_ALLOW_EMPTY) - [GND_NETS_VOLTAGES](#GND_NETS_VOLTAGES) - [MAX_ROUTING_LAYER](#MAX_ROUTING_LAYER) - [MIN_ROUTING_LAYER](#MIN_ROUTING_LAYER) diff --git a/flow/scripts/variables.yaml b/flow/scripts/variables.yaml index 2acd52398c..3dbb4d52cd 100644 --- a/flow/scripts/variables.yaml +++ b/flow/scripts/variables.yaml @@ -106,10 +106,9 @@ CORE_AREA: stages: - floorplan REPORT_CLOCK_SKEW: - description: - Report clock skew as part of reporting metrics, starting at CTS, - before which there is no clock skew. - + description: > + Report clock skew as part of reporting metrics, starting at CTS, before which + there is no clock skew. This metric can be quite time-consuming, so it can be useful to disable. stages: - cts diff --git a/yamlfix.toml b/yamlfix.toml index 7226384a44..2374071179 100644 --- a/yamlfix.toml +++ b/yamlfix.toml @@ -1,5 +1,5 @@ explicit_start = true -line_length = 120 +line_length = 80 preserve_quotes = true quote_representation = "'" section_whitelines = 0 From 57d60f413a07a76dcb6ce21c03c369a4c5968ec4 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Sat, 9 Nov 2024 16:18:08 +0000 Subject: [PATCH 6/9] simplify gh workflow - do not run in container Signed-off-by: Jack Luar --- .github/workflows/github-actions-yaml-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index e19cca9761..e7280364bb 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -9,13 +9,9 @@ jobs: name: 'Tests for variables.yaml' if: github.event_name == 'pull_request' || github.event_name == 'push' runs-on: ubuntu-latest - container: - image: openroad/ubuntu-cpp20 steps: - name: Checkout repository uses: actions/checkout@v4 - with: - submodules: 'recursive' - name: Run generate-variables-docs.py run: | python3 flow/scripts/generate-variables-docs.py From 89582a315a5b07cd620b5efc7dd8fd4d8298ffe4 Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Tue, 24 Dec 2024 05:12:42 +0000 Subject: [PATCH 7/9] simplify ci: remove docs-pr-update task Signed-off-by: Jack Luar --- .../workflows/github-actions-yaml-test.yml | 57 ------------------- 1 file changed, 57 deletions(-) diff --git a/.github/workflows/github-actions-yaml-test.yml b/.github/workflows/github-actions-yaml-test.yml index e7280364bb..f5e272aa8a 100644 --- a/.github/workflows/github-actions-yaml-test.yml +++ b/.github/workflows/github-actions-yaml-test.yml @@ -1,7 +1,6 @@ name: ORFS variables.yaml tester and linter on: - push: pull_request: jobs: @@ -22,59 +21,3 @@ jobs: run: | pip install yamlfix==1.17.0 yamlfix -c yamlfix.toml flow/scripts/variables.yaml --check - - docs-pr-update: - name: 'Create PR to update ORFS FlowVariables.md and variables.yaml' - if: github.event_name == 'push' - needs: - - docs-test-job - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - name: Run generate-variables-docs.py - run: | - python3 flow/scripts/generate-variables-docs.py - - name: Run yamlfix - run: | - pip install yamlfix==1.17.0 - yamlfix -c yamlfix.toml flow/scripts/variables.yaml - - name: Create branch if diff exists - id: variables-update - run: | - git config --local user.email "github-actions[bot]@users.noreply.github.com" - git config --local user.name "github-actions[bot]" - if [ -n "$(git status --porcelain)" ]; then - echo "has_update=true" >> "$GITHUB_OUTPUT" - else - echo "has_update=false" >> "$GITHUB_OUTPUT" - fi - git add flow/scripts/variables.yaml - git add docs/user/FlowVariables.md - git commit --signoff -m "[Docs]: Update ORFS FlowVariables.md" - - # Sanity check that no other files are unstaged - if [ -n "$(git status --porcelain)" ]; then - echo "Error: Unstaged changes after commit." - exit 1 - fi - - name: Only push if not master - if: "github.event.client_payload.branch != 'master'" - id: variables-update-pr - run: | - git push origin "HEAD:refs/pull/${{ github.event.client_payload.branch }}/head" - - name: Create variables update PR - if: "steps.variables-update.outputs.has_update == 'true' && github.event.client_payload.branch == 'master'" - uses: peter-evans/create-pull-request@v5 - with: - token: ${{ github.token }} - signoff: true - delete-branch: true - title: "[BOT] Update ORFS variables" - reviewers: | - vvbandeira - maliberty - draft: true - branch: bot-update-variables - commit-message: | - [BOT] Update ORFS variables From b4a917778a633996db964a9dcebb52bf2909aaea Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Tue, 24 Dec 2024 05:20:46 +0000 Subject: [PATCH 8/9] yamlfix variables.yaml Signed-off-by: Jack Luar --- flow/scripts/variables.yaml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/flow/scripts/variables.yaml b/flow/scripts/variables.yaml index 3dbb4d52cd..26d47aec96 100644 --- a/flow/scripts/variables.yaml +++ b/flow/scripts/variables.yaml @@ -14,7 +14,6 @@ GENERATE_ARTIFACTS_ON_FAILURE: congestion, is not a build failure(as in exit code non-zero), it is a successful(as in zero exit code) global route that produce reports detailing the problem. - Detailed route will not proceed, if there is global routing congestion This allows build systems, such as bazel, to create artifacts for global and detailed route, even if the operation had problems, without having know @@ -42,18 +41,15 @@ ROUTING_LAYER_ADJUSTMENT: but risk excessive detours and long global routing times, while low values reduce global routing failure but can complicate detailed routing. - The global routing running time normally reduces dramatically(entirely design specific, but going from hours to minutes has been observed) when the value is low(such as 0.10). - Sometimes, global routing will succeed with lower values and fail with higher values. Exploring results with different values can help shed light on the problem. Start with a too low value, such as 0.10, and bisect to value that works by doing multiple global routing runs. - As a last resort, `make global_route_issue` and using the tools/OpenROAD/etc/deltaDebug.py can be useful to debug global routing errors. If there is something specific that is @@ -401,17 +397,13 @@ HOLD_SLACK_MARGIN: Specifies a time margin for the slack when fixing hold violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). - floorplan.tcl uses min of HOLD_SLACK_MARGIN and 0(default hold slack margin). - This avoids overrepair in floorplan for hold by default, but allows skipping hold repair using a negative HOLD_SLACK_MARGIN. - Exiting timing repair early is useful in exploration where the .sdc has a fixed clock period at the design's target clock period and where HOLD/SETUP_SLACK_MARGIN is used to avoid overrepair(extremely long running times) when exploring different parameter settings. - When an ideal clock is used, that is before CTS, a clock insertion delay of 0 is used in timing paths. This creates a mismatch between macros that have a .lib file from after CTS, when @@ -422,20 +414,16 @@ HOLD_SLACK_MARGIN: macros built with OpenROAD. This is less accurate than if OpenROAD had created a placeholder clock tree for timing estimation purposes prior to CTS. - There will inevitably be inaccuracies in the timing calculation prior to CTS. Use a slack margin that is low enough, even negative, to avoid overrepair. Inaccuracies in the timing prior to CTS can also lead to underrepair, but there no obvious and simple way to avoid underrapir in these cases. - Overrepair can lead to excessive runtimes in repair or too much buffering being added, which can present itself as congestion of hold cells or buffer cells. - Another use of SETUP/HOLD_SLACK_MARGIN is design parameter exploration when trying to find the minimum clock period for a design. - The SDC_FILE for a design can be quite complicated and instead of modifying the clock period in the SDC_FILE, which can be non-trivial, the clock period can be fixed at the target frequency and the @@ -451,7 +439,6 @@ SETUP_SLACK_MARGIN: Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). - See HOLD_SLACK_MARGIN for more details. stages: - cts From 3c60cba0d8a4e7a77fb7c9c01bfd89cb04334b0b Mon Sep 17 00:00:00 2001 From: Jack Luar Date: Tue, 24 Dec 2024 06:38:20 +0000 Subject: [PATCH 9/9] Update FlowVariables.md Signed-off-by: Jack Luar --- docs/user/FlowVariables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/FlowVariables.md b/docs/user/FlowVariables.md index 18d768ef8b..f3f20475a2 100644 --- a/docs/user/FlowVariables.md +++ b/docs/user/FlowVariables.md @@ -168,7 +168,7 @@ configuration file. | SDC_FILE| The path to design constraint (SDC) file.| | | | SDC_GUT| Load design and remove all internal logic before doing synthesis. This is useful when creating a mock .lef abstract that has a smaller area than the amount of logic would allow. bazel-orfs uses this to mock SRAMs, for instance.| | | | SEAL_GDS| Seal macro to place around the design.| | | -| SETUP_SLACK_MARGIN| Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). See HOLD_SLACK_MARGIN for more details.| 0| | +| SETUP_SLACK_MARGIN| Specifies a time margin for the slack when fixing setup violations. This option allows you to overfix or underfix(negative value, terminate retiming before 0 or positive slack). See HOLD_SLACK_MARGIN for more details.| 0| | | SET_RC_TCL| Metal & Via RC definition file path.| | | | SKIP_CTS_REPAIR_TIMING| Skipping CTS repair, which can take a long time, can be useful in architectural exploration or when getting CI up and running.| | | | SKIP_GATE_CLONING| Do not use gate cloning transform to fix timing violations (default: use gate cloning).| | |