Flat Routing Visualization

[SamuelHo10]

Several changes were made for flat routing visualization.

Firstly, I modified the draw intra-logic block code so that the margins between blocks are absolute and not based on relative size. The intra-logic blocks are also now drawn to be not long and skinny, and instead must have a reasonable width-to-height ratio. Furthermore, the intra-logic blocks of maximum depth were previously not drawn. That has also been fixed.

To get the pin locations of intracluster RRNodeId, I made helper functions to convert RRNodeId into cluster_blk_id and pb_graph_pin. I also separated the code which draws edges and pins, primarily for readability and also to avoid repetition. The loops with clusternets were replaced with atomnets when flat routing is enabled. Lastly, for testing and debugging purposes, the ability to click on intracluster pins to highlight the net was added.

[amin1377]

@SamuelHo10:

Thank you for your changes! Could you please share some screenshots showing how the intra-logic connections are drawn? Also, it would be helpful if you could include a screenshot for the case when the flat-router is not enabled, just to ensure the default flow remains intact.

I’m currently testing this PR locally, but having the screenshots here would be useful for future reference.

[SamuelHo10]

For Future Reference,

Flat Routing Enabled:

Flat Routing Disabled

[vaughnbetz]

Looks good; some changes requested though.
As we discussed in the meeting, you should:

• fix the arrow direction (done I think)
• test more architecture files.
• Stratix IV: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vtr_flow/arch/titan/stratixiv_arch.timing.xml (need to use with a titan benchmark; neuron is the smallest normal one, but titan_other has some smaller ones still)
• Stratix 10: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vtr_flow/arch/titan/stratix10_arch.timing.xml (need to use a Stratix 10 mapped Titan design or a Titanium design)
• Artix 7: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vtr_flow/arch/xilinx/7series_BRAM_DSP_carry.xml
• Z1000: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vtr_flow/arch/zeroasic/z1000/z1000.xml
• The arch we use with Koios (hopefully the developer guide points you at the right task)

[soheilshahrouz]

@SamuelHo10
Thansk for this PR.

We recently added a coding style guide to the documentation. Please read the guide and make sure that the code is consistent with the style rules.

[amin1377]

Thanks, Samuel!

[SamuelHo10]

Thanks for the code reviews @amin1377 and @vaughnbetz. I've tried my best to improve the code documentation and clarity based on your suggestions. I still have yet to test the other architectures, so I will try to get that done as soon as possible.

@AlexandreSinger At our last meeting, you suggested revising the naming of the internal blocks. Previously, the format was pb_type->name [pb->name]. I've updated the code to align more closely with the hierarchical_type_name() function in vpr_types.h, so it now prints as pb_type->name[placement_index][mode_name].
Would this naming scheme be more helpful for your use case?

[AlexandreSinger]

Hi @SamuelHo10 this is certainly a step up! I appreciate being able to see the placement index.

I think including the mode is a good idea, but I recommend using something other than square braces for the mode name. Something like "fle:n2_lut5[6]" (I personally like the placement index being at the end). I am not 100% sold on the ":" but I think using square braces can be a bit confusing. Maybe a pipe would work "|", just something to differentiate the mode from the pb_type name and the placement index.

[vaughnbetz]

I agree; I think using : would be better than [] to separate mode names and instance names from the hierarchical name of the sub-block types.

[vaughnbetz]

Thanks for all the updates ... they look good @SamuelHo10 !
I have a few more requests. After addressing those and checking the listed archs, this is good to merge from my perspective.

[vaughnbetz]

OK, I'm fine with leaving this alone then.

[SamuelHo10]

@vaughnbetz Three more architectures have been successfully tested with flat routing visualization.

Artix 7:

Coffe_22nm:

Stratix IV:

I fixed an issue where the number of child blocks was miscalculated by creating a get_num_child_blocks helper function which loops through num_pb_type_children and adds up num_pb. However, I have still yet to fix another issue I found where intra-cluster pins are connecting to the wrong side of the CLB (see highlighted net in the Artix 7 image). I’ll likely need another helper function to determine the correct side for each inter-cluster pin.

Two architectures I've tested failed to route.

Stratix 10 failed with the error:
## Computing tile lookahead
terminate called after throwing an instance of 'std::out_of_range'
what(): unordered_map::at
./vtr.sh: line 12: 1969288 Aborted (core dumped) $VTR_ROOT/vpr/vpr $VTR_ROOT/vtr_flow/arch/titan/stratix10_arch.timing.xml $VTR_ROOT/vtr_flow/benchmarks/blif/2/bw.blif --flat_routing on --route_chan_width 100 -j12

Z1000 didn’t produce any errors, but it consistently failed to route regardless of the channel width setting. @soheilshahrouz suggested using the flags from vtr_flow/tasks/regression_tests/vtr_reg_nightly_test7/z1000_qor/config/config.txt. Upon reviewing the config file, I found some issues: --sweep_dangling_primary_ios off is listed twice, and -allow_dangling_combinational_nodes on is missing a hyphen.

script_params=-starting_stage vpr -track_memory_usage --route_chan_width 100 --device z1000 --clock_modeling route --constant_net_method route --const_gen_inference none --sweep_dangling_primary_ios off --sweep_dangling_primary_ios off --sweep_dangling_nets off -allow_dangling_combinational_nodes on --sweep_constant_primary_outputs off --sweep_dangling_blocks off

[vaughnbetz]

Thanks @SamuelHo10 !
This looks ready to merge once you fix that inter cluster pin issue.

Can you file an issue on flat routing + S10 to @amin1377 and an issue on the z1000 config option issues to @AlexandreSinger ?

One functionality question: is there a way to select the primitive that is placed inside the cluster blocks or otherwise visualize what the name of that primitive instance is?

It would be useful to know what Adam netless primitive is placed in each primitive slot, visually or interactively somehow.

[soheilshahrouz]

Thanks @SamuelHo10

I left a few comment on coding style.

[SamuelHo10]

@vaughnbetz I got the Z1000 architecture to work with the help of Alex. However, when visualizing intra-cluster routing, the assertion on vpr_utils.cpp:1771 gets called.

ClusterBlockId blk_id = place_ctx.grid_blocks().block_at_location({x, y, sub_tile_num, layer});
VTR_ASSERT(blk_id != ClusterBlockId::INVALID());

Inter-cluster route visualization still appears to work. I suspect the issue arises because we’re loading the RR graph externally. Other UI features, like toggling RR, also seem to break on this architecture.

[vaughnb-cerebras]

OK, we can merge without waiting for that. But we should debug this. Can you file an issue to track it, and enlist help from Alex and Amin as needed?

[vaughnbetz]

Looks good!
One more change:

1. Change the names shown in the pb_type architecture hierarchy. Currently it seems you are printing pb_name:mode_name[index].
   Change that to
   pb_name[index]:mode_name.

Also, if pb_name == mode_name (before you add the [index]) you should omit the :mode_name part.

2. Probably post this PR:

• How do we show what is placed at a lowest level pb_type location? This can hold a primitive instance. If there is no primitive instance (from the atom netlist) show the final pb_type in what / empty. If there is something there, colour it in (I think you already do this). Confirm that clicking on that primitive shows its name. We could also print its name in brackets in that location, e.g.
  lut (bob_lut_to_fix_adder)

If this looks silly due to the instance names being long we can not do it.

[vaughnbetz]

@SamuelHo10 : could you show an example of this? I'm looking for the name of the atom placed there for primitives (if there is one) which should require an atom netlist lookup.
Again, if it looks messy due to being too long a name we can not do it.

[SamuelHo10]

The names seem short for this architecture at least.

[SamuelHo10]

@vaughnbetz I've updated the text display format as follows:

Non-primitives: <block_type_name>[<placement_index>]:<mode_name>

Primitives: <block_type_name>(<block_name>)

As for the visuals, I believe the current code uses dotted white rectangles to indicate empty blocks, and solid filled rectangles to represent blocks that either are primitives or contain primitives.

[vaughnbetz]

Thanks. That looks like the right name (an atom instance name -- it should match a name in the .blif file). You're right, that one is not too long!

[vaughnbetz]

LGTM!