Skip to content

feat(variables): FLOW_INPUT_VARIANT, read inputs from another variant - #4475

Open
oharboe wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:feature/flow-input-variant
Open

feat(variables): FLOW_INPUT_VARIANT, read inputs from another variant#4475
oharboe wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:feature/flow-input-variant

Conversation

@oharboe

@oharboe oharboe commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

FLOW_VARIANT names the directory a run writes to. It also names the directory a run reads from, which means a variant that wants to reuse an upstream variant's results has to physically copy them into its own directory first. Sharing an expensive front end across experiments — one synthesis, one floorplan, many routing variants — is a cp -r per fork today.

Split the two. FLOW_INPUT_VARIANT names the variant a run reads from and gives INPUT_RESULTS_DIR, INPUT_LOG_DIR, INPUT_REPORTS_DIR and INPUT_OBJECTS_DIR alongside the existing output directories. It defaults to FLOW_VARIANT, so every INPUT_*_DIR expands to a string identical to its *_DIR counterpart and an unchanged run is unchanged.

This is the ORFS side of The-OpenROAD-Project/bazel-orfs#846, extracted as a single concern patch.

FLOW_VARIANT names the directory a run writes to. It also names the
directory a run reads from, which means a variant that wants to reuse
an upstream variant's results has to physically copy them into its own
directory first. Sharing an expensive front end across experiments —
one synthesis, one floorplan, many routing variants — is a `cp -r` per
fork today.

Split the two. FLOW_INPUT_VARIANT names the variant a run reads from
and gives INPUT_RESULTS_DIR, INPUT_LOG_DIR, INPUT_REPORTS_DIR and
INPUT_OBJECTS_DIR alongside the existing output directories. It
defaults to FLOW_VARIANT, so every INPUT_*_DIR expands to a string
identical to its *_DIR counterpart and an unchanged run is unchanged.

Reads resolve against a search path: RESULTS_DIR first,
INPUT_RESULTS_DIR second. RESULTS_DIR has to come first because a
stage runs several steps in one process — do-place writes
3_1_place_gp_skip_io.odb and the next step reads it back — and a file
this run just wrote must win over the upstream variant's copy of it.
orfs_input_path and orfs_input_glob in util.tcl are the search; every
write still goes straight to RESULTS_DIR.

load_design is the chokepoint: all 18 stage scripts hand it a bare
basename, so routing it through orfs_input_path covers the whole flow.
The remaining reads are the AUTO_MEMORIES globs, find_sdc_file, the
abstract's .spef, yosys_load's netlist, and genMetrics' log/report/
result directories.

find_sdc_file's candidate ordering now compares basenames rather than
full paths. With one directory the two are equivalent, since every
candidate shares a prefix; with two, the directory prefix would
otherwise decide which .sdc wins.

The Makefile's prerequisite graph is deliberately untouched: it still
names $(RESULTS_DIR). A forked variant is driven through the do-
targets, which is what the flow.sh path does anyway.

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

@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 the FLOW_INPUT_VARIANT feature, allowing a run to read inputs from an upstream variant directory while writing outputs to its own variant directory. This enables sharing expensive front-end stages across multiple downstream experiments without copying files. The changes include updating path resolution in Tcl scripts to search both current and input results directories, updating makefiles, and adding documentation. The review feedback highlights a critical issue in flow/util/utils.mk where using INPUT_*_DIR exclusively for metadata generation and autotuning will incorrectly read upstream metrics instead of the current variant's metrics. Additionally, the reviewer noted an inconsistency in the documentation's Mermaid diagram, which incorrectly depicts the main flow starting at global routing instead of re-running from the floorplan stage to apply new IO constraints.

Comment thread flow/util/utils.mk
Comment thread flow/util/utils.mk
Comment thread docs/user/FlowVariables.md
@oharboe
oharboe force-pushed the feature/flow-input-variant branch 4 times, most recently from 6a813d0 to 369f779 Compare August 26, 2026 14:18
@oharboe
oharboe requested a review from maliberty August 26, 2026 14:23
…ics limits

FLOW_VARIANT and genMetrics.py works only within a single variant. Implement a hard stop in genMetrics.py if FLOW_INPUT_VARIANT is different from FLOW_VARIANT, as cross-variant log/report paths are not supported.

Fix GUI reading netlist in forked variants by resolving V_FILE through orfs_input_path.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe force-pushed the feature/flow-input-variant branch from 369f779 to c27ec1e Compare August 26, 2026 14:23
@openroad-ci

openroad-ci commented Aug 26, 2026

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch. Advisory — results are log-only and do not affect build status. The authoritative QoR gate remains the local rules-file check.

Commit 6a5f462 · Jenkins build #9 · Baseline: build

63 design(s) checked — 0 with regression(s), 1 without a comparable baseline.
Design Variant Verdict local vs inline passed / failed / missing
asap7/aes base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/aes-block base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/aes-mbff base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/aes_lvt base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/coralnpu ➖ inconclusive ➖ N/A no metrics produced — flow may have failed before writing metadata.json
asap7/cva6 base ✅ pass ✅ MATCH 12 / 0 / 0
asap7/ethmac base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/ethmac_lvt base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/gcd-ccs base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/jpeg_lvt base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/mock-alu base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/mock-cpu base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/riscv32i base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/riscv32i-mock-sram base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/swerv_wrapper base ✅ pass ✅ MATCH 25 / 0 / 0
asap7/uart base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/aes base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/aes-hybrid base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/riscv32i base ✅ pass ✅ MATCH 25 / 0 / 0
gf180/uart-blocks base ✅ pass ✅ MATCH 25 / 0 / 0
gt2n/aes base ✅ pass ✅ MATCH 25 / 0 / 0
gt2n/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
gt2n/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/aes base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/i2c-gpio-expander base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/riscv32i base ✅ pass ✅ MATCH 25 / 0 / 0
ihp-sg13g2/spi base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/aes base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/ariane133 base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/ariane136 base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/black_parrot base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/bp_be_top base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/bp_fe_top base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/bp_multi_top base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/cva6 base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/dynamic_node base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/mempool_group base ✅ pass ✅ MATCH 12 / 0 / 0
nangate45/swerv base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/swerv_wrapper base ✅ pass ✅ MATCH 25 / 0 / 0
nangate45/tinyRocket base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/aes base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/chameleon base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/microwatt base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hd/riscv32i base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hs/aes base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hs/gcd base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hs/ibex base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hs/jpeg base ✅ pass ✅ MATCH 25 / 0 / 0
sky130hs/riscv32i base ✅ pass ✅ MATCH 25 / 0 / 0

oharboe and others added 3 commits August 27, 2026 13:23
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…AILURE

The FLOW_INPUT_VARIANT note about genMetrics.py was accidentally
inserted mid-sentence into the GENERATE_ARTIFACTS_ON_FAILURE
description; the note already lives in FLOW_INPUT_VARIANT's own
description. Regenerated variables.json and FlowVariables.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…variant

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

oharboe commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty coralnpu failure?

@maliberty

Copy link
Copy Markdown
Member

This looks rather error prone given the number of places you had to touch. Would it be easier to just have a copy (or symlink) target?

@oharboe

oharboe commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

This looks rather error prone given the number of places you had to touch. Would it be easier to just have a copy (or symlink) target?

That creates an iffy stateful mess in bazel-orfs I am trying to get rid of.

@oharboe

oharboe commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

This looks rather error prone given the number of places you had to touch. Would it be easier to just have a copy (or symlink) target?

We've been bitten in the past by the code to infer which .sdc file to load based on the .odb file. Why the dichotomy of .odb and .sdc? There are a million bits and bobs in .odb for all sorts of modules, what makes .sdc special? Perhaps .odb could subsume the .sdc like it does so many other things. The only thing we can do with the freedom we have today is to load the wrong .sdc data.

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.

3 participants