Node Sides Patch to Show All the Sides that a RR Node may Appear

[tangxifan]

Improve the routing resource graph data structure to store all the node sides on which a rr_node may appear.

Description

Add a truth table to the rr_node_storage data structure which indicates on which sides a rr_node may appear.
The node sides are stored in an encoded format, where a char is used to represent if a rr_node appears on {TOP, RIGHT, BOTTOM, LEFT} sides. This keeps the rr_node storage size no changed.

This pull request introduces the following changes in the rr_node methods:

• The current method node_side() is changed to access the first valid side for a given rr_node. The priority list is {TOP, RIGHT, BOTTOM, LEFT}. For example, if a node is on both TOP and BOTTOM sides. The method node_side() will return TOP. A fatal error will be thrown if the given rr_node has multiple sides.
• The current method set_node_side() will set only 1 side to a given rr_node, which overwrite all the previous side attributes. For example, if a node was set to TOP side, using set_node_side(BOTTOM) will change the side attribute to BOTTOM side only.
• Add a new method node_sides() through which developers can get a list of sides that a given rr_node may appear.
• Add a new method node_on_specific_side() which is a shortcut for developers to find if a given rr_node appears on a specific side.
• Add a new method set_node_sides() which allows developers to set multiple sets to a given rr_nodes.
• Add a new method add_node_side() through which developers can update side attributes incrementally.

More methods may be added upon the needs of routers as well as other client modules.

RR graph reader and writer have been updated to support the multiple sides

• XML file now accepts strings for multiple sides, e.g., TOP_RIGHT
• Add a tiny RRGraph modeling a 11x8 array of Titan architecture in testing purpose

Related Issue

To address issue #1577

Motivation and Context

Previously, the routing resource graph assumes that a rr_node appears ONLY on a side when it is an output/input pin.
However, as the advancing of routing architecture modeling in #1448 , a rr_node may appears on several sides in Titan architectures.
This hits the limitation of current data structure. As a result, only 1 side is stored, leading to a chain of problems, such as

• Displaying these multi-sided nodes in GUI
• Router needs all the sides for a node to create accurate lookahead mapper
• etc.

How Has This Been Tested?

Two new tests are added in this pull request, which focuses on testing the reader and writer of RRGraph XML
Instead, sanity checks are added to RR graph build-up functions, which ensure that node sides to set are consistent with the storage in RR graph.

The pull request will change the way that client functions in placer, router and other VPR modules to access the sides for a given node in routing resource graphs.

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

[tangxifan]

@litghost I patched the node_side() method in rr-graph to show all the sides for a given rr-node.
The PR is ready for review.
See if the current status is good to merge or not. We can update client functions, such as GUI and lookahead mapper, in follow-up PRs.

[litghost]

You haven't updated https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/route/rr_graph.xsd , and so the --read_rr_graph and --write_rr_graph logic hasn't been updated.

[tangxifan]

You haven't updated https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/route/rr_graph.xsd , and so the --read_rr_graph and --write_rr_graph logic hasn't been updated.

Thanks for reminding. I will patch the --read_rr_graph and --write_rr_graph functions. But before doing that, we should agree on the file format changes. Since the node side is now a list of strings, current format xs:string may not work well for nodes with several sides. I am thinking about alternatives:

• Use a list of strings, such as <attribute name="side" type="TOP,LEFT"/>. But in this case, we have to enumerate all the 16 cases w.r.t. sides in the .xsd file.

 <xs:simpleType name="loc_side">
    <xs:restriction base="xs:string">
      <xs:enumeration value="TOP"/>
      <xs:enumeration value="RIGHT,TOP"/>
      <xs:enumeration value="RIGHT"/>
      ...
    </xs:restriction>
  </xs:simpleType>

• Use a bitmask, such as <attribute name="side" type="1001"/>. It will change the data type to xs:int. We have to return a bitmask in the node_sides() method, which I am trying to avoid because it is ambiguous for developers.

    <xs:attribute name="side" type="xs:int"/>

I am open to discussion here.

[litghost]

You haven't updated https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/route/rr_graph.xsd , and so the --read_rr_graph and --write_rr_graph logic hasn't been updated.

Thanks for reminding. I will patch the --read_rr_graph and --write_rr_graph functions. But before doing that, we should agree on the file format changes. Since the node side is now a list of strings, current format xs:string may not work well for nodes with several sides. I am thinking about alternatives:

• Use a list of strings, such as <attribute name="side" type="TOP,LEFT"/>. But in this case, we have to enumerate all the 16 cases w.r.t. sides in the .xsd file.

 <xs:simpleType name="loc_side">
    <xs:restriction base="xs:string">
      <xs:enumeration value="TOP"/>
      <xs:enumeration value="RIGHT,TOP"/>
      <xs:enumeration value="RIGHT"/>
      ...
    </xs:restriction>
  </xs:simpleType>

• Use a bitmask, such as <attribute name="side" type="1001"/>. It will change the data type to xs:int. We have to return a bitmask in the node_sides() method, which I am trying to avoid because it is ambiguous for developers.

    <xs:attribute name="side" type="xs:int"/>

I am open to discussion here.

There is an advantage to leaving this as an enumeration, and leaving the old order in place. It means that old rr graph files will work with the new schema.

[tangxifan]

You haven't updated https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/master/vpr/src/route/rr_graph.xsd , and so the --read_rr_graph and --write_rr_graph logic hasn't been updated.

Thanks for reminding. I will patch the --read_rr_graph and --write_rr_graph functions. But before doing that, we should agree on the file format changes. Since the node side is now a list of strings, current format xs:string may not work well for nodes with several sides. I am thinking about alternatives:

• Use a list of strings, such as <attribute name="side" type="TOP,LEFT"/>. But in this case, we have to enumerate all the 16 cases w.r.t. sides in the .xsd file.

 <xs:simpleType name="loc_side">
    <xs:restriction base="xs:string">
      <xs:enumeration value="TOP"/>
      <xs:enumeration value="RIGHT,TOP"/>
      <xs:enumeration value="RIGHT"/>
      ...
    </xs:restriction>
  </xs:simpleType>

• Use a bitmask, such as <attribute name="side" type="1001"/>. It will change the data type to xs:int. We have to return a bitmask in the node_sides() method, which I am trying to avoid because it is ambiguous for developers.

    <xs:attribute name="side" type="xs:int"/>

I am open to discussion here.

There is an advantage to leaving this as an enumeration, and leaving the old order in place. It means that old rr graph files will work with the new schema.

No problem then. I will patch it.

[litghost]

Need method descriptions.

[litghost]

Looks really good! Please add method descriptions to the new methods on rr_graph_storage. Otherwise, I think this is ready for final review and merge if green.

[tangxifan]

@litghost I have added the comments about the new APIs about how the return values should be used. It is ready for you and @vaughnbetz to review.

[litghost]

LGTM

[tangxifan]

It seems that all the pull requests are staled because the updates on CI, I merged latest master and it contains errors on the Python test. If this has been resolved on the master branch, I can update my local one.

[litghost]

It seems that all the pull requests are staled because the updates on CI, I merged latest master and it contains errors on the Python test. If this has been resolved on the master branch, I can update my local one.

The Python CI is not marked as required, so it shouldn't block anything. The other CIs are still running.

[tangxifan]

It seems that all the pull requests are staled because the updates on CI, I merged latest master and it contains errors on the Python test. If this has been resolved on the master branch, I can update my local one.

The Python CI is not marked as required, so it shouldn't block anything. The other CIs are still running.

Got it. No worries then. Seeing some red cross is not so friendly to OCD patients :D

[litghost]

It seems that all the pull requests are staled because the updates on CI, I merged latest master and it contains errors on the Python test. If this has been resolved on the master branch, I can update my local one.

The Python CI is not marked as required, so it shouldn't block anything. The other CIs are still running.

Got it. No worries then. Seeing some red cross is not so friendly to OCD patients :D

Someone does need to do a reformat to get it green.

[vaughnbetz]

Looks good. As discussed in the meeting, I think in the follow up commit it is good to add a comment on why you went with the 16 enum solution for the sides of a block that a node appears in (compatibility with old rr_graph parsing, by keeping an enum with the first four values I think).

Also, when coding the multi-side support in a follow-on PR, please check if the node_sides (returns a bitset for iteration) is indeed used and cleaner than just looping over the sides and calling node_on_specific_side(). I don't have a big objection to the bitset, but a narrower interface is a bit simpler to maintain if it doesn't cost us anything. But if the bitset is used and works better in some callers, then keeping it makes sense.

I'm not sure why the nightly tests haven't run yet; trying a kokoro restart.

[tangxifan]

@vaughnbetz I have added the comments to clarify

• the use of 16 enumeration for node sides.
• the design purpose of the node_sides() API. And comment that it may be deprecated later depending on the client function.

Still, waiting for the nightly test to finish.

[vaughnbetz]

Are the nightly tests taking unusually long? Lots of changes in CI and in placement settings (new RL stuff) so worth flagging it if things have slowed down.

[vaughnbetz]

Changes look good, thanks. Just waiting for CI to finish ...

[tangxifan]

Are the nightly tests taking unusually long? Lots of changes in CI and in placement settings (new RL stuff) so worth flagging it if things have slowed down.

According to my experience, it was almost 1 day run. I checked the log. It started 3PM yesterday. So I expect to finish this afternoon. Otherwise, something may go wrong.

[tangxifan]

@vaughnbetz I have removed the warning message from log files, as part of the issue #1647