YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.31k stars 860 forks source link

Please add a flag to generate a simplified graphviz diagram. #4236

Open zapta opened 4 months ago

zapta commented 4 months ago

Feature Description

TL;DR, add to the yosys show command a flag to generate a simplified diagram.

Generating a graph with the show command to visualizing the design is a feature that helps to understand and describe the design. However, as the design grows, the diagram becomes complex and overwhelming as seen in the diagram below of a relatively small project. This feature request is to have an option to generate a simplified and easier to read diagram.

Below is an example that demonstrates this request. I took a graph that was generated by yosys and simplified it manually with these three changes:

  1. Eliminated all the diamond shapes of the wires, having the wires going directly from port to port.

  2. Eliminated wires of nets that connect more than N ports. In this example sys_clk and sys_reset.

  3. Changed the labels of the ports of nets that were eliminated in step 2 to indicate their connectivity. See in the example below how port label "sys_reset" changed to "(sys_reset)" and port label "reset" changed to "reset \n (sys_reset)" to indicate that they are connected to network "sys_reset".

Origianl graph (yosys generated): schema_original

Manually simplified graph: schema_simplified

dot_files.zip

mangelajo commented 4 months ago

Much cleaner output! :)

zapta commented 4 months ago

@nakengelhardt, @mmicko, any thoughts on this?

I would like to add to Apio a 'show' command that generates the diagram and launches a viewer, and having a simple diagram will make it easier to understand.

povik commented 4 months ago

You should be able to get halfway there with the selection syntax: show w:sys_clk w:sys_reset %% %n

zapta commented 4 months ago

Thanks @povik. We plan to add it as an apio command with good behavior for the default apio show such that having a flag to hide nets with fanout > N will allows us to use a fixed command without having to analyze the design.

povik commented 4 months ago

I am writing down some thoughts on this. Not sure I would want to see all those "simplified" changes being enabled by a single flag at once, but I suppose that doesn't matter much for your use case.

  • Eliminated all the diamond shapes of the wires, having the wires going directly from port to port.

Sounds like a good feature, a nice self-contained flag.

  • Eliminated wires of nets that connect more than N ports. In this example sys_clk and sys_reset.

Hmm. I wonder how could we compose this out of other Yosys features, since that seems preferable to a special-made option of show. We could have a command that creates a selection out of wires with high fanout, and use this named selection in the selection syntax passed to the show command to deselect those wires.

  • Changed the labels of the ports of nets that were eliminated in step 2 to indicate their connectivity. See in the example below how port label "sys_reset" changed to "(sys_reset)" and port label "reset" changed to "reset \n (sys_reset)" to indicate that they are connected to network "sys_reset".

Should this only happen when the port name matches the wire name?

zapta commented 4 months ago

Hi @povik, here is some formalization of this request. I hope it will address your points.

A net is a set of size(net)>0 end points (including output, input, pins, etc, but not including the net diamond itself). Each net is assigned (somehow, explained below) to one of the four net policies below and this controls how it is generated in the graph.

Net Policy 0 Net Policy1 Net Policy2 Net Policy3
Net diamond shape displayed X
Net arcs displayed X X
Net reflected in port labels X

Net diamond shape displayed, and Net arcs displayed are self explanatory. Net reflected in port labels means: if the net name equals the port name, than port label changes from port to (port) to indicates it's connected to a net of same name. Else, the port label changes from port to port\n(net) to indicates that it's connected to a network with a different name.

Currently all nets are assigned to policy 1. This request is to have different policy assignment that will improve the readability. For example, this policy assignment will be useful for apio:

Once you have this policy model implemented, you can support additional flags, such as explicit net names or regexs, to manually tune the policy assignment, but apio is likely to use only fixed flag values that are independent of the design.

Does it address your questions?

whitequark commented 4 months ago

I feel like, given the amount of constraints on the output, this could be better addressed in apio by parsing the Yosys JSON output.

zapta commented 4 months ago

Hi Catherine, my last post is not a constraint but a proposal. If you have other ideas on making the graph easier to follow please implement them, and they will benefit all yosis users.

On Mon, Feb 26, 2024 at 11:35 PM Catherine @.***> wrote:

I feel like, given the amount of constraints on the output, this could be better addressed in apio by parsing the Yosys JSON output.

— Reply to this email directly, view it on GitHub https://github.com/YosysHQ/yosys/issues/4236#issuecomment-1965948275, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQVMQM2BESM6JFWVLVR4TDYVWEFBAVCNFSM6AAAAABDYRXROCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRVHE2DQMRXGU . You are receiving this because you authored the thread.Message ID: @.***>

whitequark commented 4 months ago

What I mean is that if you would like to have a certain specific formatting for graphs in Apio it sounds great to me, but it's not clear that Apio users' needs and general Yosys users' needs are the same, or that it's a good way to spend Yosys maintainer time, or that it's a good idea to tie development cycles of the two together.

nakengelhardt commented 4 months ago

This sounds too use case specific to me. For example, except in the niche case where you are displaying a top level module that only instantiates other modules but does not contain much other logic, removing wire names will remove the only named objects in the diagram, leaving only the auto-generated private names that are unrelated to the source code. In the general case, this would make it very hard for the user to interpret the diagram at all.

The show code is also already quite complicated, and I don't know that the information needed to implement these features is available in the places the various nodes are emitted. Unless it's easy to integrate into the show code (in which case, file a PR for each individual option), I think it would make more sense for this to be a plugin maintained separately by the Apio folks.

zapta commented 4 months ago

@nakengelhardt, the simplification I described is just one proposal and if you have ideas for other simplifications I am sure they are just as good. As the concern of information loss due to removal of network names, this can be addressed by adding to net Policy 2 an 'X' Net reflected in port labels.

@whitequark, my intention was not to request an apio feature request but a general yosys/show improvement, so please consider it as such. One of the usages of the diagram is to provide an overview of the design and that use case would benefit from diagram simplification.

@whitequark, you mentioned yosys's json output.

  1. Is it considered a stable interface with other systems and tools (vs mostly internal)?
  2. Does it have an official documentation?
  3. Do you believe that it contains all the information necessary to create a diagram similar to the one show creates?
nakengelhardt commented 4 months ago

There are many things that can be connected to ports that are not whole wires with a name (even less a public name), so your proposal of replacing nets with names on the ports is also only suited to the simplest cases. That doesn't mean there can't be such an option available to use where it makes sense, we're happy to look at a PR, but if you are interested in writing such a PR, make sure it still handles all the other things that could be present in a module.

The JSON is a stable interface used to interface with other tools (nextpnr mainly). Its documentation is here. The JSON output contains all relevant information (IIRC the JSON output is complete enough to do a full round-trip of a design back to RTLIL without significant changes, just low-level things like autoindex and iterator order).

whitequark commented 4 months ago

One of the usages of the diagram is to provide an overview of the design and that use case would benefit from diagram simplification.

The show command is a tool designed by/for Yosys developers working on the compiler itself and intended for use as such; it happens to also be useful for others but that isn't its primary purpose. (I was rather surprised to learn that at least a few people consider it a core part of their workflow.) It's not really a converter of Yosys netlists to schematics or block diagrams (which sounds to me like what you want here), and adding features aimed towards that purpose must not compromise its primary purpose (including by making it ever-more-so complex to maintain).

zapta commented 4 months ago

Thank you everybody for the feedback. I will submit small incremental PRs and we can go from there. Keeping all the existing graph features for developers will be a prime directive, with flags for use cases of documentation and design exploration.