Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
79 changes: 76 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ on:
pull_request:

jobs:
build:
# Primary native Linux build used for install artifacts.
linux-ubuntu:
if: github.repository_owner != 'The-OpenROAD-Project-private'
runs-on: ubuntu-latest
steps:
Expand Down Expand Up @@ -51,7 +52,7 @@ jobs:
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: ${{ !cancelled() }}
with:
name: artifact
name: artifact-ubuntu
path: |
build/install/*
retention-days: 1
Expand All @@ -60,6 +61,78 @@ jobs:
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: ${{ !cancelled() }}
with:
name: result
name: result-ubuntu
path: |
test/results/*

# CentOS 7 Docker: old platform still common in EDA environments.
linux-centos:
if: github.repository_owner != 'The-OpenROAD-Project-private'
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
submodules: true

- name: Build image
run: docker build -f Dockerfile.centos7 --tag sta-centos7 .

- name: Test in container
run: |
mkdir -p test/results
docker run --rm \
-v "$PWD/test/results:/OpenSTA/test/results" \
--entrypoint /bin/bash sta-centos7 \
-c "cd /OpenSTA/test && ./regression || (cat results/diffs && exit 1)"

- name: Upload Test Result
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: ${{ !cancelled() }}
with:
name: result-centos7
path: |
test/results/*

# Native macOS coverage via Brewfile.
macos:
if: github.repository_owner != 'The-OpenROAD-Project-private'
runs-on: macos-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
submodules: true

- name: Install dependencies
run: brew bundle

- name: Build
run: |
eval "$(/opt/homebrew/bin/brew shellenv)"
export PATH="$(brew --prefix flex)/bin:$PATH"
export PATH="$(brew --prefix bison)/bin:$PATH"
export TCL_LIBRARY="$(brew --prefix tcl-tk@8)/lib/libtcl8.6.dylib"
export TCL_INCLUDE_PATH="$(brew --prefix tcl-tk@8)/include/tcl-tk"
export FLEX_INCLUDE_DIR="$(brew --prefix flex)/include"
cmake -S . -B build \
-D TCL_LIBRARY=$TCL_LIBRARY \
-D TCL_INCLUDE_PATH=$TCL_INCLUDE_PATH \
-D FLEX_INCLUDE_DIR=$FLEX_INCLUDE_DIR \
-DCMAKE_BUILD_TYPE=Release
cmake --build build -j "$(sysctl -n hw.ncpu)"

- name: Test
run: |
cd test
./regression || (cat results/diffs && exit 1)

- name: Upload Test Result
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
if: ${{ !cancelled() }}
with:
name: result-macos
path: |
test/results/*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the rename?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

macOS default filesystem is case-insensitive, so top-level file BUILD collides with CMake build/ dir - mkdir build fails / build dir creation clashes. BUILD.bazel is Bazel's equally-valid canonical name, so Bazel keeps working while build/ becomes creatable.

File renamed without changes.
3 changes: 2 additions & 1 deletion Brewfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ brew "bison"
brew "cmake"
brew "eigen"
brew "flex"
brew "googletest"
brew "swig"
brew "tcl-tk@8"

tap "mht208/formal"
brew "mht208/formal/cudd"
brew "mht208/formal/cudd", trusted: true
10 changes: 6 additions & 4 deletions Dockerfile.centos7
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ RUN sed -i s/mirror.centos.org/vault.centos.org/g /etc/yum.repos.d/*.repo \
&& yum install -y devtoolset-11 git wget cmake3 make eigen3-devel tcl swig3 flex zlib-devel valgrind \
&& yum clean -y all

RUN ln -sf /usr/bin/cmake3 /usr/bin/cmake
RUN ln -sf /usr/bin/cmake3 /usr/bin/cmake && \
ln -sf /usr/bin/ctest3 /usr/bin/ctest

# Download Bison
RUN wget https://ftp.gnu.org/gnu/bison/bison-3.8.2.tar.gz && \
RUN wget https://mirrors.kernel.org/gnu/bison/bison-3.8.2.tar.gz && \
tar -xvf bison-3.8.2.tar.gz && \
rm bison-3.8.2.tar.gz

Expand All @@ -42,8 +43,9 @@ RUN source /opt/rh/devtoolset-11/enable && \
make install

# Download TCL
RUN wget http://prdownloads.sourceforge.net/tcl/tcl8.6.16-src.tar.gz && \
tar -xvf tcl8.6.16-src.tar.gz && \
RUN wget https://github.com/tcltk/tcl/archive/refs/tags/core-8-6-16.tar.gz -O tcl8.6.16-src.tar.gz && \
mkdir -p tcl8.6.16 && \
tar -xvf tcl8.6.16-src.tar.gz -C tcl8.6.16 --strip-components=1 && \
rm tcl8.6.16-src.tar.gz

# Build TCL
Expand Down
124 changes: 59 additions & 65 deletions dcalc/GraphDelayCalc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ GraphDelayCalc::delaysInvalid()
debugPrint(debug_, "delay_calc", 1, "delays invalid");
delays_exist_ = false;
delays_seeded_ = false;
incremental_ = false;
iter_->clear();
// No need to keep track of incremental updates any more.
invalid_delays_.clear();
Expand All @@ -224,7 +223,7 @@ GraphDelayCalc::delaysInvalid()
void
GraphDelayCalc::delayInvalid(const Pin *pin)
{
if (graph_ && incremental_) {
if (graph_ && delays_exist_) {
if (network_->isHierarchical(pin)) {
EdgesThruHierPinIterator edge_iter(pin, network_, graph_);
while (edge_iter.hasNext()) {
Expand All @@ -248,7 +247,7 @@ GraphDelayCalc::delayInvalid(Vertex *vertex)
{
debugPrint(debug_, "delay_calc", 2, "delay invalid {}",
vertex->to_string(this));
if (incremental_) {
if (delays_exist_) {
invalid_delays_.insert(vertex);
// Invalidate driver that triggers dcalc for multi-driver nets.
MultiDrvrNet *multi_drvr = multiDrvrNet(vertex);
Expand All @@ -274,7 +273,7 @@ void
GraphDelayCalc::deleteVertexBefore(Vertex *vertex)
{
iter_->deleteVertexBefore(vertex);
if (incremental_)
if (delays_exist_)
invalid_delays_.erase(vertex);
MultiDrvrNet *multi_drvr = multiDrvrNet(vertex);
if (multi_drvr) {
Expand Down Expand Up @@ -329,7 +328,7 @@ FindVertexDelays::copy() const
void
FindVertexDelays::visit(Vertex *vertex)
{
graph_delay_calc_->findVertexDelay(vertex, arc_delay_calc_, true);
graph_delay_calc_->findVertexDelay(vertex, arc_delay_calc_);
}

// The logical structure of incremental delay calculation closely
Expand All @@ -348,7 +347,7 @@ GraphDelayCalc::findDelays(Level level)
}
else
iter_->ensureSize();
if (incremental_)
if (delays_exist_)
seedInvalidDelays();

if (!iter_->empty()) {
Expand All @@ -367,7 +366,6 @@ GraphDelayCalc::findDelays(Level level)
invalid_latch_edges_.clear();

delays_exist_ = true;
incremental_ = true;
debugPrint(debug_, "delay_calc", 1, "found {} delays", dcalc_count);
stats.report("Delay calc");
}
Expand Down Expand Up @@ -674,13 +672,12 @@ GraphDelayCalc::findInputArcDelay(const Pin *drvr_pin,
void
GraphDelayCalc::findDelays(Vertex *drvr_vertex)
{
findVertexDelay(drvr_vertex, arc_delay_calc_, true);
findVertexDelay(drvr_vertex, arc_delay_calc_);
}

void
GraphDelayCalc::findVertexDelay(Vertex *vertex,
ArcDelayCalc *arc_delay_calc,
bool propagate)
ArcDelayCalc *arc_delay_calc)
{
const Pin *pin = vertex->pin();
debugPrint(debug_, "delay_calc", 2, "find delays {} ({})",
Expand All @@ -692,32 +689,38 @@ GraphDelayCalc::findVertexDelay(Vertex *vertex,
|| (vertex->isBidirectDriver()
&& network_->isTopLevelPort(pin)))
seedRootSlew(vertex, arc_delay_calc);
else {
if (network_->isLeaf(pin)) {
if (vertex->isDriver(network_)) {
LoadPinIndexMap load_pin_index_map = makeLoadPinIndexMap(vertex);
DrvrLoadSlews load_slews_prev;
if (incremental_)
load_slews_prev = loadSlews(load_pin_index_map);
findDriverDelays(vertex, arc_delay_calc, load_pin_index_map);
if (propagate) {
if (network_->direction(pin)->isInternal())
enqueueTimingChecksEdges(vertex);
// Enqueue adjacent vertices even if the load slews did not
// change when non-incremental to stride past annotations.
if (!incremental_
|| loadSlewsChanged(load_slews_prev, load_pin_index_map))
iter_->enqueueFanout(vertex);
}
}
else {
// Load vertex.
enqueueTimingChecksEdges(vertex);
// Enqueue driver vertices from this input load.
if (propagate)
iter_->enqueueFanout(vertex);
}
}
else if (network_->isLeaf(pin)
&& vertex->isDriver(network_)) {
LoadPinIndexMap load_pin_index_map = makeLoadPinIndexMap(vertex);
DrvrLoadSlews load_slews_prev;
if (delays_exist_)
load_slews_prev = loadSlews(load_pin_index_map);
findDriverDelays(vertex, arc_delay_calc, load_pin_index_map);
if (network_->direction(pin)->isInternal())
enqueueCheckEdges(vertex);
graph_->visitFanouts(vertex, search_non_latch_pred_,
[this, &load_slews_prev, &load_pin_index_map]
(Vertex *fanout) {
// Enqueue adjacent vertices even if the load slew
// did not change when non-incremental to stride
// past annotations.
if (!delays_exist_
|| loadSlewChanged(fanout, load_slews_prev,
load_pin_index_map)) {
iter_->enqueue(fanout);
fanout->setBfsPredecessorChanged(true);
}
});
}
else if (vertex->isLoad(network_)) {
// Load vertex.
// Includes top level bidirect load vertex with wire edge to bidirect driver.
enqueueCheckEdges(vertex);
graph_->visitFanouts(vertex, search_non_latch_pred_,
[this] (Vertex *fanout) {
iter_->enqueue(fanout);
fanout->setBfsPredecessorChanged(true);
});
}
}

Expand All @@ -737,24 +740,25 @@ GraphDelayCalc::loadSlews(LoadPinIndexMap &load_pin_index_map)
}

bool
GraphDelayCalc::loadSlewsChanged(DrvrLoadSlews &load_slews_prev,
LoadPinIndexMap &load_pin_index_map)
{
GraphDelayCalc::loadSlewChanged(Vertex *load_vertex,
DrvrLoadSlews &load_slews_prev,
LoadPinIndexMap &load_pin_index_map)
{
if (load_slews_prev.empty())
return true;
size_t index = load_pin_index_map[load_vertex->pin()];
SlewSeq &slews_prev = load_slews_prev[index];;
Comment on lines +747 to +750

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Using operator[] on load_pin_index_map directly with load_vertex->pin() is unsafe because load_vertex might be visited via non-wire edges (such as isBidirectInstPath internal timing arcs) which are not populated in load_pin_index_map. If the pin is not present in the map, operator[] will insert a dummy entry with a default value of 0, causing incorrect comparisons with the first load pin's previous slew and corrupting the map.

Instead, use find() to safely check if the pin exists in the map before retrieving its index.

  auto ip = load_pin_index_map.find(load_vertex->pin());
  if (ip == load_pin_index_map.end())
    return true;
  size_t index = ip->second;
  SlewSeq &slews_prev = load_slews_prev[index];

size_t slew_count = graph_->slewCount();
for (auto const [pin, index] : load_pin_index_map) {
Vertex *load_vertex = graph_->pinLoadVertex(pin);
SlewSeq &slews_prev = load_slews_prev[index];;
for (size_t i = 0; i < slew_count; i++) {
const Slew slew = graph_->slew(load_vertex, i);
if (!delayEqual(slew, slews_prev[i], this))
return true;
}
for (size_t i = 0; i < slew_count; i++) {
const Slew slew = graph_->slew(load_vertex, i);
if (!delayEqual(slew, slews_prev[i], this))
return true;
}
return false;
}

void
GraphDelayCalc::enqueueTimingChecksEdges(Vertex *vertex)
GraphDelayCalc::enqueueCheckEdges(Vertex *vertex)
{
if (vertex->hasChecks()) {
VertexInEdgeIterator edge_iter(vertex, graph_);
Expand Down Expand Up @@ -1180,10 +1184,10 @@ GraphDelayCalc::annotateDelaysSlews(Edge *edge,
dcalc_result.drvrSlew(), scene, min_max);
if (!edge->role()->isLatchDtoQ()) {
Vertex *drvr_vertex = edge->to(graph_);
delay_changed |= annotateLoadDelays(drvr_vertex, arc->toEdge()->asRiseFall(),
dcalc_result,
load_pin_index_map, delay_zero, true,
scene, min_max);
annotateLoadDelays(drvr_vertex, arc->toEdge()->asRiseFall(),
dcalc_result,
load_pin_index_map, delay_zero, true,
scene, min_max);
}
return delay_changed;
}
Expand Down Expand Up @@ -1238,7 +1242,7 @@ GraphDelayCalc::annotateDelaySlew(Edge *edge,
// Annotate wire arc delays and load pin slews.
// extra_delay is additional wire delay to add to delay returned
// by the delay calculator.
bool
void
GraphDelayCalc::annotateLoadDelays(Vertex *drvr_vertex,
const RiseFall *drvr_rf,
ArcDcalcResult &dcalc_result,
Expand All @@ -1249,7 +1253,6 @@ GraphDelayCalc::annotateLoadDelays(Vertex *drvr_vertex,
const MinMax *min_max)
{
DcalcAPIndex ap_index = scene->dcalcAnalysisPtIndex(min_max);
bool changed = false;
VertexOutEdgeIterator edge_iter(drvr_vertex, graph_);
while (edge_iter.hasNext()) {
Edge *wire_edge = edge_iter.next();
Expand All @@ -1264,20 +1267,17 @@ GraphDelayCalc::annotateLoadDelays(Vertex *drvr_vertex,
load_vertex->to_string(this),
delayAsString(wire_delay, this),
delayAsString(load_slew, this));
bool load_changed = false;
if (!load_vertex->slewAnnotated(drvr_rf, min_max)) {
if (drvr_vertex->slewAnnotated(drvr_rf, min_max)) {
// Copy the driver slew to the load if it is annotated.
const Slew drvr_slew = graph_->slew(drvr_vertex, drvr_rf, ap_index);
graph_->setSlew(load_vertex, drvr_rf, ap_index, drvr_slew);
load_changed = true;
}
else {
const Slew slew = graph_->slew(load_vertex, drvr_rf, ap_index);
if (!merge
|| delayGreater(load_slew, slew, min_max, this)) {
graph_->setSlew(load_vertex, drvr_rf, ap_index, load_slew);
load_changed = true;
}
}
}
Expand All @@ -1290,18 +1290,12 @@ GraphDelayCalc::annotateLoadDelays(Vertex *drvr_vertex,
if (!merge
|| delayGreater(wire_delay_extra, delay, min_max, this)) {
graph_->setWireArcDelay(wire_edge, drvr_rf, ap_index, wire_delay_extra);
load_changed = true;
if (observer_)
observer_->delayChangedTo(load_vertex);
}
}
if (load_changed && observer_)
observer_->delayChangedTo(load_vertex);
// Enqueue bidirect driver from load vertex.
if (bidirectDrvrSlewFromLoad(load_pin))
iter_->enqueue(graph_->pinDrvrVertex(load_pin));
changed |= load_changed;
}
}
return changed;
}

LoadPinIndexMap
Expand Down
1 change: 0 additions & 1 deletion dcalc/PrimaDelayCalc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ PrimaDelayCalc::PrimaDelayCalc(StaState *sta) :
PrimaDelayCalc::PrimaDelayCalc(const PrimaDelayCalc &dcalc) :
DelayCalcBase(dcalc),
pin_node_map_(network_),
node_index_map_(dcalc.node_index_map_),
prima_order_(dcalc.prima_order_),
watch_pin_values_(network_),
table_dcalc_(makeDmpCeffElmoreDelayCalc(this))
Expand Down
Loading
Loading