Skip to content

Consistent handling of arguments and options - #4771

Open
oharboe wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
oharboe:args-grammar
Open

oharboe wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
oharboe:args-grammar

Conversation

@oharboe

@oharboe oharboe commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

This is a problem well worth solving once and for all. Of course I had AI help me here, but I've read all of the below, I couldn't have written it better or shorter, and I stand by it.

Backwards compatible, minimum congitive load and introduce necessary complexity slowly to the user, POLA and with a deprecation strategy

If you use ORFS variables and find it hard to remember which option has which name, and whether there's any system to them, this is for you. Nobody did anything wrong: adding a variable when one was needed, and putting off a system until a clear pattern and a real problem showed up, has worked. Both are here now. There are 253 variables, added one at a time, and no rules a user can learn once.

This pull request offers a single system. To change what ORFS passes to an OpenROAD command, you set the variable named after the command and write that command's own options. It's the same for every command, with nothing to look up, and every existing configuration keeps working.

It is a complicated subject, which is why the description is long. Please don't skip it for being long or AI-written; the details are where the problem is.

It started one morning when I saw the much awaited clock_tree_synthesis -useful_skew and went to try it in ORFS. There was no variable for it, and the only way in was CTS_ARGS. Let's go down the rabbit hole of fixing this once and for all.

CTS_ARGS replaces the flow's arguments: it discards CTS_CLUSTER_SIZE, the four other CTS_* variables and the flow's own -sink_clustering_enable -repair_clock_nets, so a user first runs the flow to learn what to put in it.

ORFS has 253 variables. "Off" is spelled SKIP_, DISABLE_, DONT_ and ENABLE_DPO=0. Giving each option of the 11 OpenROAD commands ORFS already wraps its own variable in that style would take at least 9,856 names; that number grows by about 1,250 a year, and ORFS adds about 66 variables a year.

Options of the 11 commands ORFS wraps, 2020 to 2026: all options rise from 53 to 209, those without an ORFS variable from 48 to 151, those with one from 5 to 58

Counted each October on the commit bazel-orfs pins; the table is under "Names over time".

The current rules were never written down. They grew one variable at a time. To configure a command today, a user has to know:

  • Which options have an ORFS name, and what it is. -sink_clustering_size is CTS_CLUSTER_SIZE, -notch_weight is RTLMP_NOTCH_WT, and -useful_skew has no name at all.
  • Whether a command's *_ARGS replaces or adds. Three replace what the flow passes and ten add to it. Setting RTLMP_ARGS silently discards all 21 RTLMP_* variables, and -target_util too, which comes from PLACE_DENSITY.
  • Which of four spellings means "off" for this switch, and which way round it is. One switch, repair_timing, is spelled three ways depending on the stage: SKIP_FLOORPLAN_REPAIR_TIMING, ENABLE_PLACE_REPAIR_TIMING and SKIP_CTS_REPAIR_TIMING. Two of them have one polarity and the third has the other.
  • Which settings quietly change others. Leaving RTLMP_MIN_CHANNEL_SIZE unset turns RTLMP_PIN_AWARE_CHANNELS off.

Some options have two names: GPL_TIMING_DRIVEN and -timing_driven in GLOBAL_PLACEMENT_ARGS, GLOBAL_ROUTE_USE_CUGR and -use_cugr, CTS_CLUSTER_SIZE and -sink_clustering_size in CTS_ARGS. Each pair is a question about which one wins. Twelve of the 21 RTLMP_* variables are set by no design or platform in ORFS, and two more are set by the flow and never read.

Here is how one command, clock_tree_synthesis, gets its arguments today:

flowchart LR
  A[CTS_BUF_DISTANCE] --> L[cts_args]
  B[CTS_CLUSTER_SIZE] --> L
  C[CTS_CLUSTER_DIAMETER] --> L
  D[CTS_BUF_LIST] --> L
  E[CTS_LIB_NAME] --> L
  F["-sink_clustering_enable<br/>-repair_clock_nets<br/>(hard-coded)"] --> L
  L --> X{CTS_ARGS set?}
  G[CTS_ARGS] --> X
  X -- no --> CMD[clock_tree_synthesis]
  X -- "yes: everything above is discarded" --> CMD
Loading

Research on naming:

  • People rarely pick the same name for the same thing. Two people choose the same term for a concept well under 20% of the time (Furnas et al., "The Vocabulary Problem in Human-System Communication", 1987). Every invented name sends a user to the documentation, and a duplicate name sends them twice: once to find it, and once to learn that a second one exists.
  • Negation slows reading and causes errors. A negated statement takes longer to check and is misread more often than an affirmative one (Clark and Chase, 1972). SKIP_CTS_REPAIR_TIMING=0 is a small puzzle, solved again every time it's read.
  • Two vocabularies split attention. The log prints -sink_clustering_size 30, the config says CTS_CLUSTER_SIZE=30, and the user translates between them from memory, in both directions, every time.
  • What's learned doesn't transfer. CTS_CLUSTER_SIZE means nothing in OpenROAD's documentation, its tests, its Tcl prompt or its issues.

Research on making names:

  • Bikeshedding. By Parkinson's law of triviality, a group spends time on an issue in inverse proportion to its importance, because everyone can have an opinion on a simple one. Few reviewers can judge a change to the router; all of them can argue _WT against _WEIGHT. Every new variable invites that argument.
  • The IKEA effect. People value what they built themselves (Norton, Mochon and Ariely, 2012). A name someone defended in review is theirs to keep.

This pull request names a variable after the command it's for, in upper case. It holds that command's own options, spelled as OpenROAD spells them, and only the ones that differ, in one grammar written down in one place.

Background: stages

ORFS runs a design through seven stages. Each stage is a separate tool run: it reads the previous stage's database (an .odb file) and writes its own.

flowchart LR
  synth["synth<br/>RTL → netlist<br/>(yosys)"] --> floorplan["floorplan<br/>die, macros,<br/>power grid"]
  floorplan --> place["place<br/>cells placed,<br/>resized"]
  place --> cts["cts<br/>clock tree<br/>built"]
  cts --> grt["grt<br/>global route:<br/>wires planned"]
  grt --> route["route<br/>detailed route:<br/>wires drawn"]
  route --> final["final<br/>fill, layout,<br/>reports"]
Loading

Several commands run in more than one stage: repair_timing in floorplan, place (when ENABLE_PLACE_REPAIR_TIMING is set), cts and grt, detailed_placement in place, cts and grt, and report_metrics at the end of nearly every stage. The same command sees a different design each time, because what the tool knows about wires gets better stage by stage. At floorplan nothing is placed, at cts wire lengths are estimated from placement, and at grt they come from planned routes. So a user has good reasons to set the same command differently per stage:

  • a timing margin that is right once wires are known is too pessimistic before they are;
  • repair_timing at floorplan can take long and decide little, so in early exploration it's turned off there and kept at cts and grt;
  • detailed reports are wanted at the stage being studied, and nowhere else, because they cost time on big designs.

Today each such choice needs its own variable per stage: SKIP_FLOORPLAN_REPAIR_TIMING, ENABLE_PLACE_REPAIR_TIMING, SKIP_CTS_REPAIR_TIMING, DETAILED_METRICS, SKIP_ANTENNA_REPAIR and SKIP_ANTENNA_REPAIR_POST_DRT. The grammar says it with a scope: REPAIR_TIMING_ARGS = floorplan:off.

Background: variables.yaml

Every ORFS variable has an entry in flow/scripts/variables.yaml:

CTS_CLUSTER_SIZE:
  description: >
    Maximum number of sinks per cluster.
  stages:
    - cts
  tunable: 1
  type: int
SKIP_CTS_REPAIR_TIMING:
  description: >
    Skipping CTS repair, which can take a long time, can be useful in
    architectural exploration or when getting CI up and running.
  stages:
    - cts
  default: 0

All 249 entries have a description, which generates the documentation. 210 list the stages that read them; before a stage starts, ORFS erases every variable the stage doesn't read (erase_non_stage_variables), so a stage's result depends only on its own variables. 85 have a default, which is fed back into Make as ?=. A few also have a type, tunable (for automatic parameter search) or required.

The stages list is why a stage scope inside a value matters: a variable read by several stages, such as REPAIR_TIMING_ARGS, reaches each of them, and each stage uses only the edits for it.

Background: how ORFS picks a variable's value today

ORFS is a Makefile, and a variable's value is whatever Make's rules leave standing. Make reads the files in this order:

flowchart TB
  S["settings.mk<br/>optional, local to a checkout"] --> D["the design's config.mk<br/>DESIGN_CONFIG, often with ifeq FLOW_VARIANT blocks"]
  D --> P["the platform's config.mk<br/>platforms/asap7/config.mk, mostly ?="]
  P --> Y["variables.yaml defaults<br/>emitted as export VAR ?= value"]
  Y --> T["Tcl scripts read the exported variables<br/>and build each command's options"]
Loading

VAR = value sets a value; VAR ?= value sets it only if nothing has yet. On top of the files, Make has two more sources: the command line, which beats every assignment in every file, and the shell environment, which beats ?= but loses to =. For one variable, the winner is the first of these that applies:

Priority Where it's set Example
1 make command line make DESIGN_CONFIG=... CORE_UTILIZATION=40
2 design config.mk, with = export CORE_UTILIZATION = 30
3 shell environment export CORE_UTILIZATION=40; make ...
4 design config.mk, with ?= export CORE_UTILIZATION ?= 30
5 platform config.mk, with ?= export RTLMP_MIN_CHANNEL_SIZE ?= 20 20
6 variables.yaml default CORE_UTILIZATION: default: ...

This works well for a variable with one value. It doesn't work for a list of options. Whoever wins takes the whole value, and every layer below is lost. That's why CTS_ARGS discards the five CTS_* variables and the flow's own defaults: Make has no way to say "these defaults, except this one option", so ORFS grew a variable per option to get round it.

Before and now

Before: to change one argument, a user runs the flow to see what it passes, copies all of it, and edits the copy, or looks for the ORFS name of the option, if there is one.

Now: one variable per command, holding only what is different:

CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30 ~-repair_clock_nets
REPAIR_TIMING_ARGS        = -setup_margin 0.05 grt:-setup_margin 0.1 floorplan:off
REPORT_METRICS_ARGS       = off cts:on

Reading a variable takes no ORFS documentation. The name says which command it's for, and the contents are that command's options as its help prints them.

Complexity only when it's needed

The simplest use is a command line: CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30 is what you would type at OpenROAD's prompt. Deleting, stages, calls and platform defaults are there when a user needs them, and invisible when not. Every existing config keeps working unchanged until the deprecation date, and the warnings say what to write instead.

The grammar

Written Meaning
-option value set the value
-flag add the flag
~-option delete the option, and its value if it has one
on / off run the command / don't run it
stage: only in that stage
stage.call: only at that call, by the name the log prints in front of it
{a b} or "a b" a value with spaces in it, as in Tcl

Familiar ideas

We're taking familiar ideas rather than inventing new ones. The edits are written in OpenROAD's own Tcl option syntax (-key value, -flag), so a user writes what the command's help prints, with Tcl's braces or quotes for a value with spaces. Deleting an option borrows Hydra's ~key, and scoping an edit to a stage borrows the build:cts --flag qualifier from Bazel's .bazelrc. Edits combine the way a JSON Merge Patch or a Helm values file does: each layer holds only what differs from the one below. Layers are applied in a fixed order, as with git config levels or Nix module priorities. Within a layer, the more specific scope wins and then the later edit, as in the CSS cascade. Make still chooses each variable's value exactly as before, and the grammar only merges the layers afterwards.

Defaults are layers, written in the same grammar:

flowchart LR
  O["OpenROAD's own default<br/>(nothing passed)"] --> F["the flow's arguments<br/>stage scripts"]
  F --> P["PLATFORM_ARGS<br/>platform config.mk"]
  P --> D["Deprecated variables<br/>CTS_CLUSTER_SIZE, ..."]
  D --> U["CLOCK_TREE_SYNTHESIS_ARGS<br/>design, FLOW_VARIANT, command line"]
  U --> CMD["clock_tree_synthesis<br/>-sink_clustering_size 30 ..."]
Loading

For each option, the highest layer that mentions it decides. Within a layer, a call edit beats a stage edit, a stage edit beats an unscoped one, and later beats earlier. In PLATFORM_ARGS each edit starts with its command: rtl_macro_placer:-min_channel_size {20 20}.

Make still decides each variable's value, exactly as above. What changes is that the layers are separate variables, so no layer's value can wipe out another's: the grammar merges them after Make is done.

For one option of one command, here is the full order, highest first, using -sink_clustering_size as the example:

Priority Where it's set Example
1 make command line make DESIGN_CONFIG=... CLOCK_TREE_SYNTHESIS_ARGS='-sink_clustering_size 40'
2 design config.mk, with =, also inside an ifeq FLOW_VARIANT block export CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30
3 shell environment export CLOCK_TREE_SYNTHESIS_ARGS='-sink_clustering_size 40'; make ...
4 design config.mk, with ?= export CLOCK_TREE_SYNTHESIS_ARGS ?= -sink_clustering_size 30
5 deprecated variable, set in any of the places above make DESIGN_CONFIG=... CTS_CLUSTER_SIZE=40
6 PLATFORM_ARGS, platform config.mk export PLATFORM_ARGS = clock_tree_synthesis:-sink_clustering_size 20
7 the flow's arguments, in cts.tcl -sink_clustering_enable -repair_clock_nets
8 nothing passed OpenROAD's own default

Rows 1 to 4 are Make choosing the one value of CLOCK_TREE_SYNTHESIS_ARGS, as it does for every variable, so make CLOCK_TREE_SYNTHESIS_ARGS=... replaces the design's whole value, not only the options it names. Rows 5 to 8 are separate variables, merged option by option. One consequence: a deprecated variable on the command line, make CTS_CLUSTER_SIZE=40, loses to a design that sets -sink_clustering_size in CLOCK_TREE_SYNTHESIS_ARGS.

Commands that run more than once

Some commands run more than once in a stage: grt calls repair_timing twice, the second time either with a sequence the flow hard-codes (grt.wns) or to recover power (grt.recover_power), and detailed_placement twice. An edit scoped grt: applies to every call of its command in that stage.

To reach one call, each call has a name, and the log prints it in front of the command, with each edit that changed it and the variable the edit came from:

[grt.main] repair_timing
repair_timing -setup_margin 0 -hold_margin 0 -repair_tns 100 -verbose
[grt.wns] repair_timing: -setup_margin 0.1 (REPAIR_TIMING_ARGS)
repair_timing -setup -sequence "vt_swap reroute" -skip_last_gasp -repair_tns 0 -verbose -setup_margin 0.1

The scope is copied from the log: REPAIR_TIMING_ARGS = grt.wns:-setup_margin 0.1 changes only the second. A name says what a call is for, not where it is in the script, so adding a call doesn't move the others. docs/user/CommandArguments.md lists every call.

Why ~, and not -no_option

~ deletes. It doesn't negate: ~-setup_margin means "don't pass -setup_margin", so OpenROAD uses its own default. -no_setup_margin would read as "no margin", a different and wrong meaning. It would also be a name OpenROAD doesn't have, and inventing names is the habit this pull request ends. A prefix that can never start a real option can't collide with one.

The prefix also has to survive every language a value passes through before OpenROAD sees it: Make, the shell, Tcl, Starlark and YAML. Of the prefixes that could work, ~ is the only one none of them needs escaped:

Prefix Needs escaping or quoting in
!-option interactive bash, even inside double quotes (history expansion); YAML, at the start of a value (a tag)
#-option Make (a comment); the shell and YAML, at the start of a word (a comment)
$-option Make (a variable); the shell ($- expands to the shell's flags)
@-option YAML, at the start of a value (reserved)
~-option none

The shell expands ~ only at the start of a word, and only when what follows is empty, a user name, +, - or a number; -option is none of these, and a value with spaces is quoted anyway. Tcl 8 expanded ~ only in file names, and Tcl 9 doesn't at all. YAML reads ~ as null only when it stands alone.

Why a variable per command, and not one ARGS

PLATFORM_ARGS shows that one variable with command: prefixes would work. Designs get a variable per command anyway. The first reason is least surprise: a user who sees CLOCK_TREE_SYNTHESIS_ARGS expects it to hold clock_tree_synthesis options and nothing else, and it does. The second is that one variable would lose three things Make and ORFS already give every variable:

  • A command-line override that touches one command. make REPAIR_TIMING_ARGS='grt:-setup_margin 0.1' replaces the design's repair_timing edits and nothing else. Make replaces a variable's whole value, so with one ARGS the same line would drop the design's edits to every command.
  • Stages. Each *_ARGS entry in variables.yaml lists the stages that run its command, so erase_non_stage_variables and bazel-orfs keep CLOCK_TREE_SYNTHESIS_ARGS out of place with no new code. One variable would be read by every stage.
  • Documentation and search. Each command gets its own entry and description in the generated documentation, and grep -r CLOCK_TREE_SYNTHESIS_ARGS flow/designs finds every design that changes clock tree synthesis.

PLATFORM_ARGS is one variable because a platform sets a few defaults once, and the layers above override them option by option.

Where edits come from

A design writes the differences in its config.mk:

export CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30 ~-repair_clock_nets

A platform sets defaults for every design on it, each edit starting with its command:

export PLATFORM_ARGS = rtl_macro_placer:-min_channel_size {20 20} \
                       detailed_route:-disable_via_gen

A flow variant of a design is an ordinary ifeq block, as today:

ifeq ($(FLOW_VARIANT),fast)
  export REPORT_METRICS_ARGS = off
  export REPAIR_TIMING_ARGS  = floorplan:off cts:off
endif

The command line, for a one-off experiment, replaces the design's value of that variable for this run, as Make does for every variable. The platform's and ORFS's defaults stay. Quote the value for the shell:

make DESIGN_CONFIG=designs/asap7/gcd/config.mk REPAIR_TIMING_ARGS='grt:-setup_margin 0.1'

The shell environment works as it does for every ORFS variable: it is used when the design doesn't set the variable with =.

export REPORT_METRICS_ARGS='off cts:on'
make DESIGN_CONFIG=designs/asap7/gcd/config.mk

A deprecated variable, wherever it's set, still works. It's translated into its edit and lands in the layer between the platform and the design:

export CTS_CLUSTER_SIZE = 30   # warns: write CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30

Errors stop the flow early:

  • A syntax error stops the stage before it runs, before erase_non_stage_variables: every <COMMAND>_ARGS and PLATFORM_ARGS is checked, not only the ones the stage reads.
  • An option used as a flag in one layer and with a value in another is an error.

~-option deletes an option where it is passed, and does nothing at a call that doesn't pass it, so one unscoped deletion covers every call.

One implementation

flow/scripts/args_grammar.bzl is written in the subset of Python that is also Starlark. ORFS runs it as Python: the stage scripts call flow/scripts/args.py with $PYTHON_EXE, through command_args and command_runs in util.tcl, so all of the argument handling is in Python with unit tests, which Tcl doesn't offer. Bazel load()s it, so a Bazel build can stop at analysis on a syntax error and key each stage's action only on the edits that apply to that stage. flow/test/test_args.py tests both, and checks that args_grammar.bzl stays in the common subset: ORFS has no Bazel, so a test walks the file's syntax tree and fails on what Starlark lacks (while, exceptions, classes, f-strings, sets, generators, import, is, recursion, iterating a string, anything at the top level but definitions and assignments). If the grammar ever needs Python that Starlark can't run, the Python here stays the ground truth and bazel-orfs carries its own Starlark copy.

Hooks and other files stay variables

ORFS has 32 hook variables: a PRE_<STEP>_TCL and a POST_<STEP>_TCL for each of 16 steps, sourced at the top and bottom of the step's script. A stage runs one or more steps, and each step runs once, so each hook runs once. The hooks stay as they are, for two reasons.

  • Their names already follow a grammar: PRE_ or POST_, then the step, then _TCL. A user who learns it once can name all 32, so none of them is an invented name.
  • A file needs a variable of its own. bazel-orfs passes files as sources = {"PRE_GLOBAL_PLACE_TCL": [":a.tcl"]}: a variable name mapped to a Bazel label, which Bazel turns into a path and declares as an input of the stage that reads it. A scoped value such as place.global_place:a.tcl would mix a scope into the file name. Bazel labels use : themselves (//flow/designs:a.tcl), so the two collide, and bazel-orfs would have to build the string before it could pass the file.

The same rule holds for every variable that names a file (PDN_TCL, MACRO_PLACEMENT_TCL, FASTROUTE_TCL, ...): one variable, one file, no grammar. For the same reason, an *_ARGS value doesn't name input files. An option that reads a file gets its own variable, so bazel-orfs can declare the file as an input.

Stage Step Hooks
synth synth PRE_SYNTH_TCL, POST_SYNTH_TCL
floorplan floorplan PRE_FLOORPLAN_TCL, POST_FLOORPLAN_TCL
floorplan macro_place PRE_MACRO_PLACE_TCL, POST_MACRO_PLACE_TCL
floorplan tapcell PRE_TAPCELL_TCL, POST_TAPCELL_TCL
floorplan pdn PRE_PDN_TCL, POST_PDN_TCL
place global_place_skip_io PRE_GLOBAL_PLACE_SKIP_IO_TCL, POST_GLOBAL_PLACE_SKIP_IO_TCL
place io_placement PRE_IO_PLACEMENT_TCL, POST_IO_PLACEMENT_TCL
place global_place PRE_GLOBAL_PLACE_TCL, POST_GLOBAL_PLACE_TCL
place resize PRE_RESIZE_TCL, POST_RESIZE_TCL
place detail_place PRE_DETAIL_PLACE_TCL, POST_DETAIL_PLACE_TCL
cts cts PRE_CTS_TCL, POST_CTS_TCL
grt global_route PRE_GLOBAL_ROUTE_TCL, POST_GLOBAL_ROUTE_TCL
route detail_route PRE_DETAIL_ROUTE_TCL, POST_DETAIL_ROUTE_TCL
route fillcell PRE_FILLCELL_TCL, POST_FILLCELL_TCL
final density_fill PRE_DENSITY_FILL_TCL, POST_DENSITY_FILL_TCL
final final_report PRE_FINAL_REPORT_TCL, POST_FINAL_REPORT_TCL

Deprecation

Every deprecated variable keeps working. It counts as an edit in the layer between the platform's and the design's. A stage that reads one set to anything but its default warns, with the line that replaces it:

[WARNING] CTS_CLUSTER_SIZE=30 is deprecated: write CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size 30

variables.yaml holds the replacement next to each one (deprecated: 1, replacement:), and the generated variable documentation shows it. A removal date is for review to set.

CTS_ARGS and RTLMP_ARGS replace what the flow passes, as before. DETAILED_ROUTE_ARGS keeps the same name for the new meaning, so while it holds a plain command line, with no ~, on, off or scope, it still replaces the flow's arguments; where that differs from reading the same text as edits, the log names the ~-option deletions that make the two agree.

Numbers (ORFS 75cd896b, master when this was written)

Variables
variables.yaml today 253
Deprecated: pass one option to one command 48
Deprecated: switch a step on or off 12
Deprecated: *_ARGS renamed to their command's name 3
Added: 10 named after their command, plus PLATFORM_ARGS 11
After the deprecated ones are removed 201

16 commands take a <COMMAND>_ARGS: balance_row_usage, buffer_ports, clock_tree_synthesis, cluster_flops, detailed_placement, detailed_route, global_placement, global_route, improve_placement, place_pins, repair_antennas, repair_design, repair_tie_fanout, repair_timing, report_metrics and rtl_macro_placer. Six of them had one already, under that name or another.

Kept: design inputs, platform facts, choices that span commands (CELL_PAD_IN_SITES_*, GPL_PLACE_IOS, MATCH_CELL_FOOTPRINT), hooks and other variables that name files, and synthesis. The next section lists what is left to make the rest consistent.

Name conflicts, and what is left to be fully consistent

This pull request changes nothing for an existing configuration. Where an old name and the new rule collide, the old meaning wins for now. Being fully consistent takes these follow-ups, each one a behaviour change of its own:

Today Conflict To be fully consistent
DETAILED_ROUTE_ARGS Same name as the new variable; a plain command line in it replaces the flow's arguments Read it as edits only; the log already names the ~-option deletions a design needs
GLOBAL_ROUTE_ARGS Its default, -congestion_report_iter_step 5 -verbose, lives in variables.yaml, not the flow; it edits only the first of up to seven global_route calls, and the incremental reroutes take only -allow_congestion from it Move the default into global_route.tcl, name every call, and let an unscoped edit reach them all
CTS_ARGS, RTLMP_ARGS, DETAIL_PLACEMENT_ARGS Not named after their command; the first two replace the flow's arguments Remove, once designs use the new names
GLOBAL_PLACEMENT_ARGS, PLACE_PINS_ARGS, BUFFER_PORTS_ARGS, CLUSTER_FLOPS_ARGS None: they appended, and now they edit An option given twice now appears once in the log, with the later value, which OpenROAD used anyway
63 deprecated variables, set in 79 design files and 7 platform files (198 settings, TNS_END_PERCENT and SETUP_REPAIR_PHASES the most) They warn Move each design to its <COMMAND>_ARGS and each platform to PLATFORM_ARGS, then remove the variables
SKIP_INCREMENTAL_REPAIR, OPT_POST_GRT_WNS, SKIP_DETAILED_ROUTE, CTS_SNAPSHOTS, REPORT_CLOCK_SKEW, REPORT_ENDPOINT_SLACK Switch a block of commands, or an ORFS procedure, not one command Name the block as a step that takes on/off
GPL_TIMING_DRIVEN, GLOBAL_ROUTE_USE_CUGR, GPL_KEEP_OVERFLOW, MIN_PLACE_STEP_COEF, MAX_PLACE_STEP_COEF, RECOVER_POWER, EARLY_SIZING_CAP_RATIO, GRT_SEED, MAX_REPAIR_ANTENNAS_ITER_DRT An option the flow also reads to decide something else Let the flow read the edited arguments, as it now does for -allow_congestion
The SYNTH_* and ABC_* switches yosys commands, outside this pull request The same grammar, for yosys commands

Names over time

October ORFS names read by the scripts variables.yaml Options of the 11 commands With an ORFS name Without one Names one-per-knob would take
2020 62 – 53 5 48 2,336
2021 114 – 90 16 74 4,160
2022 135 – 108 17 91 5,024
2023 153 – 130 22 108 6,208
2024 164 117 163 43 120 7,424
2025 179 174 177 55 122 8,352
2026 210 249 209 58 151 9,856

Measured on the commit bazel-orfs pins (ORFS 0fcc4186, OpenROAD 9ddb60d9); the chart at the top plots the options columns. Since 2020 ORFS has added 148 names its scripts read, and the options without one still went from 48 to 151. Two commands have no *_ARGS at all, so 21 options of repair_timing (-effort, -max_passes, -policy, ...) and 5 of repair_design (-max_wire_length, -buffer_gain, ...) can't be set from a config today, under any name.

-useful_skew is one of them. With this change it's CLOCK_TREE_SYNTHESIS_ARGS = -useful_skew, and the same goes for every option OpenROAD adds next. A new option works in ORFS as soon as OpenROAD has it: no variables.yaml entry, no Tcl to pass it on, no name to agree on in review. ORFS changes only when OpenROAD adds a command, or an option that reads a file.

AutoTuner

Today. All 20 autotuner.json files under flow/designs tune CTS_CLUSTER_SIZE and CTS_CLUSTER_DIAMETER, and both are deprecated here. AutoTuner passes each tuned value on the make command line (parse_config() in tools/AutoTuner/src/autotuner/utils.py).

This pull request breaks nothing. A deprecated variable still works from the command line, ranked above PLATFORM_ARGS and below a design's CLOCK_TREE_SYNTHESIS_ARGS. AutoTuner keeps tuning CTS_CLUSTER_SIZE as before. The trials print a deprecation warning, and no design in ORFS sets -sink_clustering_size in CLOCK_TREE_SYNTHESIS_ARGS that would override the tuned value.

Proposal: tune an option, not a variable.

  • An autotuner.json key can name an option the way the grammar writes it:
    "CLOCK_TREE_SYNTHESIS_ARGS = -sink_clustering_size": {"type": "int", "minmax": [10, 40], "step": 1},
    "REPAIR_TIMING_ARGS = grt:-setup_margin": {"type": "float", "minmax": [0.0, 0.1], "step": 0}
    The text is exactly what the deprecation warning prints, replacement: in variables.yaml without the value.
  • One value per variable. parse_config() collects the keys for one variable into one value and appends it to the design's own, because a value on the command line replaces the design's whole value, and the later edit wins. The design's value comes from make print-CLOCK_TREE_SYNTHESIS_ARGS (flow/scripts/variables.mk), with the DESIGN_CONFIG, PLATFORM and FLOW_VARIANT AutoTuner already passes. It passes CLOCK_TREE_SYNTHESIS_ARGS="<design's edits> -sink_clustering_size 23". A value with spaces is quoted with quote_value() from args_grammar.bzl.
  • Old keys keep working. AutoTuner turns a deprecated key into its new form with the same replacement: field the warnings use, so the 20 autotuner.json files can move when convenient, or never.
  • Any option can be tuned. Any option of a command in the grammar's command table can be tuned, with no tunable: entry. OpenROAD rejects an option it doesn't know on the first trial. Scopes give AutoTuner knobs it can't reach today: a margin in one stage, or one call.
  • One special case retires. _PINS_DISTANCE, which AutoTuner turns into PLACE_PINS_ARGS="-min_distance <value>", already tunes one option through a *_ARGS variable. It becomes "PLACE_PINS_ARGS = -min_distance".

This is an AutoTuner change and isn't part of this pull request.

Tests

  • flow/test/test_args.py: parsing, every syntax error and its message, per-stage selection, layer merging and its order, scopes, on/off, deletion, flag/value conflicts, the round trip of format_args(parse_args(x)), the old meaning of DETAILED_ROUTE_ARGS, the deprecation warnings, and that variables.yaml agrees with the grammar's table of commands. The same module also loads in Bazel; the subset test fails on an f-string, a while loop and a recursive function added to it.
  • gcd on nangate45 and asap7 through route, on master and on this branch: every stage's .odb is bit-identical (18 of 18 hashes on each platform). Two command lines read differently with the same meaning: asap7's floorplan repair_timing passed -skip_last_gasp twice and now passes it once, and the first global_route lists -resistance_aware before GLOBAL_ROUTE_ARGS's default instead of after it. The only other difference in the logs is the new name in front of each call, and the deprecation warnings for the variables the platform and design set: RTLMP_MIN_CHANNEL_SIZE on both, and SETUP_REPAIR_PHASES, SKIP_LAST_GASP and ENABLE_RESISTANCE_AWARE on asap7, once per step that reads them.

This works for bazel-orfs too

bazel-orfs hands each stage only the variables that stage reads, so an edit for one stage doesn't rebuild the others. A stage: scope inside a variable would defeat that if bazel-orfs couldn't read it. Because the grammar is Starlark, bazel-orfs load()s it from ORFS, with no second implementation:

  • a syntax error in a BUILD file's arguments stops bazelisk build --nobuild at analysis, before any action is scheduled;
  • each stage's action is keyed only on the edits for that stage, so grt:-setup_margin 0.1 doesn't rebuild cts.

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a unified command argument editing mechanism for ORFS, replacing numerous deprecated individual variables with structured <COMMAND>_ARGS and PLATFORM_ARGS variables. This is supported by a new Python/Starlark parser (args_grammar.bzl) and runner script (args.py), along with updated documentation and flow scripts. The review identified two critical Starlark compatibility issues in args_grammar.bzl that will cause syntax and runtime errors when loaded by Bazel: the use of an anonymous lambda function and the list remove method. Both must be refactored to use Starlark-compliant alternatives.

Comment thread flow/scripts/args_grammar.bzl Resolved
Comment thread flow/scripts/args_grammar.bzl Resolved
@oharboe oharboe changed the title Consistent handling of arguments and options: one variable per OpenROAD command, deprecate 63 of 253 variables Consistent handling of arguments and options Oct 30, 2026
oharboe and others added 3 commits October 30, 2026 10:32
A <COMMAND>_ARGS variable holds edits to the arguments ORFS passes to an
OpenROAD command, written with the command's own options: -option value,
-flag, ~-option to delete, on/off to run the command or not, each scoped to
a stage (cts:) or one call (grt.wns:) when needed. PLATFORM_ARGS holds a
platform's defaults, each edit naming its command. Layers merge lowest
first: the flow's arguments, PLATFORM_ARGS, deprecated variables,
<COMMAND>_ARGS.

args_grammar.bzl is written in the subset of Python that is also Starlark,
so a Bazel build can load() it to check the grammar at analysis and give
each stage only its own edits. args.py runs it for the stage scripts and
checks every variable before a stage starts. test_args.py tests both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Every command a stage script runs with arguments a user could change, 16 in
all, takes them from command_args and command_runs in util.tcl, which call
args.py with $PYTHON_EXE so the merge has unit tests. Each call has a name,
stage.call, that an edit can be scoped to.

Nothing changes for a design that sets no new variable: the flow's
arguments, deprecated variables included, are the command line as before.
erase_non_stage_variables checks every <COMMAND>_ARGS and PLATFORM_ARGS
first, so a syntax error stops the stage before it runs.

variables.yaml gains a <COMMAND>_ARGS for each command that had none, and
PLATFORM_ARGS. The 63 variables that passed one option to one command, or
switched one step on or off, are marked deprecated with the line that
replaces them: they keep working, and a stage that reads one warns with
that line. CTS_ARGS and RTLMP_ARGS still replace the flow's arguments, and
so does DETAILED_ROUTE_ARGS while it holds a plain command line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
How to change the arguments ORFS passes to an OpenROAD command: the
grammar, the layers and how Make picks each one's value, use cases from a
design, a platform, a flow variant, the command line and the shell, the
name of every call, and the deprecated variables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe marked this pull request as ready for review October 30, 2026 09:41
@oharboe

oharboe commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

@luarss FYI

@openroad-ci

openroad-ci commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit 0185851 · Jenkins build #3 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 0 without a comparable baseline.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants