Variables sweep by oharboe · Pull Request #4141 · The-OpenROAD-Project/OpenROAD-flow-scripts

oharboe · 2026-04-13T15:39:02Z

No description provided.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

gemini-code-assist

Code Review

This pull request introduces and documents a wide range of new flow variables for the OpenROAD flow, updating FlowVariables.md and variables.yaml. The review feedback highlights several improvement opportunities, including clarifying case sensitivity for corner-specific variables, addressing redundancies between SETUP_MOVE_SEQUENCE and SETUP_REPAIR_SEQUENCE, and resolving naming confusion between similar variables like DFF_LIB_FILE and SYNTH_KEPT_MODULES. Additionally, the reviewer pointed out that CTS_BUF_CELL lacks integration in the synthesis scripts.

gemini-code-assist · 2026-04-13T15:40:59Z

+CORNERS:
+  description: >
+    List of timing corner names for multi-corner analysis (e.g. "slow fast").
+    When set, liberty files are loaded per corner using [CORNER]_LIB_FILES
+    environment variables and timing models are generated per corner.
+  stages:
+    - All stages


The notation [CORNER]_LIB_FILES is ambiguous regarding case sensitivity. Since environment variables are case-sensitive, it would be helpful to clarify if the corner name should be in uppercase or lowercase (e.g., SLOW_LIB_FILES vs slow_LIB_FILES).

gemini-code-assist · 2026-04-13T15:40:59Z

+SETUP_MOVE_SEQUENCE:
+  description: >
+    Passed as -sequence to repair_timing. This should be a string of move
+    keywords separated by commas.
+  stages:
+    - cts
+    - floorplan
+    - grt


SETUP_MOVE_SEQUENCE appears to be redundant with the existing SETUP_REPAIR_SEQUENCE. Both are described as comma-separated strings of move keywords for repair_timing. Consider consolidating these into a single variable or clearly documenting the distinction between them to avoid user confusion.

This appears to be a pre-existing bug but please just remove SETUP_REPAIR_SEQUENCE as it isn't connected to anything.

gemini-code-assist · 2026-04-13T15:40:59Z

+DFF_LIB_FILE:
+  description: >
+    Single Liberty file for flip-flop technology mapping (dfflibmap) during
+    synthesis. Takes precedence over DFF_MAP_FILE when set.
+  stages:
+    - synth


DFF_LIB_FILE (singular) is added and used in synth.tcl, but DFF_LIB_FILES (plural) already exists in the configuration. Since Yosys dfflibmap only supports a single liberty file, the plural version might be misleading. Consider deprecating the plural version if it is no longer intended for use.

It appears DFF_LIB_FILES was misnamed and just should be renamed to DFF_LIB_FILE

gemini-code-assist · 2026-04-13T15:40:59Z

+SYNTH_KEPT_MODULES:
+  description: >
+    Space-separated list of modules to keep as separate hierarchies during
+    synthesis. Unlike the automatic size-based discovery
+    (SYNTH_MINIMUM_KEEP_SIZE), the list of kept modules is known statically,
+    which enables build systems to synthesize them in parallel.
+  stages:
+    - synth


SYNTH_KEPT_MODULES is very similar in name to SYNTH_KEEP_MODULES. While the description mentions parallel synthesis for build systems, SYNTH_KEEP_MODULES is the one currently used in synth.tcl to preserve hierarchy. This naming similarity can cause confusion; consider merging them or using more distinct names to clarify their roles.

SYNTH_KEPT_MODULES is wrong

gemini-code-assist · 2026-04-13T15:40:59Z

+CTS_BUF_CELL:
+  description: |
+    Single cell name for clock tree buffer insertion.
+  stages:
+    - cts


CTS_BUF_CELL is defined here but is not integrated into the cts_args list in cts.tcl. If it is intended to be used as a root buffer or a specific cell for clock tree synthesis, it should be appended to the arguments passed to clock_tree_synthesis.

CTS_BUF_LIST is already there, what is this?

maliberty · 2026-04-13T22:16:31Z

Agreed with various gemini comments above

Not connected to anything in the codebase. Only SETUP_MOVE_SEQUENCE is used (in util.tcl). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

DFF_LIB_FILES (plural) was never used. The actual variable is DFF_LIB_FILE (singular), used in synth.tcl for dfflibmap. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

The actual variable used in synth.tcl is SYNTH_KEEP_MODULES, which is already documented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Not used anywhere in the codebase. CTS_BUF_LIST is the variable used in cts.tcl and is already documented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

read_liberty.tcl uppercases the corner name via [string toupper], so "slow" maps to SLOW_LIB_FILES. Document this to remove ambiguity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

oharboe · 2026-04-14T15:51:28Z

@maliberty reduce scope to make it paletable. We can refine the rest later in seperate PRs.

oharboe and others added 19 commits April 13, 2026 17:37

Add CORNERS to variables.yaml

16fa054

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add ADDITIONAL_SITES to variables.yaml

74241eb

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add GPL_KEEP_OVERFLOW to variables.yaml

b8f4a00

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add SETUP_MOVE_SEQUENCE to variables.yaml

d8eb357

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add SKIP_BUFFER_REMOVAL to variables.yaml

e595894

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add VIA_IN_PIN_MIN_LAYER to variables.yaml

33a983a

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add VIA_IN_PIN_MAX_LAYER to variables.yaml

0bb21d2

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add DISABLE_VIA_GEN to variables.yaml

9f0ba99

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add OR_K to variables.yaml

46ed1b8

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add OR_SEED to variables.yaml

49693cb

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add DFF_LIB_FILE to variables.yaml

8d2d09d

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add SYNTH_KEPT_MODULES to variables.yaml

4d01dd2

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add SYNTH_OPERATIONS_ARGS to variables.yaml

641d80c

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add CTS_BUF_CELL to variables.yaml

e0f5a56

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add CLUSTER_FLOPS_ARGS to variables.yaml

9cfb092

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add ENABLE_RESISTANCE_AWARE to variables.yaml

6d3056f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add LEC_AUX_VERILOG_FILES to variables.yaml

d3c3658

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add CACHED_REPORTS to variables.yaml

2365a89

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Add CDL_FILE to variables.yaml

c35aff8

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

gemini-code-assist Bot reviewed Apr 13, 2026

View reviewed changes

oharboe requested a review from maliberty April 13, 2026 15:43

oharboe and others added 7 commits April 14, 2026 17:47

Remove SETUP_REPAIR_SEQUENCE from variables.yaml

5ff048e

Not connected to anything in the codebase. Only SETUP_MOVE_SEQUENCE is used (in util.tcl). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Remove wrong SYNTH_KEPT_MODULES from variables.yaml

b3e8c5b

The actual variable used in synth.tcl is SYNTH_KEEP_MODULES, which is already documented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Remove CTS_BUF_CELL from variables.yaml

c9ecd6c

Not used anywhere in the codebase. CTS_BUF_LIST is the variable used in cts.tcl and is already documented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

Merge remote-tracking branch 'origin/master' into HEAD

df68f30

variables: doc update

5707982

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

maliberty approved these changes Apr 14, 2026

View reviewed changes

maliberty enabled auto-merge April 14, 2026 17:27

maliberty merged commit 4af876b into The-OpenROAD-Project:master Apr 14, 2026
7 of 8 checks passed

oharboe deleted the variables-sweep branch April 19, 2026 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Variables sweep#4141

Variables sweep#4141
maliberty merged 26 commits into
The-OpenROAD-Project:masterfrom
oharboe:variables-sweep

oharboe commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

maliberty Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

maliberty Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

maliberty Apr 13, 2026

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Uh oh!

maliberty Apr 13, 2026

Uh oh!

maliberty commented Apr 13, 2026

Uh oh!

oharboe commented Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

oharboe commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

maliberty commented Apr 13, 2026

Uh oh!

oharboe commented Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants