-
Notifications
You must be signed in to change notification settings - Fork 416
Docs: Adding clock architecture documentation #1104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@HackerFoo cant seem to add you as a reviewer for some reason. But mentioning you here in case you find this helpful |
FYI - @litghost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together @mustafabbas ! It looks like a great start.
I've included various comments below, but one general comment about the documentation .rst file formatting. My general approach is to use the single-sentance per line approach, as this tends to produce much easier to read revision control diffs for changes. So my preference would be to convert the added documentation to put each sentance on a separate line (sphinx/readthedocs) will keep any adjacent lines in the .rst in the same paragraph when its formatted.
Finally, we should somewhere have documentation about how the virtual-sink works in VPR. Its probably not here in the architecture documentation but we do need a diagram showing how this is modelled (i.e. single virtual sink for all clock networks of the same type). At the moment this should probably go in the VPR source code, but in the future when we've added support for defining this in the rr_graph.xml, it should likely go in the documentation there.
doc/src/arch/reference.rst
Outdated
|
||
**Specifing Clocking Purely for Power Estimation** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall correctly, the power estimation code just assumes a simple rib-spine structure whose power consumption is estimated from the parameters described here.
My question, is:
- Does the power estimation code still assume a rib-spine regardless of what's specified in
<clocknetworks>
? Or, - Does power estimation considers the new clock network structure (plus these parameters) to do power estimation,?
My suspicion is that it does the first, which is likely not what end-users would expect. In which case we'll want to make some explicit mention (perhaps a warning?) in the documentation that this is the case. (We should also file an issue to eventually get the power estimation code driven from the actual section).
doc/src/arch/reference.rst
Outdated
</clock_routing > | ||
</clocknetworks > | ||
|
||
The ``<metal_layers>`` element describes the per unit length electrical parameters used to implement the clock distribution wires. There can be one or more wiring implementation options (metal layer, width and spacing) that are used by the later clock network specification and each is described in a separate ``<metal_layer>`` sub-element. The syntax of the wiring electrical information is: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd likely add that the metal layer width and spacing are the physical implementation characteristics which result in the Rmetal/Cmetal but that we only model them as those characteristics (as currently written someone might expect width/spacing options for the metal layers).
doc/src/arch/reference.rst
Outdated
:req_param name: A unique string for reference. | ||
:req_param num_inst: which describes the number of parallel instances of the clock distribution types described in the ``<clock_network>`` sub-elements. | ||
|
||
The supported clock distribution types are ``<spine>`` and ``<rib>``. ``Spine`` is used to describe vertical clock distribution wires. Whereas, ``Rib`` is used to describe a horizontal clock distribution wire. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have diagrams illustrating what these options are (c.f. https://docs.verilogtorouting.org/en/latest/arch/reference/#grid-location-tags).
Probably one each for spine and rib. It would also be good to label the various options (e.g. x, startx, starty, endy, repeatx, repeaty).
doc/src/arch/reference.rst
Outdated
|
||
The supported clock distribution types are ``<spine>`` and ``<rib>``. ``Spine`` is used to describe vertical clock distribution wires. Whereas, ``Rib`` is used to describe a horizontal clock distribution wire. | ||
|
||
.. arch:tag:: <spine metal_layer="string" x="integer" starty="integer" endy="integer" repeatx="integer" repeaty="integer"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many of these parameters aren't actually integers but expressions (e.g. H
, H/2
, W
). Should probably change the type to something like 'expr'.
We should also have a section which just describes what the expressions are and what the variables mean (e.g. W, H). If I recall correctly these are the same variables/expressions as used in grid construction, so we can likely just cross-ref to https://docs.verilogtorouting.org/en/latest/arch/reference/#grid-location-expressions. Although we should remind readers here that W and H are the device grid width/height and that w
and h
aren't available.
doc/src/arch/reference.rst
Outdated
:opt_param repeatx: The horizontal repeat factor of the spine along the device. Value can be relative to the device size. | ||
:opt_param repeaty: The vertical repeat factor of the spine along the device. Value can be relative to the device size. | ||
|
||
The provided example clock network defines two spines, and neither repeats as each spans the entire height of the device and is locally at the horizontal midpoint of the device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add a cross-reference back to the example you are referring to. Likely means you'll need to label the example code block so you can :ref:
it.
doc/src/arch/reference.rst
Outdated
.. code-block:: xml | ||
<clocknetworks> | ||
<metal_layers > | ||
<metal_layer name="global_spine" Rmetal="50.42" Cmetal="20.7e-15"/> | ||
<metal_layer name="global_rib" Rmetal="50.42" Cmetal="20.7e-15"/> | ||
</metal_layers > | ||
|
||
<!-- Full Device: Center Spine --> | ||
<clock_network name="spine1" num_inst="2"> | ||
<spine metal_layer="global_spine"12 x="W/2" starty="0" endy="H"> | ||
<switch_point type="drive" name="drive" yoffset="H/2" buffer="drive_buff"/> | ||
<switch_point type="tap" name="tap" yoffset="0" yincr="1"/> | ||
</spine> | ||
</clock_network> | ||
<!-- Full Device: Each Grid Row --> | ||
<clock_network name="rib1" num_inst="2"> | ||
<rib metal_layer="global_rib" y="0" startx="0" endx="W" repeatx="W" repeaty="1"> | ||
<switch_point type="drive" name="drive" xoffset="W/2" buffer="drive_buff"/> | ||
<switch_point type="tap" name="tap" xoffset="0" xincr="1"/> | ||
</rib> | ||
</clock_network> | ||
|
||
<clock_routing> | ||
<!-- connections from inter-block routing to central spine --> | ||
<tap from="ROUTING" to="spine1.drive" locationx="W/2" locationy="H/2" switch="general_routing_switch" fc_val="1.0"/> | ||
<!-- connections from spine to rib --> | ||
<tap from="spine1.tap" to="rib1.drive" switch="general_routing_switch" fc_val="0.5"/> | ||
<!-- connections from rib to clock pins --> | ||
<tap from="rib1.tap" to="CLOCK" switch="ipin_cblock" fc_val="1.0"/> | ||
</clock_routing > | ||
</clocknetworks > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the example, but I think we'll need a diagram illustrating what this clock network looks like. Otherwise only people very familiar with how this is implemented in VPR will be able to follow.
doc/src/arch/reference.rst
Outdated
Lastly the ``<clock_routing>`` element consists of a group of ``tap`` elements that each separately describe the connectivity from one routing resource (pin or wire) to another. The tap element and its attribute sare as follows: | ||
|
||
.. arch:tag:: <tap from="string" to="string" locationx="integer" locationy="integer" switch="string" fc_val="float"> | ||
:req_param from: The routing resource to make a connection from. The referecing schema can be seen in the clock network exambple where the unique clock network name is forllowe by a period and then the switch name. In addition a special literal ``"ROUTING"`` can be used to reference a connection from the general inter-block routing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want to expand this a bit more formally illustrating the two cases:
clock_name.tap_name
format, andROUTING
special value
doc/src/arch/reference.rst
Outdated
|
||
.. arch:tag:: <tap from="string" to="string" locationx="integer" locationy="integer" switch="string" fc_val="float"> | ||
:req_param from: The routing resource to make a connection from. The referecing schema can be seen in the clock network exambple where the unique clock network name is forllowe by a period and then the switch name. In addition a special literal ``"ROUTING"`` can be used to reference a connection from the general inter-block routing. | ||
:req_param to: The routing resource to make a connection to. A special literal ``"CLOCK"`` describes connections from clock network tap points that supply the clock to clock pins on blocks at the tap locations; these clock inputs are already specified on blocks in the VTR architecture file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here
doc/src/arch/reference.rst
Outdated
Lastly the ``<clock_routing>`` element consists of a group of ``tap`` elements that each separately describe the connectivity from one routing resource (pin or wire) to another. The tap element and its attribute sare as follows: | ||
|
||
.. arch:tag:: <tap from="string" to="string" locationx="integer" locationy="integer" switch="string" fc_val="float"> | ||
:req_param from: The routing resource to make a connection from. The referecing schema can be seen in the clock network exambple where the unique clock network name is forllowe by a period and then the switch name. In addition a special literal ``"ROUTING"`` can be used to reference a connection from the general inter-block routing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also small typo: forllowe
-> followed
doc/src/arch/reference.rst
Outdated
:req_param locationx: (Only when using the special literal "ROUTING") The x grid location of inter-block routing. | ||
:req_param locationy: (Only when using the special literal "ROUTING") The y grid location of inter-block routing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably need a bit more detail about how this works.
What does it do at the specific X/Y location? Does it pick all wires passing that location (i.e. both CHANX and CHANY)? Does it only connect to wires starting/ending at that X/Y, or wires which have switchpoints, or any wire?
doc/src/arch/reference.rst
Outdated
@@ -1897,6 +1902,98 @@ The clocking configuration is specified with ``<clock>`` tags within the ``<cloc | |||
:opt_param C_wire_per_m: The wire capacitance, in Farads per Meter. | |||
:opt_param buffer_size: The size of each clock buffer. | |||
|
|||
**Specifing a Clock Architecture** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't talk about the virtual clock sink, and how that is placed within the grid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that it's specified in the clocknetworks/clock_routing/tap[@from="ROUTING"]
element. See setup_clock_connections()
in setup_clocks.cpp
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the current code seems to only support one virtual clock network root, with the index stored in the DeviceContext
.
@mustafabbas It might be good to also include your thesis somewhere, or at least a reference. |
Thanks @mustafabbas, those figures look great! I've fixed up some of the formatting and clarified some of the descriptions. Once CI passes this should be good to merge. |
Thanks a lot Mustafa! |
Documentation for the clock architecture spec
Related Issue
Addresses part of #928 (comment)
Motivation and Context
Adds documentation for an implemented architecture option
How Has This Been Tested?
N/A
Types of changes
Documentation
Checklist: