Skip to content

pad: add custom comparator for GridGraphEdge set to prevent non-determinism #10632

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.

Sign up for GitHub

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

Merged
eder-matheus merged 2 commits into from
Jun 11, 2026
+53 −43
Merged

pad: add custom comparator for GridGraphEdge set to prevent non-determinism #10632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f11f922
pad: add custom comparator for GridGraphEdge set to prevent non-deter…
eder-matheus Jun 10, 2026
f2ae0d8
pad: gemini reviews
eder-matheus Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions src/pad/src/RDLRouter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ namespace pad {

namespace {

// Order routes by their source terminal id so ripup iteration (and the edge
// re-add order it drives) is deterministic, not heap-address dependent.
struct RDLRoutePtrLess
{
bool operator()(const std::shared_ptr<RDLRoute>& lhs,
const std::shared_ptr<RDLRoute>& rhs) const
{
return lhs->getTerminal()->getId() < rhs->getTerminal()->getId();
}
};

bool compareRouteTargets(const RouteTarget& lhs, const RouteTarget& rhs)
{
if (lhs.center.x() != rhs.center.x()) {
Expand Down Expand Up @@ -381,7 +392,7 @@ void RDLRouter::route(const std::vector<odb::dbNet*>& nets)
makeGraph();

// Determine access points
std::unordered_map<odb::Point, std::set<GridGraphEdge>> remove_edges;
std::unordered_map<odb::Point, GridGraphEdgeSet> remove_edges;

@gemini-code-assist gemini-code-assist Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While fixing the non-determinism of GridGraphEdge sets is a great step, there is another source of non-determinism in RDLRouter::route at line 652:

std::set<RDLRoutePtr> ripup;

Since RDLRoutePtr is a std::shared_ptr<RDLRoute>, std::set will default to comparing the memory addresses of the shared pointers. This makes the iteration order of ripup (and consequently the order in which uncommitRoute and addGraphEdge are called) dependent on the heap layout, which varies across builds and runs.

To ensure complete determinism, ripup should use a custom comparator that orders the routes deterministically, for example by their terminal ID:

struct RDLRoutePtrLess
{
  bool operator()(const RDLRoutePtr& lhs, const RDLRoutePtr& rhs) const
  {
    return lhs->getTerminal()->getId() < rhs->getTerminal()->getId();
  }
};
std::set<RDLRoutePtr, RDLRoutePtrLess> ripup;
References
  1. Iteration over containers with pointer keys is non-deterministic unless a custom comparator is defined for the pointer type to ensure a stable, deterministic order.

for (auto& [net, iterm_targets] : routing_targets_) {
for (auto& [iterm, targets] : iterm_targets) {
for (auto& target : targets) {
Expand Down Expand Up @@ -649,7 +660,7 @@ void RDLRouter::route(const std::vector<odb::dbNet*>& nets)
}

// find routes to ripup
std::set<RDLRoutePtr> ripup;
std::set<RDLRoutePtr, RDLRoutePtrLess> ripup;
for (const auto& failed_route : failed) {
for (const auto& route : routes_) {
if (!route->isRouted()) {
Expand Down Expand Up @@ -797,7 +808,7 @@ static odb::Point getValidGridPoint(
}

void RDLRouter::cleanupGraphEdges(
const std::unordered_map<odb::Point, std::set<GridGraphEdge>>& edges)
const std::unordered_map<odb::Point, GridGraphEdgeSet>& edges)
{
if (edges.empty()) {
return;
Expand All @@ -824,7 +835,7 @@ void RDLRouter::cleanupGraphEdges(

void RDLRouter::populateTerminalAccessPoints(
RouteTarget& target,
std::unordered_map<odb::Point, std::set<GridGraphEdge>>& edges) const
std::unordered_map<odb::Point, GridGraphEdgeSet>& edges) const
{
// determine new access point in graph
std::set<odb::Point> snap_pts;
Expand Down Expand Up @@ -1181,29 +1192,24 @@ bool RDLRouter::is45DegreeEdge(const odb::Point& pt0,
return RDLRoute::is45DegreeEdge(pt0, pt1);
}

std::set<GridGraphEdge> RDLRouter::getVertexEdges(
const GridGraphVertex& vertex) const
GridGraphEdgeSet RDLRouter::getVertexEdges(const GridGraphVertex& vertex) const
{
std::set<GridGraphEdge> edges;
GridGraphEdgeSet edges;
Comment thread
eder-matheus marked this conversation as resolved.

// The graph is undirected, so out_edges already yields every incident edge.
GridGraph::out_edge_iterator oit, oend;
std::tie(oit, oend) = boost::out_edges(vertex, graph_);
for (; oit != oend; oit++) {
edges.insert(*oit);
}
GridGraph::in_edge_iterator iit, iend;
std::tie(iit, iend) = boost::in_edges(vertex, graph_);
for (; iit != iend; iit++) {
edges.insert(*iit);
}

return edges;
}

std::vector<RDLRouter::GridEdge> RDLRouter::commitRoute(
const std::vector<GridGraphVertex>& route)
{
std::set<GridGraphEdge> edges;
GridGraphEdgeSet edges;
for (const auto& v : route) {
const auto v_edges = getVertexEdges(v);
edges.insert(v_edges.begin(), v_edges.end());
Expand Down
19 changes: 16 additions & 3 deletions src/pad/src/RDLRouter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#pragma once

#include <algorithm>
#include <cstdint>
#include <map>
#include <memory>
Expand Down Expand Up @@ -67,6 +68,18 @@ using GridWeightMap
using GridGraphVertex = GridGraph::vertex_descriptor;
using GridGraphEdge = GridGraph::edge_descriptor;

// Order edges by (source, target) vertex indices; the descriptor's default
// operator< compares a heap pointer, giving build-dependent iteration.
struct GridGraphEdgeLess
{
bool operator()(const GridGraphEdge& lhs, const GridGraphEdge& rhs) const
{
return std::minmax(lhs.m_source, lhs.m_target)
< std::minmax(rhs.m_source, rhs.m_target);
}
};
Comment thread
eder-matheus marked this conversation as resolved.
using GridGraphEdgeSet = std::set<GridGraphEdge, GridGraphEdgeLess>;

struct RouteTarget
{
// center point of the target shape
Expand Down Expand Up @@ -213,11 +226,11 @@ class RDLRouter

void populateTerminalAccessPoints(
RouteTarget& target,
std::unordered_map<odb::Point, std::set<GridGraphEdge>>& edges) const;
std::unordered_map<odb::Point, GridGraphEdgeSet>& edges) const;
void cleanupTerminalAccessPoints(odb::dbITerm* iterm,
std::vector<RouteTarget>& targets) const;
void cleanupGraphEdges(
const std::unordered_map<odb::Point, std::set<GridGraphEdge>>& edges);
const std::unordered_map<odb::Point, GridGraphEdgeSet>& edges);
std::set<odb::Point> generateTerminalAccessPoints(const odb::Point& pt,
bool do_x) const;
TerminalAccess insertTerminalAccess(const RouteTarget& target,
Expand All @@ -227,7 +240,7 @@ class RDLRouter
odb::PtrMap<odb::dbITerm, std::vector<RouteTarget>> generateRoutingTargets(
odb::dbNet* net) const;
odb::dbTechLayer* getOtherLayer(odb::dbTechVia* via) const;
std::set<GridGraphEdge> getVertexEdges(const GridGraphVertex& vertex) const;
GridGraphEdgeSet getVertexEdges(const GridGraphVertex& vertex) const;

void buildIntialRouteSet();
int reportFailedRoutes(
Expand Down
44 changes: 18 additions & 26 deletions src/pad/test/rdl_route_failed.ok
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,45 @@
[INFO PAD-0037] End of routing iteration 2: 85.4% complete
[INFO PAD-0037] End of routing iteration 3: 81.9% complete
[INFO PAD-0037] End of routing iteration 4: 84.4% complete
[INFO PAD-0037] End of routing iteration 5: 89.0% complete
[INFO PAD-0037] End of routing iteration 6: 89.9% complete
[INFO PAD-0037] End of routing iteration 7: 89.5% complete
[INFO PAD-0037] End of routing iteration 8: 89.0% complete
[INFO PAD-0037] End of routing iteration 9: 88.8% complete
[INFO PAD-0037] End of routing iteration 10: 88.8% complete
[WARNING PAD-0006] Failed to route the following 17 nets:
[INFO PAD-0037] End of routing iteration 5: 89.2% complete
[INFO PAD-0037] End of routing iteration 6: 90.1% complete
[INFO PAD-0037] End of routing iteration 7: 90.3% complete
[INFO PAD-0037] End of routing iteration 8: 90.3% complete
[INFO PAD-0037] End of routing iteration 9: 89.2% complete
[INFO PAD-0037] End of routing iteration 10: 90.3% complete
[WARNING PAD-0006] Failed to route the following 13 nets:
DVDD
BUMP_11_3/PAD -> u_v18_6/DVDD, u_v18_7/DVDD, u_v18_5/DVDD, u_v18_4/DVDD, u_v18_8/DVDD, ... (67 possible terminals)
BUMP_16_11/PAD -> u_v18_15/DVDD, u_v18_16/DVDD, u_v18_14/DVDD, u_v18_13/DVDD, u_v18_12/DVDD, ... (67 possible terminals)
BUMP_1_13/PAD -> u_v18_26/DVDD, u_v18_27/DVDD, u_v18_28/DVDD, u_v18_25/DVDD, u_v18_29/DVDD, ... (67 possible terminals)
BUMP_4_1/PAD -> u_v18_1/DVDD, u_v18_2/DVDD, u_v18_0/DVDD, u_v18_3/DVDD, u_v18_4/DVDD, ... (67 possible terminals)
DVSS
BUMP_0_8/PAD -> u_vzz_30/DVSS, u_vzz_29/DVSS, u_vzz_31/DVSS, u_vzz_32/DVSS, u_vzz_28/DVSS, ... (67 possible terminals)
BUMP_15_11/PAD -> u_vzz_15/DVSS, u_vzz_16/DVSS, u_vzz_14/DVSS, u_vzz_13/DVSS, u_vzz_12/DVSS, ... (67 possible terminals)
BUMP_15_7/PAD -> u_vzz_12/DVSS, u_vzz_13/DVSS, u_vzz_11/DVSS, u_vzz_14/DVSS, u_vzz_10/DVSS, ... (67 possible terminals)
BUMP_1_9/PAD -> u_vzz_29/DVSS, u_vzz_30/DVSS, u_vzz_28/DVSS, u_vzz_31/DVSS, u_vzz_27/DVSS, ... (67 possible terminals)
VDD
BUMP_11_12/PAD -> u_vdd_9/VDD, u_vdd_pll/VDD, u_vdd_8/VDD, u_vdd_7/VDD, u_vdd_10/VDD, ... (33 possible terminals)
BUMP_1_10/PAD -> u_vdd_13/VDD, u_vdd_14/VDD, u_vdd_12/VDD, u_vdd_15/VDD, u_vdd_11/VDD, ... (33 possible terminals)
VSS
BUMP_0_10/PAD -> u_vss_13/VSS, u_vss_14/VSS, u_vss_12/VSS, u_vss_15/VSS, u_vss_11/VSS, ... (33 possible terminals)
BUMP_14_9/PAD -> u_vss_6/VSS, u_vss_5/VSS, u_vss_7/VSS, u_vss_4/VSS, u_vss_8/VSS, ... (33 possible terminals)
BUMP_16_6/PAD -> u_vss_5/VSS, u_vss_6/VSS, u_vss_4/VSS, u_vss_7/VSS, u_vss_3/VSS, ... (33 possible terminals)
p_bsg_tag_data_o
BUMP_2_10/PAD -> u_bsg_tag_data_o/PAD
p_clk_A_i
BUMP_7_13/PAD -> u_clk_A_i/PAD
p_clk_async_reset_i
BUMP_9_14/PAD -> u_clk_async_reset_i/PAD
p_co2_v_o
BUMP_0_13/PAD -> u_co2_v_o/PAD
p_ddr_addr_15_o
BUMP_4_0/PAD -> u_ddr_addr_15_o/PAD
p_ddr_addr_2_o
BUMP_8_2/PAD -> u_ddr_addr_2_o/PAD
p_ddr_addr_6_o
BUMP_7_0/PAD -> u_ddr_addr_6_o/PAD
p_ddr_addr_7_o
BUMP_7_1/PAD -> u_ddr_addr_7_o/PAD
p_ddr_addr_4_o
BUMP_7_4/PAD -> u_ddr_addr_4_o/PAD
p_ddr_ck_n_o
BUMP_11_4/PAD -> u_ddr_ck_n_o/PAD
p_ddr_dq_0_io
BUMP_3_7/PAD -> u_ddr_dq_0_io/PAD
p_ddr_dq_11_io
BUMP_4_5/PAD -> u_ddr_dq_11_io/PAD
p_ddr_dq_13_io
BUMP_3_5/PAD -> u_ddr_dq_13_io/PAD
p_ddr_dq_2_io
BUMP_1_8/PAD -> u_ddr_dq_2_io/PAD
p_ddr_dq_7_io
BUMP_4_10/PAD -> u_ddr_dq_7_io/PAD
p_ddr_dq_8_io
BUMP_0_4/PAD -> u_ddr_dq_8_io/PAD
p_ddr_dq_9_io
BUMP_1_4/PAD -> u_ddr_dq_9_io/PAD
[ERROR PAD-0007] Failed to route 17 nets.
[ERROR PAD-0007] Failed to route 13 nets.
pass
1 change: 0 additions & 1 deletion src/pad/test/rdl_route_max_iterations.ok
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
[WARNING PAD-0006] Failed to route the following 18 nets:
DVDD
BUMP_11_13/PAD -> u_v18_19/DVDD, u_v18_20/DVDD, u_v18_18/DVDD, u_v18_21/DVDD, u_v18_17/DVDD, ... (67 possible terminals)
BUMP_7_3/PAD -> u_v18_3/DVDD, u_v18_4/DVDD, u_v18_2/DVDD, u_v18_5/DVDD, u_v18_1/DVDD, ... (67 possible terminals)
VSS
BUMP_0_7/PAD -> u_vss_14/VSS, u_vss_13/VSS, u_vss_15/VSS, u_vss_12/VSS, u_vss_0/VSS, ... (33 possible terminals)
BUMP_10_13/PAD -> u_vss_9/VSS, u_vss_pll/VSS, u_vss_10/VSS, u_vss_8/VSS, u_vss_7/VSS, ... (33 possible terminals)
Expand Down
Loading