rdl: a number of changes to how generated register maps are handled - #524
Merged
Conversation
Aaron-Hartwig
force-pushed
the
aaron/cosmo-seq-adoc
branch
from
August 21, 2026 11:54
c9f710e to
4fc0bcb
Compare
A bsv_library that referenced its RDL package only as srcs = [":x_rdl[bsv]"] silently dropped the register-map docs. propagate_rdl_maps reads ctx.attrs.deps, and srcs entries are bare Artifacts with no providers attached, so RDLDocMaps never entered the graph and the bitstream came out with no maps/ directory. minibar and qsfp_x32 already listed the rdl target in both srcs and deps, so they were fine. gimlet_sequencer, gimlet_sdle_only and both sidecar mainboard revisions were not. Add the missing deps edge to the three bsv_library targets involved. Since the failure is silent and easy to repeat, bsv_library now checks for it: if a srcs artifact is owned by a target whose name ends in _rdl and that target is absent from deps, analysis fails with a message saying what to add. rdl.bzl already enforces the _rdl suffix on every rdl_file target, so the name check is as reliable as that convention. The migration guide was teaching the broken pattern, which is how this spread; fix the example and spell out why the rdl target belongs in both srcs and deps. Also dedup in propagate_rdl_maps. It accumulated a plain list, so artifacts repeated once per distinct dep path -- qsfp_x32 carried the same two artifacts ten times. Harmless downstream because collect_rdl_maps dedups by artifact, but unbounded in principle. maps/ copy actions per bitstream, before -> after: gimlet_sequencer, gimlet_sdle_only 0 -> 2 each minibar_controller 2 -> 4 mainboard rev_b, rev_cd 0 -> 4 each qsfp_x32 2 -> 2 8x ignition_target/psc (no RDL) 0 -> 0 VHDL bitstreams unchanged: cosmo_seq 23, grapefruit 16, cosmo_hp 8.
The maps/ directory next to a bitstream was not a declared output of anything. It only appeared because collect_rdl_maps' copy artifacts were passed as hidden inputs to place-and-route (BSV) or synthesis (yosys, vivado) -- a hack the code called out as "a bit sketchy". Two real consequences: CI could publish stale or missing maps. The bitstream jobs run on self-hosted runners with clean: false to preserve buck-out, and upload dirname($OUTPUT), i.e. the whole target directory. On a warm cache the place-and-route action is a hit, so the copies need never re-materialize and whatever was in maps/ from a previous run gets uploaded. Editing a register description forced a full place-and-route, because hidden inputs are part of the action digest. On the vivado path it forced re-synthesis and everything after it. Return the copies as DefaultInfo other_outputs plus a [maps] sub-target instead, so materialization is a property of building the target. In yosys.bzl and vivado.bzl collect_rdl_maps is called from the synthesis helper rather than the rule impl, so the maps ride out on that step's own DefaultInfo and get re-exported by the bitstream rule; calling collect_rdl_maps a second time would double-declare the outputs. Only maps come off the hidden lists. in_json_file stays: it is what makes synthesis re-run when source file *contents* change, and that was a tricky bug to find the first time. Verified: a cold build in a throwaway isolation dir (0% cache hits) materializes maps/; aquery confirms the nextpnr and icepack actions no longer take maps as inputs; rebuilding sidecar mainboard after its maps gained a file completed in 2 commands with place-and-route cached, where before it would have forced a fresh ECP5 P&R. buck2 build --show-output still prints the bitstream as field 2, which .github/workflows/build.yml depends on.
These four rdl targets emitted an .adoc register map under cobble but lost it in the move to buck2; their BUILD files still ask for one. Now that rdl_file accepts .adoc and _MAPS_DIR_EXTENSIONS carries it, they land in maps/ alongside the html and json. Two names could not be carried over verbatim. rdl.bzl requires the output basename to match the .rdl stem, so minibar_regs.adoc becomes minibar_controller.adoc and sidecar_qsfp_x32_controller_regs.adoc becomes qsfp_x32_controller.adoc. gimlet/sequencer is deliberately not included -- it never requested an .adoc under cobble either. All four are flat single address maps, so this is the first real exercise of the MapExporter (flatten_names = False) branch of regmap_adoc.jinja2; cosmo_seq_top is a map of maps and does not cover it. All four render clean under asciidoctor --failure-level=WARN with no duplicate section names, and minibar's two nested ignition_controller addrmaps at 0x100/0x200 flatten to IGNITION_CONTROLLER0_* as expected.
Aaron-Hartwig
force-pushed
the
aaron/cosmo-seq-adoc
branch
from
August 21, 2026 20:37
7192626 to
9e38a47
Compare
Aaron-Hartwig
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A number of commits here:
hidden)