Fix subcommands#4164
Conversation
The rules_python bootstrap wrapper sets RUNFILES_DIR to its own runfiles tree. When run_command.py spawned OpenROAD, it inherited that variable and looked for Tcl init files in the Python runfiles instead of its own, causing "application-specific initialization failed" in Bazel builds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
`export OPENROAD_EXE := $(OPENROAD_EXE)` rebinds the variable to origin "file", which puts it on the UNSET_VARIABLES_NAMES list that UNSET_AND_MAKE clears before invoking a sub-make. In sub-make the variable is gone and the `?=` fallback resolves to the in-tree `tools/install/OpenROAD/bin/openroad` path that does not exist in a Bazel sandbox, so every stage invoked via UNSET_AND_MAKE (floorplan, place, route, final) fails with "openroad: No such file or directory". Using a bare `export` preserves the original origin (environment when bazel-orfs supplies it, file when the local default fills in) so the env value survives the UNSET_VARS step and sub-makes see the right binary. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Updates rules-base.json for: asap7/aes-block, asap7/riscv32i, asap7/mock-alu, gf180/aes-hybrid, gf180/uart-blocks, nangate45/tinyRocket, nangate45/ariane133, nangate45/ariane136, nangate45/bp_be_top, nangate45/bp_fe_top, nangate45/swerv_wrapper, sky130hd/microwatt.
- Bump bazel-orfs to d998bbff78c4557 - Add bazel-orfs-verilog transitive dependency - Update rules_python to 1.8.5, remove compatibility_level - Bump ORFS Docker image to 26Q1-816-gf40d2f346 - Add variables_yaml to orfs.default() - Add orfs_designs() repo rule for all 6 public-PDK platforms - Add commented-out OpenROAD-from-source instructions
Add orfs_design() targets and exports_files() to all design directories across asap7, gf180, ihp-sg13g2, nangate45, sky130hd, and sky130hs platforms, plus their source directories.
Add bazel-orfs.md with quick start guide, available targets, build results for 59 designs across 6 platforms, and status of blocked designs.
Add transitive dependency overrides required by the new bazel-orfs: openroad, qt-bazel, mock-klayout, glpk, and toolchains_llvm. The new bazel-orfs builds OpenROAD from source by default. 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>
Use local_path_override to point the openroad module at the tools/OpenROAD submodule rather than fetching from GitHub. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
The `include` filegroups at rtl/BUILD.bazel and rtl/register_interface/include/BUILD.bazel only globbed their own directory, so headers referenced via relative include paths from subdirectories (e.g. `common_cells/registers.svh`, `axi/typedef.svh`, `register_interface/typedef.svh`) were missing from the yosys sandbox. Explicitly list the .svh files that live in sibling/child packages so VERILOG_INCLUDE_DIRS resolution finds them. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates bazel-orfs into the OpenROAD-flow-scripts, enabling Bazel-based builds and tests for ORFS designs. The changes include significant updates to MODULE.bazel for dependency management, the addition of an LLVM toolchain, and the creation of BUILD.bazel files for numerous designs across multiple PDKs. Feedback highlights a potential issue where several designs appear to be missing clock constraints, leading to zeroed timing metrics. Additionally, suggestions were made to improve the robustness of PDK extension lookups, ensure consistency in file globbing patterns, and reduce redundancy in the module's dependency overrides.
| }, | ||
| "constraints__clocks__count": { | ||
| "value": 1, | ||
| "value": 0, |
There was a problem hiding this comment.
The constraints__clocks__count has dropped from 1 to 0, and multiple timing metrics (WS, TNS) in this file have been reset to 0.0. This strongly suggests that the design is being processed without any clock constraints (SDC file missing or not loaded). This pattern is also present in other designs like uart-blocks and ariane136. Please verify that the Bazel flow is correctly identifying and applying timing constraints, as a CPU design with zero clocks is likely incorrect.
| for ext in { | ||
| "asap7": ["cfg", "gds", "lef", "lib", "lib.gz", "lyt", "mk", "rules", "sdc", "sv", "tcl", "v"], | ||
| "gf180": ["cfg", "gds", "lef", "lib.gz", "lyt", "mk", "rules", "tcl", "v"], | ||
| "ihp-sg13g2": ["gds", "json", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | ||
| "nangate45": ["cfg", "gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | ||
| "sky130hd": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | ||
| "sky130hs": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | ||
| }[pdk] |
There was a problem hiding this comment.
The hardcoded dictionary lookup [pdk] is fragile. If a new PDK is added to the list at the bottom of this file but omitted from this dictionary, it will cause a KeyError during the loading phase. It is safer to use .get(pdk, []) or to consolidate the PDK metadata into a single structure that is iterated over. Note that the libs attribute on line 75 already uses .get().
| for ext in { | |
| "asap7": ["cfg", "gds", "lef", "lib", "lib.gz", "lyt", "mk", "rules", "sdc", "sv", "tcl", "v"], | |
| "gf180": ["cfg", "gds", "lef", "lib.gz", "lyt", "mk", "rules", "tcl", "v"], | |
| "ihp-sg13g2": ["gds", "json", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | |
| "nangate45": ["cfg", "gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | |
| "sky130hd": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | |
| "sky130hs": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | |
| }[pdk] | |
| for ext in { | |
| "asap7": ["cfg", "gds", "lef", "lib", "lib.gz", "lyt", "mk", "rules", "sdc", "sv", "tcl", "v"], | |
| "gf180": ["cfg", "gds", "lef", "lib.gz", "lyt", "mk", "rules", "tcl", "v"], | |
| "ihp-sg13g2": ["gds", "json", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | |
| "nangate45": ["cfg", "gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "v"], | |
| "sky130hd": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | |
| "sky130hs": ["gds", "lef", "lib", "lyt", "mk", "rules", "tcl", "tlef", "v"], | |
| }.get(pdk, []) |
|
|
||
| filegroup( | ||
| name = "verilog", | ||
| srcs = glob(include = ["*.v"]), |
There was a problem hiding this comment.
This glob is less inclusive than the one used in other design BUILD.bazel files (e.g., ariane). For consistency and to ensure SystemVerilog files are supported if added, consider using the more general pattern.
| srcs = glob(include = ["*.v"]), | |
| srcs = glob(include = ["*.v", "*.sv"], allow_empty = True), |
| git_override( | ||
| module_name = "bazel-orfs", | ||
| commit = "f8a4b694b37c8f5322323eba9a9ae37f9541ee17", | ||
| commit = "f8b36bf4a92c6e84317a70bab215bdcd2e25a26e", |
There was a problem hiding this comment.
The commit hash f8b36bf4a92c6e84317a70bab215bdcd2e25a26e is repeated in 7 different places across git_override and archive_override blocks (lines 13, 20, 28, 37, 38, 45, 46). This makes version updates highly manual and error-prone. While Bzlmod has limitations regarding variables in overrides, consider if some of these can be consolidated or if a script should be provided to automate the bump.
1 participant
No description provided.