Excluded ips

[benoitdenkinger]

Added EXCLUDED_IPS keyword argument to fetch IPs without certain dependencies with a simplified function to replace flatter_graph.

[Risto97]

Hello @benoitdenkinger ,
Can you explain a bit more this pull request.

When would you need to use EXCLUDE_IPS when calling get_ip_sources()?
How can you avoid doing the topological sort with flatten_graph by using the new function you added?

[benoitdenkinger]

When moving to implementation, dividing a design into hierarchical blocks is often needed. In this case, it could be nice to get all the sources from one IP excluding the sources of some lower levels hierarchical blocks in order to implement only the needed part.

Could you (re)explain me the reason for the need of the topological sorting?

I didn't see a way to do that easily with the current solution, so I wanted to try doing it another way to test if such a feature works and is interesting. What I did is to rely on the INTERFACE_LINK_LIBRARIES property to traverse the IPs and get the source files. This way, I can easily stopped at a IP/hierarchical level provided by the EXCLUDED_IPS parameter. It's probably not working for everything but this is for now only to test the EXCLUDED_IPS feature (actually I opened the PR mostly to test the gitlab-ci-approved labeling to trigger the gitlab CI from a fork PR).

[Risto97]

When moving to implementation, dividing a design into hierarchical blocks is often needed. In this case, it could be nice to get all the sources from one IP excluding the sources of some lower levels hierarchical blocks in order to implement only the needed part.

But shouldnt you do this filtering outside?
Something like

get_ip_links(ips ${IP_LIB})
list(FILTER ips EXCLUDE <REGEX>)

foreach(ip ${ips})
    synthesis_tool(${ip})
endforeach()

Not sure how this should work with xcelium, as if you exclude a certain IP block, it will not simulate as you would get missing module error?
I am probably missing the context of what you are trying to do.

Could you (re)explain me the reason for the need of the topological sorting?

For many languages like SystemVerilog or SystemRDL it is needed to compile source files in hierarchical order, but this is not easy to do when you have tree graph of dependencies. It is not easy to understand which IP needs to be compiled before another as the tree branches.
If you instead convert a tree graph into a flat dependecy list, then its easy to compile them in lowest-highest hierarchy order.
Topological sorting does that, most basic explanation here https://www.geeksforgeeks.org/dsa/topological-sorting/.

Forr tools like Verilator also, all the files need to be passed to the CLI tool as one command, and these files need to be passed in order, lowest hierarchy modules first, highest last.

Ok I see this is a draft pull request, but if you need help or you have some specific use case that we cannot cover yet with HWIP API, lets discuss to see what options there are

[benoitdenkinger]

EXCLUDE IPS FEATURE

USE CASE

About excluding IP outside the function, the problem is that I want to exclude an IP and all its dependencies. If I have:
IP1 -> IP2 -> IP3
Excluding IP2 after get_ip_links would still give me the IP3 sources. This is probably not a problem technically speaking because if only IP2 references IP3, then IP3 sources are just not used. But this is not very clean.

The context is that now I have a design I want to implement. Let's say I have:
TOP -> IP1
TOP -> IP2

When I simulate my design, I don't exclude anything. But when I implement it, I want to implement IP1 and IP2 on their own and then the TOP using the implemented IP1_NETLIST and IP2_NETLIST IPs. When I simulate the implemented design, now I have a new design:
TOP_NETLIST -> IP1_NETLIST
TOP_NETLIST -> IP2_NETLIST

This feature is to provide an easy way to retrieve IP sources to interface with implementation tools. I didn't think of other scenarios, but maybe there are.

FEATURE IMPLEMENTATION

Regarding the dependency search and the correct ordering, this is what my function ip_recursive_get_target_property does. It recursively looks for the INTERFACE_LINK_LIBRARIES property of a target/ip. By recursively searching and prepending the target/ip to a list, you get the dependencies in the correct order, as long as you linked them in the correct order. Then, I remove the duplicated entries keeping only the first occurrence to make sure the correct order is preserved.

The CI tests are passing and on my relatively simple design it is also working, but I don't know if this scales properly. Now I'm using it only with the HWIP and the INTERFACE_LINK_LIBRARIES, so if a dependency is not listed on this property I don't see it, but as long as the targets/dependencies are HWIPs, I think it works.

[Risto97]

About excluding IP outside the function, the problem is that I want to exclude an IP and all its dependencies. If I have:
IP1 -> IP2 -> IP3

But what if you have something above, if you exclude B should D be excluded or not? Because C also depends on it.

This kind of graph is what topological sorting solves, so from this there are 2 possible solutions:
D-C-B-A
D-B-C-A
But it always guarantees that the dependencies are in order low to high.

Excluding IP2 after get_ip_links would still give me the IP3 sources. This is probably not a problem technically speaking because if only IP2 references IP3, then IP3 sources are just not used. But this is not very clean.

But why don't you just use sources of IP1?
You can do that by using NO_DEPS argument in all get_ip_...() Functions, like so:

get_ip_sources(sources IP1 SYSTEMVERILOG NO_DEPS)

Note that in regular CMake there a concept of PRIVATE, PUBLIC, INTERFACE, but we don't have that in SoCMake as it's not very clear how to do it for HDL languages.

The CI tests are passing and on my relatively simple design it is also working, but I don't know if this scales properly

I was worried that this would break the tests, as it would not correctly do the topological sort so some tests would fail.
Like this one (https://github.com/HEP-SoC/SoCMake/blob/develop/tests/tests/ip_link/ip_link1.cmake), but I feel like maybe we don't have enough tests to ensure that the correct order is respected by hwip for all the edge cases.
I will need to check a bit more carefully your implementation.

[benoitdenkinger]

But what if you have something above, if you exclude B should D be excluded or not? Because C also depends on it.

If you exclude B, D will remain as long as it is linked by another IP. If not, it is excluded when B is.

But why don't you just use sources of IP1?
You can do that by using NO_DEPS argument in all get_ip_...() Functions, like so:

For very simple design yes, but for more complex ones it is often not good enough.

Note that in regular CMake there a concept of PRIVATE, PUBLIC, INTERFACE, but we don't have that in SoCMake as it's not very clear how to do it for HDL languages.

Yes I know, but sometimes we do link other kind of targets (e.g., C or C++ ones) and I was not sure if this change could break this. It shouldn't, as long as the functions of HWIP are used only for HDL sources. But for example the function get_ip_property is quite generic and I don't know if it was ever used to retrieve more standard cmake properties and where getting only the INTERFACE targets is not enough. We can also declare such use cases (if any) are not the intended way of using the HWIP functions.

I was worried that this would break the tests

Indeed, the test you mentioned (ip_link1) failed at the beginning because I was appending instead of prepending, but now it works.