check_rr_graph: make zero capacity nodes valid and not drawn.

[acomodi]

Signed-off-by: Alessandro Comodi acomodi@antmicro.com

Description

This PR allows to have zero-capacity nodes when checking the rr_graph. In addition zero-capacity nodes are not drawn.

How Has This Been Tested?

It has been tested on my local machine, but it should not require any additional regression tests.

Types of changes

• Bug fix (change which fixes an issue)
• New feature (change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed

[kmurray]

@acomodi @mithro Can you give a bit more context/motivation for why zero capacity nodes are needed?

[acomodi]

Hi @kmurray,
Basically, this patch is to avoid the following error:

Type: Routing
File: verilog-to-routing/vpr/src/route/check_rr_graph.cpp
Line: 395
Message: in check_rr_node: inode 1085242 (type 4) has a capacity of 0 but required 1.
...

The error is produced by:

if (capacity != tracks_per_node) {                                                                                              
    vpr_throw(VPR_ERROR_ROUTE, __FILE__, __LINE__,                                                                              
            "in check_rr_node: inode %d (type %d) has a capacity of %d.\n", inode, rr_type, capacity);
}                                                                                                                               
break;  

Nodes with zero-capacity (correct me if I'm wrong) should not be affected by the above check, hence they should be treated differently.

@mithro and @litghost can provide further insight about this.

[vaughnbetz]

The key question is why there are 0 capacity nodes in the rr-graph. The check can be disabled as you show, but why is it necessary? No prior architectures have used 0 capacity rr-graph nodes, as they can never be used by the router and hence don't seem to serve any purpose.

If the underlying problem is that the rr-graph has degenerate nodes that shouldn't be there, they should be taken out of the rr-graph xml file. If there's a reason why they have to be there, that reason should be described in the comments.

[litghost]

The key question is why there are 0 capacity nodes in the rr-graph. The check can be disabled as you show, but why is it necessary? No prior architectures have used 0 capacity rr-graph nodes, as they can never be used by the router and hence don't seem to serve any purpose.

The reason for 0 capacity nodes has to do with the CHANX/Y ptc assignment step when reading RR graphs. See https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/route/rr_graph_reader.cpp#L751-L795 Basically that code enforces that at a particular (x, y), the number of channels present must be equal to the highest PTC value amoungst the channels. A relaxation of this requirement would remove the need for padding channels with capacity 0.

[vaughnbetz]

Thanks. That code is unnecessarily limiting. We should change the allocation of the vector (indices[CHANY][ix][iy][0]) to instead set its size to the maximum ptc_num at that location + 1. Basically:

1. loop through once finding the maximum ptc_num at each (x,y) and storing it for each (x,y) in some variable
2. then resize or construct indices[CHANY][ix][iy][0] to have ptc_num(ix,iy) + 1 entries.
3. Initialize all the values in the vector to -1 (OPEN/INVALID) so any ptc_num values that aren't used have an invalid node value (we shouldn't look at them).
4. Then the load loops can run normally, and should never fail the size check.

There are not too many points in the code where we use this indices data structure; it's a reverse lookup that lets us find the nodes at a certain location when constructing an rr_graph, and I don't think it's used for much else. There may be a few spots where code needs to be updated to understand that there may not be any rr_node at a certain x,y,ptc_num (i.e. you get OPEN/INVALID).

[vaughnbetz]

Upside of this change: we remove the restriction that ptc_num must be sequential (not have any gaps between 0 and some maximum) at each (x,y) location. That makes it easier to import rr_graphs.

[litghost]

There are not too many points in the code where we use this indices data structure; it's a reverse lookup that lets us find the nodes at a certain location when constructing an rr_graph, and I don't think it's used for much else. There may be a few spots where code needs to be updated to understand that there may not be any rr_node at a certain x,y,ptc_num (i.e. you get OPEN/INVALID).

The reason I went with 0 capacity nodes was to avoid unexpected behavior changes where the chan PTC is used. I agree the remove this restriction is ideal, and it is superior to do that than to accommodate 0 capacity nodes. @acomodi Could you work with the VTR devs to make that change?

[mithro]

It was a hack but the zero capacity nodes were / are also useful for putting a visual break (in the GUI) between segments of one type and another. (IE The segments which run 4 tiles and the segments which run 8 tiles.)

This makes it easier to see which groups of wires are being used.

I think a probably better alternative is to allow metadata to provide information to the GUI on how to display things. Then we can give segment types different colors.

[acomodi]

@litghost, @vaughnbetz Sure, I need to get more familiar regarding this matter. Can you open a ticket explaining the issue (so that it is easily traceable) please?

[litghost]

It was a hack but the zero capacity nodes were / are also useful for putting a visual break (in the GUI) between segments of one type and another. (IE The segments which run 4 tiles and the segments which run 8 tiles.)

This makes it easier to see which groups of wires are being used.

I think the proposal here is rather than assigning a PTC to a 0 capacity node, instead have the GUI treat an OPEN location in the PTC lookup as a 0 capacity node. I think it is effectively the same, but reduces memory footprint, and will simplify a couple steps in routing import.

[litghost]

@litghost, @vaughnbetz Sure, I need to get more familiar regarding this matter. Can you open a ticket explaining the issue (so that it is easily traceable) please?

vaughnbetz explained the algorithm change in #599 (comment) pretty well, the question is then changing all sites in VTR where the PTC lookup to handle OPEN/INVALID node ids.

[litghost]

I think a probably better alternative is to allow metadata to provide information to the GUI on how to display things. Then we can give segment types different colors.

FYI, the PTC is the metadata about GUI placement. So even with the proposed change, we still have the ability to control placement. This is about removing the need for 0 capacity padding nodes.

[vaughnbetz]

Yes, agreed that the GUI visual breaks will occur if you leave some ptc_num values empty in some channel segments, without the padding nodes. So this change still allows that, but I agree with @litghost that it saves memory.

[vaughnbetz]

Superceded by the ability to have non-consecutive ptc_nums.