lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.51k stars 745 forks source link

[primgen/fusesoc] Generated primitive cores cannot depend on other generated primitive cores #6604

Open msfschaffner opened 3 years ago

msfschaffner commented 3 years ago

Our current primgen mechanism can't reliably handle generated primitives that rely on other generated primitives.

The problem seems to arise due to the fact that primgen is currently only called for primitives that are discovered in the dependency graph BEFORE executing all generators.

Hence, if a core file of a technology-specific primitive implementation references another core file of a generated primitive not used anywhere else, that second primitive will not be properly generated with primgen, as the dependency only becomes evident AFTER the primgen generator pass.

Example:

In this case, fusesoc calls primgen only for the primitives directly used in the DUT, but not on lowrisc:prim:clock_gating, which leads to a missing abstract primitive wrapper for lowrisc:prim:clock_gating even tough the dependency has been specified in the lowrisc:prim_techXY:otp core.

A solution to avoid this problem is to define dependencies on other generated primitives in the abstract wrapper core file (see e.g.: #6606) in order to enforce generation of the corresponding abstract primitive wrappers. Not sure there is a better solution, since it is a sort of "Chicken/Egg" problem that requires multiple iterations until convergence.

PS: I am not sure this is a duplicate. If an issue for this already exists, please link it here and close this one.

imphil commented 3 years ago

That's a known limitation, tracked upstream at https://github.com/olofk/fusesoc/issues/404

It's non-trivial to solve properly and will likely need a rethinking of the way we generate primitives right now (if we don't want to go for the "iterate dependency resolution until stable" approach, which is slow and error-prone).

It's on my list of things to look at, but for now, the workaround is simply to add the dependencies one level up, as you did in #6606.

msfschaffner commented 3 years ago

Ok good - thanks for linking that upstream bug. I just wanted to make sure we track this somewhere in our OT issues as well.