Repository navigation
bazel: retire bazel-orfs and test/orfs - #11546
Conversation
OpenROAD CI relies on fast single-concern tests; flow-level integration testing lives in ORFS and bazel-orfs. Drop bazel-orfs, bazel-orfs-verilog, orfs, yosys, yosys-slang, rules_verilator and verilator from MODULE.bazel and delete test/orfs. rules_bison/rules_flex 0.3.1 and rules_m4 0.2.x do not load under Bazel 9; yosys masked this by raising them via MVS. Declare the versions that were actually resolved: rules_bison/rules_flex 0.4 and rules_m4 0.3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
rules_verilator and verilator are no longer dependencies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request completely removes the bazel-orfs integration, including its associated tests (such as mock-array and gcd), configurations, and documentation, from the OpenROAD repository. It cleans up the MODULE.bazel and .bazelrc files by removing dependencies like rules_verilator, verilator, and orfs, while upgrading rules_bison and rules_flex to version 0.4 and adding rules_m4. The remaining documentation is updated to guide developers on how to run ORFS flows externally. The reviewer suggested clarifying the instructions for running the whittle.py script in docs/user/Bazel.md by specifying the correct working directory and paths.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty @jhkim-pii Drinking my own cool-aid: drop heavy duty integration tests that nobody but the author will run in favour of extracted single concern working as intended tests. |
|
@gadfort FYI |
Chesterton's fence: 6977921 moved regression_rule_test's openroad to cfg=exec so that `bazelisk test -c opt ...` built OpenROAD once, because bazel-orfs flow tests in this repo also ran openroad as a build tool (exec). Those tests are gone (#11546); nothing here uses //:openroad as a tool any more. The fence now causes the duplicate build it was put up to avoid: cc_tests, hier_case_test's runner and the openroadpy extension that the Python regression tests load are all cfg=target, so a mixed test run compiled the OpenROAD libraries in both configurations. cquery of //src/gpl/test:ar01-tcl_test + //src/odb/test:test_block-py_test: before: //src/gpl:gpl in 2 configurations (exec + target) after: //src/gpl:gpl in 1 configuration (target) It also needed ba87b1b's workaround, since the sanitizer configs only instrument the target configuration: a second openroad_sanitized attr, a select() pair and //bazel:sanitizer_build. With the binary under test in cfg=target, all of that goes away and the sanitizer configs instrument the Tcl tests without a special case. POLA: `bazelisk build :openroad` followed by `bazelisk test` now reuses the same binary; drop the docs explaining why it did not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Removes
bazel-orfs,bazel-orfs-verilogandorfs(plusyosys,yosys-slang,rules_verilatorandverilator, which only existed for them) fromMODULE.bazel, and deletestest/orfs/(gcd and mock-array), along with the twoverilatordev-dep checks intest/BUILD.Side effect on version selection:
rules_bison/rules_flex0.3.1 andrules_m40.2.x do not load under Bazel 9 (CcInforemoved, missing//bin:m4). yosys had been masking this by raising them through MVS. This PR declares the versions master actually resolved:rules_bison/rules_flex0.4 andrules_m40.3.libffiresolves to 3.4.7.bcr.3 instead of bcr.4.Testing strategy
From now on, OpenROAD relies on fast, single-concern tests for its promises, not on flow-level integration tests. Each test asserts one intended behavior on small inputs and runs in seconds. Writing such tests used to be the expensive part; with AI-assisted development it is now cheap, so we write them when we need them rather than keep a heavyweight integration suite that approximates them. Full-flow integration testing stays in ORFS, where the flow lives. OpenROAD CI stays fast and lightweight.
What prompted this
Regenerating
rules-base.jsonused to be a local-only flow:bazelisk run //test/orfs/gcd:gcd_update, then commit the file. That is no longer possible.76be2a8eb, merged 2026-09-23) removed the rules files,checkMetadata.py,genRuleFile.pyandRULES_JSON. It replaced them withmake metadata-check, which postsmetadata.jsonto the QoR dashboard server and compares it with the dashboard's latest master build.9347dbeb).gcd_updatealready fails locally (cp: target '.../rules-base.json': Not a directory). bazel-orfs fixed that inbad2c699, after our pin551d4c72, and picking up the fix means bumping into the ORFS change above.Keeping
test/orfswould mean either staying pinned to a stale ORFS/bazel-orfs indefinitely, or taking a server-dependent QoR gate into OpenROAD CI. The first leavestest/orfsgoing stale. The second adds more CI brittleness to an already fragile multi-repo chain. Neither is worth it.About mock-array
mock-array has traditionally been a highly valuable regression test. Its regular N×N array of identical, abutted macros built from a hierarchical block is simple RTL, but it hits corners that production SoCs hit. It exposed:
test/orfs/mock-array/power.tcl#8857, Verilator failed with 17k warnings in asap7/mock-array hierarchical flow #8108, andwrite_macro_placementescaping (475eb39)It is, however, a poor fit for the ORFS Makefile flow. It needs bazel-orfs for:
orfs_runchecksThat is why it lived in OpenROAD, and why OpenROAD had to carry bazel-orfs.
If we find we need mock-array coverage again, we will resurrect mock-array in bazel-orfs, which already builds OpenROAD from source and owns the flow interface. We will then have AI generate specific single-concern OpenROAD test cases for whatever it catches.
What we get immediately
test/orfs/, many of them just regeneratingrules-*.jsonafter yosys and STA bumps.Bazel-breaking-changes.mdgoes.bazelisk test ...gets faster; the mock-array 8x8 flow was the slowest target.Lost coverage: the risk we accept
Flow-level breakage in general is still caught by ORFS's daily OpenROAD-bump PR (
github-actions-cron-update-OR.yml), within a day but after merge. The following is exercised bytest/orfsand by nothing else in OpenROAD tests or public ORFS CI. It is lost until mined back.Integration level
bazelisk test ...MockArray_{4x4,8x8}_base_testOPENROAD_HIERARCHICAL=1) and flat modeMockArray_*_{base,flat}_testcts/test/array*covers CTS alone)Specific concerns
.odb/.sdcat floorplan, place, cts, grt, route, finalgcd_idempotent_test_*(check_same.*)whittle.pyend-to-end with the realopenroad(etc/test_whittle.pyonly uses mocks)gcd_whittle_testread_spef -pathper instance, no unexpected warnings (#8297, #8551). The existing tolerance ofpin * not found(io_lsbOuts_0) needs a lookMockArray_*_path_groups_test,load_mock_array.tcl,parasitics.tclfind_timing_paths -from/-toandgroup_path -throughon those reloaded outputspath_groups.tclplace_macrowith escaped hierarchical names (ces_row\[r\].ces_col\[c\].ces), exact location and orientation, grid invariants (count, one orientation, aligned rows and columns, X abutment)place_mock_array.tcl,check_macro_layout.tclwrite_macro_placementwith escaped hierarchical nameswrite_macros.tcl-mirrored_pinsspread over several layers per directionelement-io.tclOPENROAD_HIERARCHICAL=1) block used as a macroElement_*_generate_abstractBLOCKS_grid_strategy.tcl,array_spacing_*Accepted without replacement
rules-*.json(they only record current output, and ORFS #4552 has retired rules files upstream)MockArray_8x8_*DETAILED_ROUTE_END_ITERATIONOPENROAD_HIERARCHICAL=1, explicit die area,ABC_CLOCK_PERIOD_IN_PSand an env-driven SDC (ORFS/yosys features;ABC_CLOCK_PERIOD_IN_PSis only used by gf12)gcd_testSYNTH_BLACKBOXES+ stitchedSYNTH_NETLIST_FILESin one design (each is covered separately by ORFSasap7/uartandasap7/aes)Everything else
test/orfstouched is covered by existing OpenROAD tests or ORFS designs:ppl/test/write_pin_placement*,ppl/test/annealing*,tap/test/abutting_macros_step*,tap/test/cut_rows_halo,cts/test/array*,gpl/testat density 0.8,dbSta/test/read_vcd, OpenSTAsearch_timing_model_readbackasap7/gcd,asap7/swerv_wrapper,asap7/aes-block,asap7/riscv32i-mock-sram,asap7/uart,asap7/aes,gf180/aes-hybridNot lost, because it never actually ran:
power*.tcl) and Verilator netlist checks were skipped (needs_vcd → continue,ELEMENT_POWER_TESTS = []).MACRO_PLACEMENT_TCL, so the MPL fixes above are already unguarded.path_groupsran under bare OpenSTA, not openroad, so reg2reg path group is missing with read_db -hier #8166 was not really covered.Recovering coverage: mine on breakage
test/orfsstays in git history at this PR's parent commit (6e51da9). When master breaks on one of the concerns above (in the ORFS bump PR or a user report), the fix PR adds a single-concern regression test mined from that history with an AI coding assistant. If mock-array as a whole turns out to be needed, it is resurrected in bazel-orfs (see above). Suggested prompt:Verification
bazelisk build //:openroadandbazelisk test ... --keep_goingpass locally (4437 pass; the 2 failures were the verilator dev-dep checks, removed in the second commit and re-verified with//test/...).bazelisk mod graphhas no orfs, yosys or verilator.