Skip to content

Conversation

@AcKoucher
Copy link
Contributor

@AcKoucher AcKoucher commented Mar 21, 2025

This PR changes the approach w.r.t. how we handle pads in macro placement.

Goals

  1. Ensure that pad instances are correctly mapped to their cluster instead of being mapped to the root cluster.
  2. Ensure that PAD clusters' connections created from the physical nets (netlist connections) are properly considered.

Needed Context: MPL Nets

In MPL, the nets between clusters are made based on the netlist connections (see ClusteringEngine::buildNetListConnections()) - i.e., odb nets - along with an additional layer of connectivity based on the data flow structure (see ClusteringEngine::buildDataFlowConnections()) that we create at the beginning of the clustering process.

Current Approach

We create one cluster for each pad and map the bterm associated with that pad to the cluster. However, we don't map the instance itself to the cluster. Also, with the current approach, the pad clusters are created based on the data flow data structure in order to avoid creating clusters for pads that have no connection with the core. This requires:

  • Using the bterm associated with the pad as the vertex when creating the data flow data structure (the path of that pad will be treated as an io path);
  • Mapping pads and bterms.

With this, the connections are being built wrongly:

NetList Connections

  • For a net with a bterm, the cluster of the bterm is correctly identified as a PAD Cluster. However, this net is useless cluster connection wise, because it only connects the bterm to the pad.

  • For a net with a pad instance, the cluster of the instance is incorrectly identified as the root cluster. This creates a wrong connection between the source cluster and the root.

DataFlow Connections

As the data flow data structure is created based on a hypergraph whose vertices are adapted to be bterms instead of the actual pad instances, the connections generated based on it are apparently correct and are responsible for the bundled nets that we see on the debug mode today.

New Approach

We create one cluster for each pad, map the pad instance to the cluster and don't use the bterm. PAD Clusters are created based on the list of pads in the block, so all pads have clusters. With this approach we can get rid of the bterm and pads mapping and also simplify the data flow creation by just assigning vertices to instances (i.e., designs with pads have no IO vertices). This requires:

  • Ensuring the clustering process won't mess up the association of the pad instance -> PAD cluster.

With this, the connections should be properly considered:

NetList Connections

  • Nets with bterms are skipped when we have pads.
  • For a net with a pad instance, the cluster of the pad is identified as its PAD Cluster.

DataFlow Connections

There's no need to adapt vertices to be bterms instead of the actual pad instances. We just mark pad vertices as stoppers as they were registers or macro pins and go on.

Important: A possible future enhancement would be to only create PAD clusters for PADs with connections to the core using the new approach. I didn't include this change here, because I thought that this PR was already quite complicated conceptually.

@AcKoucher AcKoucher changed the title mpl: fix pad instances mapping and consider macro-to-pad data flow connections mpl: fix pad clusters' connections Mar 21, 2025
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

    1) Associate instance instead of bterm to pad cluster
    2) Ensure that the previously mentioned association is not undone in
       autoclustering process.
    3) Use the pad instance as the actual vertex in the hypergraph
       for the data flow data structure
    4) Skip bterms when updating connections based on odb net
    5) Create pad clusters for all pads

Signed-off-by: Arthur Koucher <[email protected]>
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher AcKoucher marked this pull request as ready for review May 26, 2025 20:14
@AcKoucher AcKoucher requested a review from maliberty May 26, 2025 20:14
@AcKoucher
Copy link
Contributor Author

Running Secure-CI.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

     PADs in the design when computing connections.

Signed-off-by: Arthur Koucher <[email protected]>
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher
Copy link
Contributor Author

AcKoucher commented May 27, 2025

Secure-CI showed a failure in mock-array which led me to find a bug in my implementation.
I fixed it and I'm running a new Secure-CI.
This should be ready for review.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@maliberty
Copy link
Member

Just waiting on "I fixed it and I'm running a new Secure-CI."

@AcKoucher
Copy link
Contributor Author

There's a GRT high congestion failure in a private design which I'm investigating. It doesn't seem strictly related to the macro placement itself.

@AcKoucher
Copy link
Contributor Author

As discussed in the R&D synch, the changes here are waiting on @arthurjolo 's CTS enhancements so we can re-evaluate repair_timing results.

@AcKoucher
Copy link
Contributor Author

Re-running Secure-CI now that #7532 was merged.

@AcKoucher
Copy link
Contributor Author

Secure-CI passed.

@eder-matheus eder-matheus enabled auto-merge June 13, 2025 19:42
@eder-matheus eder-matheus merged commit d5e2cb7 into The-OpenROAD-Project:master Jun 13, 2025
11 checks passed
@AcKoucher AcKoucher deleted the mpl-pad-inst-association branch June 13, 2025 19:47
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.

3 participants