OpenROAD icon indicating copy to clipboard operation
OpenROAD copied to clipboard
verified publisher icon The-OpenROAD-Project

odb, dbSta, rsz: Produced wrong timing edges after netlist editing in hierarchical flow

Open jhkim-pii opened this issue 2 weeks ago • 0 comments

Describe the bug

Resizer has multiple buffering logics here and there (e.g., rebuffer, makeRepeater, splitLoadMove, cloneMove, ...). In a hierarchical flow, I found that those buffering logics can break timing edges in STA data structure. This is true for the new insert buffer logic under development.

When resizer edits a netlist, multiple APIs work in a tightly coupled manner.

  • Buffering logic in Resizer that directly edits the ODB. e.g., Rebuffer::fullyRebuffer, RepairDesign::makeRepeater, ...
  • ODB callbacks for the netlist change notification. e.g., dbStaCbk::inDbITermPreDisconnect(), dbStaCbk::inDbITermDestroy() ...
  • STA callbacks for the STA data structure update. e.g., dbNetwork::connectPinAfter(), dbNetwork::disconnectPinBefore(), ...
  • STA has internal caches for performance and they are updated when the netlist is being traversed. So dbNetwork APIs are also closely related. e.g., dbNetwork::net(Pin*), dbNetwork::pin(Term*), dbNetwork::staToDb(), ...

So they should be modified carefully not to cause any inconsistency b/w ODB and STA data structures.

But what makes things more complicated is that the data structures for nets used by STA and ODB are different.

ODB

  • There are flat net (dbNet) and hierarchical net (dbModNet).

STA

  • Net is basically a hierarchical net of ODB.
  • For performance, STA has internal caches and they can mimic the flat net behavior of ODB (e.g., quickly find the driver pin of a net)

In STA domain, a simple wire disconnection operation triggers a single disconnectPinBefore() callback. But in ODB, the same wire disconnection operation consists of two disconnections of flat and hierarchical nets, which leads to two disconnectPinBefore() callbacks in STA.

If the disconnection of flat and hier net sequences are changed, STA callback sequence changes and it can produce a wrong entries in cache (e.g., net_drvr_pin_map_ cache). When STA builds a timing graph, it uses the net_drvr_pin_map_ cache. So the wrong entries in net_drvr_pin_map_ cache can produce a wrong timing graph, which is a critical issue

e.g.,

  • Duplicated timing edge
  • Missing timing edge
  • Wrong timing edge b/w incorrect driver and load pin ...

The wrong timing edge can cause crash or mal-function including incorrect QoR reporting in hierarchical flow.

Expected Behavior

No crash. No error. No incorrect timing edges in STA. Consistent ODB and STA data structures.

Environment

-- The CXX compiler identification is GNU 13.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- OpenROAD version: v2.0-26758-g6db0147224
-- System name: Linux
-- Compiler: GNU 13.3.0
-- Build type: DEBUG
-- Install prefix: /usr/local
-- C++ Standard: 20
-- C++ Standard Required: ON
-- C++ Extensions: OFF
-- The C compiler identification is GNU 13.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Performing Test C_COMPILER_SUPPORTS__-Wall
-- Performing Test C_COMPILER_SUPPORTS__-Wall - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wall
-- Performing Test CXX_COMPILER_SUPPORTS__-Wall - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-array-bounds
-- Performing Test C_COMPILER_SUPPORTS__-Wno-array-bounds - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-array-bounds
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-array-bounds - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-nonnull
-- Performing Test C_COMPILER_SUPPORTS__-Wno-nonnull - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-nonnull
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-nonnull - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-maybe-uninitialized
-- Performing Test C_COMPILER_SUPPORTS__-Wno-maybe-uninitialized - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-maybe-uninitialized
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-maybe-uninitialized - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format-overflow
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format-overflow - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format-overflow
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format-overflow - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-variable
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-variable - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-variable
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-variable - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-function
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-function - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-function
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-function - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-write-strings
-- Performing Test C_COMPILER_SUPPORTS__-Wno-write-strings - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-write-strings
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-write-strings - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-sign-compare
-- Performing Test C_COMPILER_SUPPORTS__-Wno-sign-compare - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-sign-compare
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-sign-compare - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-deprecated
-- Performing Test C_COMPILER_SUPPORTS__-Wno-deprecated - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-deprecated
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-deprecated - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-c++11-narrowing
-- Performing Test C_COMPILER_SUPPORTS__-Wno-c++11-narrowing - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-c++11-narrowing
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-c++11-narrowing - Failed
-- Performing Test C_COMPILER_SUPPORTS__-Wno-register
-- Performing Test C_COMPILER_SUPPORTS__-Wno-register - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-register
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-register - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format
-- Performing Test C_COMPILER_SUPPORTS__-Wno-format - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-format - Success
-- Performing Test C_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal
-- Performing Test C_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-reserved-user-defined-literal - Failed
-- Performing Test C_COMPILER_SUPPORTS__-fpermissive
-- Performing Test C_COMPILER_SUPPORTS__-fpermissive - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-fpermissive
-- Performing Test CXX_COMPILER_SUPPORTS__-fpermissive - Success
-- Performing Test C_COMPILER_SUPPORTS__-x
-- Performing Test C_COMPILER_SUPPORTS__-x - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-x
-- Performing Test CXX_COMPILER_SUPPORTS__-x - Failed
-- Performing Test C_COMPILER_SUPPORTS__c++
-- Performing Test C_COMPILER_SUPPORTS__c++ - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__c++
-- Performing Test CXX_COMPILER_SUPPORTS__c++ - Failed
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-but-set-variable
-- Performing Test C_COMPILER_SUPPORTS__-Wno-unused-but-set-variable - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-but-set-variable
-- Performing Test CXX_COMPILER_SUPPORTS__-Wno-unused-but-set-variable - Success
-- Performing Test C_COMPILER_SUPPORTS__-std=c++17
-- Performing Test C_COMPILER_SUPPORTS__-std=c++17 - Failed
-- Performing Test CXX_COMPILER_SUPPORTS__-std=c++17
-- Performing Test CXX_COMPILER_SUPPORTS__-std=c++17 - Success
-- Performing Test C_COMPILER_SUPPORTS__-fno-exceptions
-- Performing Test C_COMPILER_SUPPORTS__-fno-exceptions - Success
-- Performing Test CXX_COMPILER_SUPPORTS__-fno-exceptions
-- Performing Test CXX_COMPILER_SUPPORTS__-fno-exceptions - Success
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- TCL readline library: /usr/lib/x86_64-linux-gnu/libtclreadline.so
-- TCL readline header: /usr/include/x86_64-linux-gnu
-- Found SWIG: /usr/local/bin/swig (found suitable version "4.3.0", minimum required is "4.0")
-- Using SWIG >= 4.3.0 -flatstaticmethod flag for python
-- boost: 1.86.0
-- Found GTest: /usr/local/lib/cmake/GTest/GTestConfig.cmake (found version "1.13.0")
-- GTest: 1.13.0
-- Found Python3: /usr/include/python3.12 (found version "3.12.3") found components: Development Development.Module Development.Embed
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.3")
-- Found Threads: TRUE
-- spdlog: 1.15.0
-- Found BISON: /usr/bin/bison (found version "3.8.2")
-- Found Doxygen: /usr/bin/doxygen (found version "1.9.8") found components: doxygen dot
-- STA version: 2.7.0
-- STA git sha: 9c9b5659d6a7ecbe02ea1204aa89079a77db1d3e
-- System name: Linux
-- Compiler: GNU 13.3.0
-- Build type: DEBUG
-- Build CXX_FLAGS: -g
-- Install prefix: /usr/local
-- Found FLEX: /usr/bin/flex (found version "2.6.4")
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- TCL readline library: /usr/lib/x86_64-linux-gnu/libtclreadline.so
-- TCL readline header: /usr/include/x86_64-linux-gnu/tclreadline.h
-- CUDD library: /usr/local/lib/libcudd.a
-- CUDD header: /usr/local/include/cudd.h
-- SSTA: 0
-- Found SWIG: /usr/local/bin/swig (found suitable version "4.3.0", minimum required is "3.0")
-- STA library: /workspace/clean/OpenROAD-flow-scripts/tools/OpenROAD/build/libOpenSTA.a
-- STA executable: /workspace/clean/OpenROAD-flow-scripts/tools/OpenROAD/build/sta
-- Found re2: /opt/or-tools/lib/cmake/re2/re2Config.cmake (found version "11.0.0")
-- Found Clp: /opt/or-tools/lib/cmake/Clp/ClpConfig.cmake (found version "1.17.7")
-- Found Cbc: /opt/or-tools/lib/cmake/Cbc/CbcConfig.cmake (found version "2.10.7")
-- Found SCIP: /opt/or-tools/lib/cmake/scip/scip-config.cmake (found version "9.0.0")
-- Found OpenMP_CXX: -fopenmp (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
-- Found OR-Tools: /opt/or-tools/lib/cmake/ortools (version: 9.11.4210)
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- Found OpenMP_C: -fopenmp (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
-- GUI is enabled
-- Could NOT find VTune (missing: VTune_LIBRARIES VTune_INCLUDE_DIRS) 
-- TCL library: /usr/lib/x86_64-linux-gnu/libtcl.so
-- TCL header: /usr/include/tcl/tcl.h
-- Found Eigen3: /usr/local/share/eigen3/cmake/Eigen3Config.cmake (found version "3.4.1")
-- TCL readline enabled
-- Tcl Extended disabled
-- Python3 enabled
-- Configuring done (3.0s)
-- Generating done (0.6s)

To Reproduce

Adding the following APIs in dbSta and call it after Resizer operation. If there is any inconsistency b/w ODB and STA, it will report WARNING or ERROR. Currently, if hierarchical flow is enabled (OPENROAD_HIERARCHICAL=1), many designs report the inconsistency issue.

int dbSta::checkSanity()
{
  ensureGraph();
  ensureLevelized();

  int pre_warn_cnt = logger_->getWarningCount();

  checkSanityDrvrVertexEdges();

  int post_warn_cnt = logger_->getWarningCount();
  return post_warn_cnt - pre_warn_cnt;
}

void dbSta::checkSanityDrvrVertexEdges(const odb::dbObject* term) const
{
  if (term == nullptr) {
    logger_->error(
        utl::STA, 2056, "checkSanityDrvrVertexEdges: input term is null");
    return;
  }

  sta::Pin* pin = db_network_->dbToSta(const_cast<odb::dbObject*>(term));
  if (pin == nullptr) {
    logger_->error(utl::STA,
                   2057,
                   "checkSanityDrvrVertexEdges: failed to convert dbObject to "
                   "sta::Pin for {}",
                   term->getName());
    return;
  }

  checkSanityDrvrVertexEdges(pin);
}

void dbSta::checkSanityDrvrVertexEdges(const Pin* pin) const
{
  if (db_network_->isDriver(pin) == false) {
    return;
  }

  sta::Graph* graph = this->graph();
  sta::Vertex* drvr_vertex = graph->pinDrvrVertex(pin);

  if (drvr_vertex == nullptr) {
    logger_->warn(utl::STA,
                  2058,
                  "checkSanityDrvrVertexEdges: could not find driver vertex "
                  "for pin {}",
                  db_network_->pathName(pin));
    return;
  }

  std::set<sta::Vertex*> visited_to_vertices;
  sta::VertexOutEdgeIterator edge_iter(drvr_vertex, graph);
  while (edge_iter.hasNext()) {
    sta::Edge* edge = edge_iter.next();
    sta::Vertex* to_vertex = edge->to(graph);

    debugPrint(logger_,
               utl::RSZ,
               "insert_buffer_check_sanity",
               10,
               "    Edge from {} to {}",
               drvr_vertex->to_string(this).c_str(),
               to_vertex->to_string(this).c_str());

    if (visited_to_vertices.find(to_vertex) != visited_to_vertices.end()) {
      logger_->error(utl::STA,
                     2059,
                     "Duplicate edge found from {} to {}",
                     drvr_vertex->to_string(this).c_str(),
                     to_vertex->to_string(this).c_str());
    }
    visited_to_vertices.insert(to_vertex);
  }

  // Compare with ODB connectivity
  bool has_inconsistency = false;
  odb::dbObject* drvr_obj = db_network_->staToDb(pin);
  odb::dbNet* net = nullptr;
  if (drvr_obj) {
    if (drvr_obj->getObjectType() == odb::dbITermObj) {
      net = static_cast<odb::dbITerm*>(drvr_obj)->getNet();
    } else if (drvr_obj->getObjectType() == odb::dbBTermObj) {
      net = static_cast<odb::dbBTerm*>(drvr_obj)->getNet();
    }
  }

  if (net) {
    std::set<sta::Pin*> odb_loads;
    for (odb::dbITerm* iterm : net->getITerms()) {
      if (iterm != drvr_obj) {
        odb_loads.insert(db_network_->dbToSta(iterm));
      }
    }
    for (odb::dbBTerm* bterm : net->getBTerms()) {
      if (bterm != drvr_obj) {
        odb_loads.insert(db_network_->dbToSta(bterm));
      }
    }

    std::set<sta::Pin*> sta_loads;
    for (sta::Vertex* to_vertex : visited_to_vertices) {
      sta_loads.insert(to_vertex->pin());
    }

    for (sta::Pin* sta_load : sta_loads) {
      if (odb_loads.find(sta_load) == odb_loads.end()) {
        logger_->warn(utl::STA,
                      2062,
                      "Inconsistent load: STA has load '{}', but ODB does not.",
                      db_network_->pathName(sta_load));
        has_inconsistency = true;
      }
    }

    for (sta::Pin* odb_load : odb_loads) {
      if (sta_loads.find(odb_load) == sta_loads.end()) {
        logger_->warn(utl::STA,
                      2063,
                      "Inconsistent load: ODB has load '{}', but STA does not.",
                      db_network_->pathName(odb_load));
        has_inconsistency = true;
      }
    }
  }

  if (has_inconsistency) {
    logger_->error(utl::STA,
                   2064,
                   "Inconsistencies found in driver vertex edges for pin {}.",
                   db_network_->pathName(pin));
  }
}

void dbSta::checkSanityDrvrVertexEdges() const
{
  // Iterate over all driver vertices in the graph.
  sta::VertexIterator vertex_iter(this->graph());
  while (vertex_iter.hasNext()) {
    sta::Vertex* vertex = vertex_iter.next();
    if (vertex->isDriver(this->network())) {
      checkSanityDrvrVertexEdges(db_network_->staToDb(vertex->pin()));
    }
  }
}

Relevant log output

Screenshots

No response

Additional Context

No response

jhkim-pii avatar Dec 10 '25 06:12 jhkim-pii