Skip to content

[Pack][Timing] Pre-Cluster Timing Analysis May Not Be Aware of Molecules #2972

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
AlexandreSinger opened this issue Apr 11, 2025 · 1 comment

Comments

@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Apr 11, 2025

The PreClusterDelayCalculator computes the max edge delay of a timing connection in the following function:

tatum::Time max_edge_delay(const tatum::TimingGraph& tg, tatum::EdgeId edge_id) const override {
tatum::NodeId src_node = tg.edge_src_node(edge_id);
tatum::NodeId sink_node = tg.edge_sink_node(edge_id);
auto edge_type = tg.edge_type(edge_id);
if (edge_type == tatum::EdgeType::PRIMITIVE_COMBINATIONAL) {
return prim_comb_delay(tg, src_node, sink_node);
} else if (edge_type == tatum::EdgeType::PRIMITIVE_CLOCK_LAUNCH) {
return prim_tcq_delay(tg, src_node, sink_node);
} else {
VTR_ASSERT(edge_type == tatum::EdgeType::INTERCONNECT);
//External net delay
return tatum::Time(inter_cluster_net_delay_);
}
}

All nets that connect atom blocks together are labelled as INTERCONNECT in the timing_graph_builder here:

//Walk through the netlist nets adding the edges representing each net to
//the timing graph. This connects the timing graph nodes of each netlist
//block together.
for (AtomNetId net : netlist_.nets()) {
add_net_to_timing_graph(net);
}

void TimingGraphBuilder::add_net_to_timing_graph(const AtomNetId net) {
//Create edges from the driver to sink tnodes
AtomPinId driver_pin = netlist_.net_driver(net);
if (!driver_pin) {
//Undriven nets have no timing dependencies, and hence no edges
VTR_LOG_WARN("Net %s has no driver and will be ignored for timing purposes\n", netlist_.net_name(net).c_str());
return;
}
NodeId driver_tnode = netlist_lookup_.atom_pin_tnode(driver_pin);
VTR_ASSERT(driver_tnode);
for (AtomPinId sink_pin : netlist_.net_sinks(net)) {
NodeId sink_tnode = netlist_lookup_.atom_pin_tnode(sink_pin);
VTR_ASSERT(sink_tnode);
tg_->add_edge(tatum::EdgeType::INTERCONNECT, driver_tnode, sink_tnode);
}
}

The timing graph is created before clustering. The prepacker then occurs after creating the timing graph, but before clustering. The prepacker will pre-cluster atoms together. These molecules will be observed by the packer (i.e. molecules will not get broken). Since the timing graph was created before the prepacker and it does not get updated after prepacking (as far as I am aware), this implies that the max edge delay computed by this calculator is not taking the prepacked molecules into account.

If a timing graph edge is between two atoms within the same molecule, the internal delay of that implemented molecule should be used instead of the inter_cluster_net_delay value.

Proposed next steps:

  1. Verify that the assumptions above is true by adding an assertion within the max_edge_delay. The PreClusterDelayCalculator has access to the prepacker, it can check if two atoms are in the same molecule.
  2. If we verify that this is correct, we should add a case to this method which will check if the edge is absorbed into a molecule and use a more accurate delay.
  3. Verify the pre-clustering timing report to ensure that the molecule delays are being observed.

See #2965 (comment) for context

@vaughnbetz Let me know if I am missing anything here.

@vaughnbetz
Copy link
Contributor

vaughnbetz commented Apr 11, 2025 via email

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

No branches or pull requests

2 participants