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

Get @kc8apf eblif attribute/parameter change into upstream #3

Closed
mithro opened this issue Mar 12, 2018 · 6 comments
Closed

Get @kc8apf eblif attribute/parameter change into upstream #3

mithro opened this issue Mar 12, 2018 · 6 comments

Comments

@mithro
Copy link
Owner

mithro commented Mar 12, 2018

Take a look at verilog-to-routing#281

@kc8apf has a WIP change at verilog-to-routing@2235541

@kc8apf is happy for @daveshah1 to steal this.

@kmurray is happy with the way this is going. Lets get it upstream into vpr.

@daveshah1
Copy link
Collaborator

I have rebased this onto current master (including the change to pugixml), finished the parameter support and tidied it up a bit in 68565f1.

The only remaining issue in my mind is the output syntax. @kmurray gave this example in VPR verilog-to-routing#281:

        <parameters>
            <parameter name="my_param" value="my_param_value"/>
        </parameters>
       <attributes>
            <attribute name="my_attr" value="my_attr_value"/>
        </attributes>

But the current implementation does not use the parameters or attributes wrappers, instead putting attributes and parameters straight into the block thus:

<attribute name="test_attr">1</attribute>
<parameter name="test_param">x</parameter>

I'm not sure which option is preferable though?

@mithro
Copy link
Owner Author

mithro commented Mar 13, 2018

I think we prefer my example with the value inside the tag <t>value</t>? I think this because the value of attribute / parameters might have double quotes or other similar types of characters which are harder to escape inside the tag?

It also lets us expand with maybe a "type" value?

<attributes>
  <attribute name="xxx" type="yyy">value</attribute>
</attributes>

@kmurray
Copy link

kmurray commented Mar 13, 2018

I'm happy with either format, although to be consistent with the current .net format the <attribute>/<parameter> tags should be wrapped in containing <attributes>/<parameters> tags.

I'm not sure adding a 'type' field is a good idea. Treating the attributes/params only as strings means VPR can just blindly pass them through and doesn't have to understand what they mean. To me this seems less fragile and more general (since the potential set of params/attributes which can be set by other tools is unbounded).

@daveshah1
Copy link
Collaborator

I will add the <attributes> and <parameters> wrappers to the PR. Do you think it is best to generate empty wrappers when there are no attributes/parameters, or should the wrappers only be generated when there is at least one attribute/parameter?

@kmurray
Copy link

kmurray commented Mar 13, 2018

Looking at the existing format it seems like the convention is to generate empty wrappers even when there are no elements (e.g. <clocks> is still specified even for combinational primitives). Probably best to stick with the convention.

@daveshah1
Copy link
Collaborator

This has been implemented in 2851778

@mithro mithro closed this as completed Mar 17, 2018
mithro pushed a commit that referenced this issue Apr 7, 2018
1fc200f Merge pull request #7 from sterin/master
9c78efb Makefile: add support for ABC_USE_STDINT_H
4a39f32 Merge pull request #2 from rqou/master
8d472cd Rename new flag to ABC_USE_STDINT_H
d879336 Merge pull request #5 from sterin/master
c5aebf6 README: minor updates
2fd7ba5 Merge pull request #1 from gpshead/patch-1
41eb4ea Merge pull request #3 from kmurray/fix_cmake_libabc_dependency
c3be5dc CMake: Ensure abc executable depends on libabc
40c8a39 Add an option to use C99 stdint.h
ce4b3cf point to github instead of bitbucket

git-subtree-dir: abc
git-subtree-split: 1fc200ffacabed1796639b562181051614f5fedb
mithro pushed a commit that referenced this issue May 28, 2019
Initial FASM output from VPR
mithro pushed a commit that referenced this issue Aug 26, 2020
mithro pushed a commit that referenced this issue Jun 3, 2021
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

No branches or pull requests

3 participants