Skip to content

orfs_synth: filter non-synth stage arguments, avoid rebuilds #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oharboe merged 1 commit into from
Jan 6, 2026
Merged

orfs_synth: filter non-synth stage arguments, avoid rebuilds #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ orfs_synth(
name = "Mul_synth",
arguments = {
"SDC_FILE": "$(location :test/constraints-combinational.sdc)",
# Test locally, modify this, no re-synthesis should take place
"PLACE_DENSITY": "0.53",
},
data = [":test/constraints-combinational.sdc"],
module_top = "Mul",
Expand Down
91 changes: 63 additions & 28 deletions openroad.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ def _yosys_impl(ctx):
),
]

orfs_synth = rule(
_orfs_synth = rule(
implementation = _yosys_impl,
attrs = yosys_attrs() |
synth_attrs() |
Expand All @@ -1172,6 +1172,46 @@ orfs_synth = rule(
executable = True,
)

def _filter_stage_args(
stage,
**kwargs):
"""Filter and prepare the arguments for a specific stage."""

Comment on lines +1178 to +1179
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

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

The _filter_stage_args function lacks a docstring explaining its parameters and return value. While line 1178 provides a brief description, it should document the stage parameter and the **kwargs structure including expected keys like arguments, data, settings, etc.

Suggested change
"""Filter and prepare the arguments for a specific stage."""
"""
Filter and prepare the keyword arguments for a specific flow stage.
This helper normalizes and combines generic and per-stage arguments,
sources, and data before invoking a particular stage of the flow.
Args:
stage: String name of the stage to prepare arguments for (for example
"synth", "place", or "route"). This value is used to select
per-stage entries from the structures in ``**kwargs``.
**kwargs: Keyword arguments controlling how stage arguments are built.
Recognized keys include:
* ``arguments``: dict mapping generic argument names to values that
apply to all stages.
* ``data``: list of data targets or files that apply to all stages.
* ``settings``: dict of generic settings (similar structure to
``arguments``) that apply to all stages.
* ``extra_configs``: dict mapping stage name to a list of additional
configuration files or labels for that stage.
* ``sources``: dict or structure containing source files or labels
that are shared across stages.
* ``stage_arguments``: dict mapping stage name to a dict of
arguments specific to that stage.
* ``stage_sources``: dict mapping stage name to additional source
entries specific to that stage.
* ``stage_data``: dict mapping stage name to a list of data entries
(targets or files) specific to that stage.
Any additional keyword arguments are passed through unchanged to the
returned dictionary.
Returns:
A dict containing the prepared keyword arguments for ``stage`` with at
least the following keys:
* ``arguments``: merged generic and per-stage arguments.
* ``data``: combined generic and per-stage data.
* ``extra_configs``: list of extra configuration entries for the
given stage.
* ``settings``: merged generic and per-stage settings.
Any additional keys provided in ``**kwargs`` are also included in the
returned dict.
"""

Copilot uses AI. Check for mistakes.
def _args(**kwargs):
return kwargs
Comment on lines +1180 to +1181
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

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

The nested _args() helper function simply returns its arguments as a dictionary. This adds unnecessary indirection - you can directly return a dictionary literal in the outer function's return statement instead.

Copilot uses AI. Check for mistakes.

arguments = kwargs.pop("arguments", {})
data = kwargs.pop("data", [])
settings = kwargs.pop("settings", {})
extra_configs = kwargs.pop("extra_configs", {})
sources = kwargs.pop("sources", {})
stage_arguments = kwargs.pop("stage_arguments", {})
stage_sources = kwargs.pop("stage_sources", {})
stage_data = kwargs.pop("stage_data", {})

return _args(
arguments = get_stage_args(
stage,
arguments = arguments,
sources = sources,
stage_arguments = stage_arguments,
),
data = get_sources(stage, stage_sources, sources) +
stage_data.get(stage, []) + data,
extra_configs = extra_configs.get(stage, []),
settings = get_stage_args(
stage,
arguments = settings,
),
**kwargs
)

def orfs_synth(**kwargs):
return _orfs_synth(**_filter_stage_args(
"synth",
**kwargs
))

def _make_impl(
ctx,
stage,
Expand Down Expand Up @@ -1704,7 +1744,7 @@ UPDATE_RULES_IMPL = struct(stage = "update_rules", impl = orfs_update_rules)
TEST_STAGE_IMPL = struct(stage = "test", impl = orfs_test)

STAGE_IMPLS = [
struct(stage = "synth", impl = orfs_synth),
struct(stage = "synth", impl = _orfs_synth),
struct(stage = "floorplan", impl = orfs_floorplan),
struct(stage = "place", impl = orfs_place),
struct(stage = "cts", impl = orfs_cts),
Expand Down Expand Up @@ -2084,28 +2124,23 @@ def _orfs_pass(
synth_step = steps[0]
step_name = _step_name(name, variant, synth_step.stage)
step_names.append(step_name)
synth_step.impl(
synth_step.impl(**_filter_stage_args(
synth_step.stage,
name = step_name,
arguments = get_stage_args(
synth_step.stage,
stage_arguments,
arguments,
sources,
),
data = get_sources(synth_step.stage, stage_sources, sources) +
stage_data.get(synth_step.stage, []),
stage_arguments = stage_arguments,
arguments = arguments,
sources = sources,
deps = macros,
extra_configs = extra_configs.get(synth_step.stage, []),
module_top = top,
settings = get_stage_args(
synth_step.stage,
arguments = settings,
),
variant = variant,
verilog_files = verilog_files,
pdk = pdk,
stage_sources = stage_sources,
settings = settings,
extra_configs = extra_configs,
stage_data = stage_data,
**kwargs
)
))
orfs_deps(
name = "{}_deps".format(_step_name(name, variant, synth_step.stage)),
src = _step_name(name, variant, synth_step.stage),
Expand All @@ -2122,19 +2157,19 @@ def _orfs_pass(
)
step_name = _step_name(name, stage_variant, step.stage)
src = previous_stage.get(step.stage, _step_name(name, variant, prev.stage))
step.impl(
step.impl(**_filter_stage_args(
step.stage,
name = step_name,
stage_arguments = stage_arguments,
arguments = arguments,
sources = sources,
stage_sources = stage_sources,
settings = settings,
extra_configs = extra_configs,
src = src,
arguments = get_stage_args(step.stage, stage_arguments, arguments, sources),
data = get_sources(step.stage, stage_sources, sources) +
stage_data.get(step.stage, []) +
data,
extra_configs = extra_configs.get(step.stage, []),
variant = variant,
settings = get_stage_args(
step.stage,
arguments = settings,
),
stage_data = stage_data,
data = data,
**(
kwargs |
_kwargs(
Expand All @@ -2143,7 +2178,7 @@ def _orfs_pass(
) |
more_kwargs
)
)
))
if add_deps:
orfs_deps(
name = "{}_deps".format(step_name),
Expand Down