Skip to content

Update yosys submodule#42

Closed
github-actions[bot] wants to merge 13 commits into
bazel-orfs-updatefrom
update-yosys
Closed

Update yosys submodule#42
github-actions[bot] wants to merge 13 commits into
bazel-orfs-updatefrom
update-yosys

Conversation

@github-actions
Copy link
Copy Markdown

Automated changes by create-pull-request GitHub action

@oharboe oharboe force-pushed the bazel-orfs-update branch from 2f82049 to c44b22d Compare May 14, 2026 16:29
@github-actions github-actions Bot force-pushed the update-yosys branch 3 times, most recently from bb71402 to eeabe83 Compare May 14, 2026 16:54
maliberty and others added 2 commits May 14, 2026 19:16
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
oharboe and others added 2 commits May 15, 2026 12:48
flow/README.md "Triaging a failing _test" → "Yosys-environment false
positive" already calls out that bazel-built yosys and make-built
yosys can produce different 1_2_yosys.v for the same RTL, drifting
QoR past rules-base.json thresholds even though OpenROAD itself is
bit-deterministic.  Today the only way to spot the drift is to
re-run `@bazel-orfs//:make-yosys-netlist`; if a designer has a
freshly-failing _test in front of them, they have no way to see
"the yosys netlist changed" vs "a real regression".

Persist a fingerprint in rules-base.json instead so the next _test
just prints it:

  - genMetrics.py: emit `synth__canonical_netlist__hash` (SHA-1 of
    1_1_yosys_canonicalize.rtlil, the canonical RTLIL pre-ABC) and
    `synth__netlist__hash` (SHA-1 of 1_2_yosys.v, post-ABC).  Having
    both lets the user see whether divergence is in the front-end
    flow or downstream of ABC.

  - genRuleFile.py: new `literal` mode passes the metric value
    through verbatim (no padding / rounding / float coercion).  The
    two hash fields use it with `level: warning` + `compare: ==`,
    so `_update` writes them into rules-base.json and `_test`
    treats a hash mismatch as a diagnostic, not a failure.

  - checkMetadata.py: a warning-level mismatch previously printed
    "[WARN] field pass test: a == b" — confusing when a != b yet
    "pass" implied a match.  Say "differs" instead so the hash
    mismatch reads naturally without changing the no-error contract
    (still doesn't increment ERRORS).

rules-base.json files are unchanged in this commit; the next
`_update` per design adds the two hash fields automatically, and
existing _test runs are unaffected until that happens.

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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
genElapsedTime.py used to pick exactly one of .odb / .rtlil / .v as
"the result file" for a log and hash that.  Synth produces both
1_2_yosys.v and 1_2_yosys.sdc; OpenROAD stages produce both an
.odb and a .sdc.  Folding all of them into one column means a
divergent .sdc (an SDC-generator change) and a divergent .odb (a
real flow change) look identical in the elapsed-time triage table.

Replace `get_hash` with `get_hashes` that returns every result-file
extension that exists, and emit one row per (stage, extension)
with the elapsed time and peak memory on the stage's first row
only.  Column order is `.v / .rtlil / .odb / .sdc` so the primary
data artifact comes first and the constraint file last.

Added a regression test (`test_emits_one_row_per_result_extension`)
to lock the two-row-per-stage behaviour in; existing tests cover
the no-result-file fallback (single row with ext="" and hash=N/A)
since the table now has an Ext column.

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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@github-actions github-actions Bot force-pushed the update-yosys branch 3 times, most recently from 649ad00 to 7fd3e25 Compare May 15, 2026 13:25
…t-staging/secure-fix-variables

fix make issue missing variables
@github-actions github-actions Bot force-pushed the update-yosys branch 2 times, most recently from 6fcff6d to 4d99545 Compare May 15, 2026 15:37
- genElapsedTime.py: hash .def / .spef / .gds in addition to .v /
  .rtlil / .odb / .sdc so the elapsed-time table covers all primary
  flow artifacts, not just the synth + odb subset.
- genMetrics.py: drop the flow/README.md "Triaging a failing _test"
  pointer next to the netlist-hash metrics; the triage section will
  land in a later PR.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@github-actions github-actions Bot force-pushed the update-yosys branch 3 times, most recently from deb77ba to 970c76a Compare May 16, 2026 13:13
@github-actions github-actions Bot force-pushed the update-yosys branch 9 times, most recently from ef0a4c2 to 224da8f Compare May 17, 2026 18:37
@github-actions github-actions Bot force-pushed the update-yosys branch 6 times, most recently from b654bdb to 5f54eba Compare May 17, 2026 22:09
oharboe added 3 commits May 18, 2026 06:37
os.path.realpath() follows symlinks.  Inside a Bazel sandbox, the
bazel-out/ tree is symlinked from the sandbox back to the bare
execroot; realpath resolves through that symlink and yields the
bare-execroot path, not the sandbox path.

That alone doesn't matter for os.path.relpath(a, b) when both
operands are realpath'd from the same sandbox — the relative
result is unchanged.  But the resulting <lef-files> path in the
generated klayout.lyt is later resolved by klayout against the LYT
file's location.  Klayout opens the LYT (also through a symlink),
resolves through to the bare execroot, and then looks for the
sibling klayout_tech.lef at the bare-execroot path — where the
in-flight file does not exist during action execution (only the
sandbox copy does), so def2stream fails with errno=2.

Fix: don't realpath.  os.path.relpath produces the correct relative
path from sandbox-relative inputs directly.  Map files keep their
absolute form via abspath for the unchanged-under-non-sandbox case.

Surfaced for the first time on bazelisk build
//flow/designs/sky130hd/gcd:gcd_final_blender_html — the
orfs_blender macro is the first bazel-side consumer of orfs_gds
(klayout def2stream), so the latent path bug had no prior trigger.

Mirrors bazel-orfs's patches/0037-fix-generate_klayout_tech-drop-realpath.patch
into ORFS directly so bazel-orfs no longer needs to carry it.

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
The previous switch to relpath (commit 01652ca) cleaned up the
content but did not actually fix def2stream's errno=2 under a Bazel
sandbox.  Investigation via strace shows klayout's
Layout.read(def, layout_options) opens the input DEF (a sandbox
symlink to the bare execroot, staged from orfs_final), realpath's
it to the bare-execroot results dir, and then resolves <lef-files>
relative paths against THAT dir.  Sibling intermediates like
objects/klayout_tech.lef only exist in the per-action sandbox, not
at the bare execroot, so the lookup lands at
.../execroot/_main/bazel-out/.../klayout_tech.lef and fails with
errno=2.

Declaring klayout_tech.lef as a Bazel output of orfs_gds doesn't
help -- Bazel only materialises declared outputs at the bare
execroot at action completion, not while the action is still
running and klayout is reading them.

Plain abspath (NOT realpath -- realpath would chase Bazel
input-file symlinks back out to the bare execroot for any input
already at the bare execroot) writes the in-sandbox absolute LEF
path into the LYT.  KLayout opens that absolute path directly with
no relative-path resolution involved, so the sibling lookup never
escapes the sandbox.

Cost: the LYT's content now embeds the per-invocation sandbox
number, so the orfs_gds action cannot be cross-action cached.
That's an acceptable trade-off for correctness while we wait for
a klayout-level fix or a Bazel feature that lets us write a
stable bare-execroot path from inside the action.

Confirmed end-to-end:
  bazelisk build //flow/designs/sky130hd/gcd:gcd_final_blender_html
produces 6_final.gds and the blender_html sh_binary, with both
the sandbox sandbox-id N going forward and after fresh action
re-runs (verified by deleting outputs and rebuilding).

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Earlier commit a1552a8 switched generate_klayout_tech.py to write
absolute LEF paths into the LYT (bypasses klayout's relative-resolution
escape to the bare-execroot under a Bazel sandbox).  The unit test
test_basic_generation still asserted the old relpath form and failed
in CI:

  AssertionError: '../tech.lef' not found in
    '...<lef-files>/tmp/tmpwlgkq96a/tech.lef</lef-files>...'

Update the assertion to expect os.path.abspath(lef_path).

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@github-actions github-actions Bot force-pushed the update-yosys branch 5 times, most recently from bffbc3d to 5bb1b00 Compare May 18, 2026 05:04
…enROAD-Project#4234

Two follow-ups from gemini's review of The-OpenROAD-Project#4234:

  * The map-file test in test_with_map_files still asserted
    `os.path.realpath(map_path)`, inconsistent with the LEF test's
    `os.path.abspath` switch.  On platforms whose tmpdir contains
    symlinks (macOS /var -> /private/var) the test would fail.
    Switch to `os.path.abspath` to match the LEF case.

  * `reference_dir` and `use_relative_paths` are now both unused.
    Update the function docstring to say so, default both to
    `None` / `False` so callers can drop them, and demote the CLI
    flags to `required=False` so external scripts that don't pass
    them still work.  flow/Makefile still passes `--reference-dir`
    so we keep accepting the flag.

Signed-off-by: Øyvind Harboe <oyvind@ascenium.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…ect#4234

Three follow-ups from gemini's second review pass:

  * Restore the original positional argument order
    (template_lyt, output_lyt, lef_files, reference_dir, map_files,
     use_relative_paths) so Python callers that pass the args
    positionally still work.  Prior fix reshuffled `map_files` ahead
    of `reference_dir`, breaking positional compatibility.

  * Re-order the Args block in the docstring to match the new
    signature order.

  * `map_files` is now `None`-defaulted (to keep the positional-order
    fix above), so iterate via `map_files or []` rather than dereferencing
    `None` directly.

Signed-off-by: Øyvind Harboe <oyvind@ascenium.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@oharboe oharboe deleted the branch bazel-orfs-update May 18, 2026 05:47
@oharboe oharboe closed this May 18, 2026
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