lowRISC / opentitan

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

[tools/fusesoc] Provide repo_top #6517

Open matutem opened 3 years ago

matutem commented 3 years ago

It is useful to refer to some files relative to the repo_top (i.e. opentitan) in *.core files, otherwise we end up with long sequences of ../'s, which can be confusing and can incur extra support.

Perhaps this feature is already supported in our .core files, but the fusesoc documentation is limited.

Assigning to Tom hoping he can properly re-assign this.

rswarbrick commented 3 years ago

@matutem: Can you give an example of a long chain of ../'s in code you're writing? I don't think that core files are supposed to refer to anything in a tree above them (and I didn't think that worked at all!). What problem are you trying to solve?

imphil commented 3 years ago

Core files should not contain relative paths above the file hierarchy, i.e. no paths starting with "../". The alternative is likely to place the core file somewhere else, but we'll need to have a look at a specific example.

matutem commented 3 years ago

Too many examples. Here is a nasty one: hw/ip/clkmgr/dv/env/clkmgr_env.core: ip_hjson: ../../../../top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson

$ find hw/ip -name "*core" -exec grep -H '../../' {} \; | wc 54 162 4234

The alternative of having all paths rooted at repo_top has strong adherents.

imphil commented 3 years ago

@matutem We really need to look at the bigger picture that brought us into this discussion. I'm pretty sure this specific enhancement proposal is solving your problem in the wrong way -- but I really need to know more about the problem to propose a better solution.

A couple of comments in the meanwhile:

Maybe we can pick up this discussion here again after we have finished the ipgen/ip templating discussion?

matutem commented 3 years ago

The autogen-related paths are the most egregious examples of parent paths. The vast majority of them are of this sort:

hw/ip/uart/dv/env/uart_env.core: ip_hjson: ../../data/uart.hjson hw/ip/uart/dv/sva/uart_sva.core: spec: ../../data/uart.hjson

These are present for perhaps all ip blocks. From what you say, it seems these dv-related core files should be placed under the ip directory, as in hw/ip/uart. Is that so?