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

interchange: arch: do not allow site pips within sites #700

Merged
merged 2 commits into from
May 13, 2021

Conversation

acomodi
Copy link
Contributor

@acomodi acomodi commented May 10, 2021

During general routing, the only site pips that can be allowed are those which connect a site wire to the routing interface. Or, in more general terms, all the PIPs connecting a driver to the routing interface.

This patch though might be too restrictive when dealing with architectures that require more than one site PIPs to route from a driver within a site to the routing interface (which is something that should be allowed in the interchange).

After all, backtracking to validate a path might still be required, as #695 is not enough to cover the faulty edge-cases, where a series of PIPs were used to completely route thru a site.

@gatecat
Copy link
Member

gatecat commented May 10, 2021

I think we should do the proper backtracking, it shouldn't be too hard to implement. This will likely cause some failures as I don't believe the site router pre binds routing in all cases (iirc only it only pre binds going into inputs, not from outputs, and the router would probably be have to modified to take into account pre binded routing going from outputs).

@gatecat
Copy link
Member

gatecat commented May 10, 2021

One option would be to store legal site pips that are in the input/output cones of bels used by the net after site routing, then the check doesn't need to do the backtracking itself and just consults that set of pips.

@acomodi
Copy link
Contributor Author

acomodi commented May 11, 2021

This will likely cause some failures as I don't believe the site router pre binds routing in all cases

Yeah, I believe this too. I imagine that at the moment this is not causing any failures as we do not hit edge cases in the tests. In fact AFAIU, this check was to allow, for instance, routing from a FF output through an OUTMUX and then to the general interconnect. If there were other site pips to be traversed, than the path to route from the FF to the output would be illegal, even though it should be explored.

One option would be to store legal site pips that are in the input/output cones of bels used by the net after site routing, then the check doesn't need to do the backtracking itself and just consults that set of pips.

Hmm right. If I understand this correctly, this would mean checking all downhill site pips from a BEL output (as the input should be already pre-binded) that lead to the site output ports and mark all those site PIPs as valid, correct?

@gatecat
Copy link
Member

gatecat commented May 11, 2021

Hmm right. If I understand this correctly, this would mean checking all downhill site pips from a BEL output (as the input should be already pre-binded) that lead to the site output ports and mark all those site PIPs as valid, correct?

Yeah, I think this is the way forward

During general routing, the only site pips that can be allowed are those
which connect a site wire to the routing interface.

This might be too restrictive when dealing with architectures that
require more than one site PIPs to route from a driver within a site to the routing
interface (which is something that should be allowed in the
interchange).

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@acomodi
Copy link
Contributor Author

acomodi commented May 12, 2021

@gatecat I have implemented the suggested solution, all the driver wires downhill PIP's that enable reaching an output site port are stored and than visited when determining a validity of a PIP.

I think the solution might be improved to store some data in the site_router_cache as there may be quite some repetitive pattern. In fact the valid PIPs will always be the same for a given site wire.

@gatecat
Copy link
Member

gatecat commented May 12, 2021

I think it should be possible to improve performance by doing it when we bind site routing, in between placement and routing, rather than throughout placement at the point of checking the validity of site routing?

But yeah, overall the site routing stuff could doubtless be a lot faster.

@acomodi
Copy link
Contributor Author

acomodi commented May 12, 2021

At the moment the list of valid PIPs is already built in between placement and routing, during the bindSiteRouting step, so it shouldn't impact placement run-time.

I'll investigate some optimizations to this patch, and also check the impact on the murax design (and whether all the edge cases are solved).

@gatecat
Copy link
Member

gatecat commented May 12, 2021

Ah, sorry, I misread the misread the GitHub diff, that's fine then.


// There needs to be at least one path that leads
// from a cell driver to a site output sink
NPNR_ASSERT(result);
Copy link
Member

Choose a reason for hiding this comment

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

this isn't necessarily true in two cases:

    1. all of the users of the net are contained within the site
    1. the net has no users (this is a trivial case and just needs && !net->users.empty() added to the if statement)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added fix for case ii.

For i. I might be missing something here, but I guess that, if a site wire corresponding to a net driver got assigned to a net, that would mean that that net was already handled during site routing, therefore there is no need to check for it's valid PIPs.

Copy link
Member

Choose a reason for hiding this comment

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

For i. I might be missing something here, but I guess that, if a site wire corresponding to a net driver got assigned to a net, that would mean that that net was already handled during site routing, therefore there is no need to check for it's valid PIPs

It's true that we don't care about the set of valid PIPs; but that means we also need to ignore the net and not have an assertion that is likely to fail (because it doesn't matter if we find PIPs to a site pin or not).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, right, I wonder if this assertion needs to be there at all at this point. As the router would fail as well if there are no PIPs to route to a site pin, with or without this assertion. Given that there are drivers within a site that may not have routes to a site pin at all, and these drivers are not handled by the router (I think).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think it's safest just to remove the assertion for now.

@acomodi acomodi force-pushed the fix-illegal-site-thru branch 3 times, most recently from 7638612 to 4a4ef38 Compare May 12, 2021 18:32
Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@gatecat gatecat merged commit 21d594a into YosysHQ:master May 13, 2021
@acomodi acomodi deleted the fix-illegal-site-thru branch May 13, 2021 10:05
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.

2 participants