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

[Branch] carry-rework #40

Open
mithro opened this issue Sep 5, 2019 · 4 comments
Open

[Branch] carry-rework #40

mithro opened this issue Sep 5, 2019 · 4 comments

Comments

@mithro
Copy link
Member

mithro commented Sep 5, 2019

Why did we need this? (what does this change enable us to do)

VPR uses a different model for how the CARRY4 works.

This is needed because "chain" pack patterns must only have one root pin.

See

What did it change?

Combines the 4 * CARRY modules found in a slice into a CARRY4 module.

Should it be merged upstream - if not, when can we delete it?

When VPR supports generic tree style pack patterns for carry chains.

@eddiehung
Copy link

synth_xilinx -abc9 already emits CARRY4 instead of MUXCY/XORCY, and there is already some special handling for the -vpr option. Can you provide a PR of the changes you'd like to make? As I remember with talking to @litghost in the past, that your CARRY4 is different to the vendor one?

@litghost
Copy link

litghost commented Sep 6, 2019

As I remember with talking to @litghost in the past, that your CARRY4 is different to the vendor one?

The CARRY4 we use with VPR is different than the vendor one due to a limitation in how VPR models carry chains. VPR had (and probably still has) has limitation on how carry chains are defined. Specifically if the carry chain can both reach the dedicated fabric and the general routing fabric, it doesn't work correctly. Because of this limitation, the -vpr flag handles the CARRY4 block differently.

There is also the fact that unregistered CARRY4 outputs can easily result in congestion at the output muxes, resulting the need for passing through the output FF's as latches (which is bad for performance).

In the long run, these limitations will be removed, but they are not a priority right now as this branch modifies the CARRY4 instantiations that accomidates VPR's limitations.

@eddiehung
Copy link

In the long run, these limitations will be removed, but they are not a priority right now as this branch modifies the CARRY4 instantiations that accomidates VPR's limitations.

Sorry, my impression from #43 is that we wanted to work towards driving this delta down to zero @mithro? If so, let me study the problem some more and see if there's a more elegant way of achieving this, now that the flow has evolved some more...

@litghost
Copy link

litghost commented Sep 9, 2019

Yosys currently already has a custom CARRY4 for VPR, this branch updates it based on the -abc9 work. I haven't pushed it upstream yet, but it is something that is planned.

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