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

Adds a tieoff method for module instances with unconnected inputs #811

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

clavin-xlnx
Copy link
Member

No description provided.

+ getName() + "/" + port.getName() + " skipping...");
}
if (pin != null && (pin.getNet() == null || pin.getNet().getSource() == null)) {
boolean useVccInverter = pin.getName().contains("RST");
Copy link
Collaborator

@eddieh-xlnx eddieh-xlnx Sep 12, 2023

Choose a reason for hiding this comment

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

This seems like an important condition to document the behaviour of.

The name of this boolean is useVccInverter but then when it gets passed to connectToStaticNet() it becomes its connectToVcc argument which does not seem to reconcile, because it doesn't look like this latter method does any inversion?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've modified the code to clarify and harmonize the usage.

src/com/xilinx/rapidwright/design/ModuleInst.java Outdated Show resolved Hide resolved
+ "] has no source, connecting to " + (connectToVCC ? "VCC" : "GND"));
}

connectToStaticNet(port.getName(), connectToVCC);
Copy link
Collaborator

@eddieh-xlnx eddieh-xlnx Jan 16, 2024

Choose a reason for hiding this comment

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

Do we know if simply connecting the SPI to VCC does what we expect? Does any intra-site routing/SitePIPs need to be set?
EDIT: Perhaps not relevant if we stop inverting.

Comment on lines +814 to +823
// Connect logically
EDIFCell parent = getCellInst().getParentCell();
EDIFNet logicalStaticNet = EDIFTools.getStaticNet(type, parent, getDesign().getNetlist());
EDIFPortInst portInst = getCellInst().getPortInst(portName);
if (portInst == null) {
logicalStaticNet.createPortInst(portName, getCellInst());
} else {
portInst.getNet().removePortInst(portInst);
logicalStaticNet.addPortInst(portInst);
}
Copy link
Collaborator

@eddieh-xlnx eddieh-xlnx Jan 16, 2024

Choose a reason for hiding this comment

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

When connectToVCC is true, this would also make the port logically connected to VCC (whereas if you're relying on the inverter, shouldn't it be GND?)
EDIT: Perhaps not relevant if we stop inverting.

}
if (pin != null && (pin.getNet() == null || pin.getNet().getSource() == null)) {
// If a port has a RST inverter, drive it with VCC to get GND
boolean connectToVCC = pin.getName().contains("RST");
Copy link
Collaborator

@eddieh-xlnx eddieh-xlnx Jan 16, 2024

Choose a reason for hiding this comment

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

Actually, I'm beginning to think this is a little risky. You are connecting all module (not primitive) inputs containing RST to VCC which means you can't rely on all fanouts of this RST port to have inverters. It may even be driving LUTs.

I think it's safer to rely on the RouterHelper.invertPossibleGndPinsToVccPins() method (called by default during RWRoute.preprocess()) to make this transformation on a primitive level.

Co-authored-by: eddieh-xlnx <[email protected]>
Signed-off-by: Chris Lavin <[email protected]>
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