Skip to content
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

WIP: Automated pack patterns #101

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

acomodi
Copy link
Contributor

@acomodi acomodi commented Dec 29, 2020

This PR addresses #89, and builds on top of #75.

It tries to add automation in the pack patterns generation without requiring the (* pack *) attribute added to the wire for which tha pack_pattern needs to be generated.

The implementation takes care also of the propagation of the pack pattern only to the first level of children pb_types, modifying the already generated XML to add the pack pattern.

TODO:

  • Add more tests specific to the pack pattern generation
  • Fix the documentation accordingly

mkurc-ant and others added 10 commits December 22, 2020 19:24
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@mithro
Copy link
Collaborator

mithro commented Dec 29, 2020

The implementation takes care also of the propagation of the pack pattern only to the first level of children pb_types, modifying the already generated XML to add the pack pattern.

Could you explain this a little more?

@@ -18,14 +18,12 @@ module CBLOCK (
COUT
);
input wire [3:0] I;
(* carry="C" *)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the (* carry *) removed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understood, carry was used to generate a pack pattern itself. I am not sure why this needed to be handled in a special case, but with the new code, the pack pattern should be automatically detected.

@@ -1,80 +1,74 @@
<?xml version='1.0' encoding='utf-8'?>
<pb_type xmlns:xi="http://www.w3.org/2001/XInclude" name="CARRY" num_pb="1">
<pb_type xmlns:xi="http://www.w3.org/2001/XInclude" num_pb="1" name="CARRY">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the order changing here? I think these use to be alphabetically sorted?

@@ -0,0 +1,25 @@
Forking net annotation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably call this some "internal and top"?

@acomodi
Copy link
Contributor Author

acomodi commented Dec 29, 2020

Could you explain this a little more?

@mithro Sure.

In v2x the pb_type/model generation is an iterative step, where the hierarchy must be generated from leaf pb_types to the top-level ones.

For instance, if a top-level pb_type is to be generated, all its children needs to be already generated in a previous step, and those will be added as <xi:include>s.
This poses an issue with the pack_pattern automatic generation, as, if a wire on a top-level pb_type (or intermediate pb_type) requires a pack_pattern, than the whole path of direct connections of its children pb_types (that are included with <xi:include> require the addition of the same pack_pattern.

pack_patterns

In the above example, there is a three level hierarchy

  • LOGIC
    • SLICE_MODES
      • MODE 1
      • MODE 2
    • FLIP FLOP

At the moment of generation of the LOGIC the SLICE_MODES one has already been generated, hence we need to modify it to add the pack pattern required to connect the SLICE_MODES to FLIP FLOP. If the additional pack pattern in SLICE_MODE is not added, VPR throws an error as it would not be able to find the source block for that pack pattern.

Copy link
Contributor

@mkurc-ant mkurc-ant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one concern about modifying already generated XMLs. What if such XML is included by more than one pb_type? Say you have a single FF model used in two places in the arch (eg. different modes). For such a case different pack-pattern names may be required. So effectively the included XML would need to be duplicated.

@mithro
Copy link
Collaborator

mithro commented Dec 29, 2020

I have one concern about modifying already generated XMLs. What if such XML is included by more than one pb_type? Say you have a single FF model used in two places in the arch (eg. different modes). For such a case different pack-pattern names may be required. So effectively the included XML would need to be duplicated.

We should not be modifying already generated XML.

@acomodi
Copy link
Contributor Author

acomodi commented Dec 29, 2020

We should not be modifying already generated XML.

@mithro I think that we would need to duplicate the XML then, as @mkurc-ant suggested. And change the reference in the top level pb_type to include the newly generated child with the pack patterns.

@mithro
Copy link
Collaborator

mithro commented Dec 29, 2020

In v2x the pb_type/model generation is an iterative step, where the hierarchy must be generated from leaf pb_types to the top-level ones.

For instance, if a top-level pb_type is to be generated, all its children needs to be already generated in a previous step, and those will be added as <xi:include>s.
This poses an issue with the pack_pattern automatic generation, as, if a wire on a top-level pb_type (or intermediate pb_type) requires a pack_pattern, than the whole path of direct connections of its children pb_types (that are included with <xi:include> require the addition of the same pack_pattern.

This is what the https://github.com/SymbiFlow/vtr-xml-utils/blob/master/vtr_xml_utils/resources/pack-patterns.xsl and https://github.com/SymbiFlow/vtr-xml-utils/blob/master/vtr_xml_utils/resources/convert-prefix-port.xsl are suppose to achieve?

GitHub
Contribute to SymbiFlow/vtr-xml-utils development by creating an account on GitHub.
GitHub
Contribute to SymbiFlow/vtr-xml-utils development by creating an account on GitHub.

@mithro
Copy link
Collaborator

mithro commented Dec 29, 2020

See also https://github.com/SymbiFlow/vtr-xml-utils/blob/master/tests/convert_and_merge_composable_tests/composable-interconnect-pack_patterns.xml

GitHub
Contribute to SymbiFlow/vtr-xml-utils development by creating an account on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants