[rtl] Fix prim_generic mapping in ibex_top#2443
Open
SamuelRiedel wants to merge 2 commits into
Open
Conversation
Explicitly include `lowrisc:prim_generic:all` in `ibex_top_tracing` instead of `ibex_top` to bind abstract `prim` dependencies to a specific implementation (generic, in this case). This is done to resolve the primitive mappings rather than to fix missing source dependencies. The dependencies are all listed as virtual `prim`s in `ibex_top`. Keeping this mapping isolated ensures that other wrappers can cleanly bind `ibex_top` to alternative backends, such as `prim_xilinx` for FPGA targets.
Adding a dependency on a specific implementation (`prim_generic`) of the virtual `prim` core for linter and Verilator targets forces FuseSoC to use the correct instances, thereby allowing us to lint or export filelists of `ibex_top` without a wrapper defining the virtual core's implementation.
Contributor
Author
|
I created a quick OT draft PR to verify that the CI passes also in OT with those changes: lowRISC/opentitan#30265 |
gautschimi
approved these changes
Jun 1, 2026
vogelpi
approved these changes
Jun 1, 2026
Contributor
vogelpi
left a comment
vogelpi
left a comment
There was a problem hiding this comment.
Thanks @SamuelRiedel for the PR and the detailed description, very nice work!
marnovandermaas
approved these changes
Jun 2, 2026
Contributor
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
This change looks good to me and better maps the expected behaviour.
rswarbrick
approved these changes
Jun 2, 2026
Contributor
rswarbrick
left a comment
rswarbrick
left a comment
There was a problem hiding this comment.
Looks good, and thanks for the careful explanation.
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
5 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.
This PR course-corrects #2397, where I mistakenly moved
prim_genericintoibex_top.core. While that change fixed an immediate FuseSoC failure, it broke the core-file's technology-independent abstraction layer.ibex_topis designed to rely strictly on virtual primitive dependencies (prim:*). It isn't meant to act as a final top-level module. Instead, wrappers likeibex_top_tracingare supposed to bind these abstract primitives to a concrete implementation, e.g., toprim_generic. This keepsibex_toptechnology-independent and an FPGA wrapper can useprim_xilinxfor example.I previously treated the
lowrisc:prim_generic:alldependency as a standard source file dependency. Becauseibex_topis the module actually implementing Ibex, it felt logical to put the dependency there instead of the wrapper to makeibex_top's dependency list complete on its own. However, in my current understanding, this dependency actually serves to define the instantiation mapping. Becauseibex_topalready declares its source dependencies virtually, hardcodingprim_genericthere stripped away the ability for wrappers to cleanly set their own primitives' implementation.The original mistake stemmed from our documentation, which instructs users to run linting or generate filelists targeting
ibex_topdirectly. Doing so bypasses the standard wrappers, leaving FuseSoC with no idea which implementation of the virtual prims to use, and to choose randomly, which then failed.To fix this cleanly now, this PR:
prim_genericdependency fromibex_top.coreagain and moves it back toibex_top_tracing.core.prim_genericonly for the linter and Verilator targets withinibex_top.This restores FPGA builds and ensures the documented linting/filelist workflows still work.