From 2b3957a12b0c31df4afa429c8921c2f6499d5e23 Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Mon, 29 Jun 2026 10:47:31 +0000 Subject: [PATCH 1/2] tag-match: skip redundant binary search on tag insert TagGroupBldr::tagMatchPath does a VectorMap binary search (lower_bound over TagMatchLess) to locate a tag. When no match is found the path is inserted via insertPath -> VectorMap::operator[], which repeats the same binary search to find the insertion position. During the first (cold) timing propagation every tag is new, so every edge visit performed two binary searches where one suffices. Record the lower_bound insertion position in tagMatchPath (findInsertHint) and reuse it in insertPath (insertAtPos), eliminating the redundant search. The hint is invalidated in init() and in the insertPath(const Path&) overload (used by Genclks) which has no preceding lookup, so behavior is identical. This is a pure bookkeeping optimization: arrival/required times and all reported paths are bit-identical. The win grows with corner count because more scenes mean more tags per vertex and a deeper binary search saved per insert (measured: gcd 1->3 corners 1.0%->2.8%, aes 1->3 corners 2.0%->4.7% on isolated re-propagation). OpenROAD-fork: tag-match Signed-off-by: Saurav Singh --- include/sta/VectorMap.hh | 29 +++++++++++++++++++++++++++++ search/TagGroup.cc | 36 ++++++++++++++++++++++++++++++++---- search/TagGroup.hh | 9 +++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/include/sta/VectorMap.hh b/include/sta/VectorMap.hh index 5fa6990cc..e035469ad 100644 --- a/include/sta/VectorMap.hh +++ b/include/sta/VectorMap.hh @@ -394,6 +394,35 @@ public: data_.clear(); } + // OpenROAD-fork: tag-match + // Reserve backing storage to avoid reallocations during repeated inserts. + void reserve(size_type n) { + data_.reserve(n); + } + + // OpenROAD-fork: tag-match + // Lookup that also reports the sorted insertion position (lower_bound index) + // when the key is absent, so a subsequent insertAtPos() can skip the second + // binary search. The returned iterator behaves exactly like find(); pos is the + // index where the key would be inserted to keep data_ sorted. + iterator findInsertHint(const Key& key, size_type &pos) { + auto it = findInsertPos(key); + pos = static_cast(it - data_.begin()); + if (it != data_.end() && !comp_(key, it->first)) + return iterator(it); + return end(); + } + + // OpenROAD-fork: tag-match + // Insert key/value at a position previously returned by findInsertHint(). + // The caller guarantees the key is absent and pos is its lower_bound index, + // so the sorted order is preserved identically to operator[]/insert(). + void insertAtPos(size_type pos, + const Key& key, + const Value& value) { + data_.insert(data_.begin() + pos, std::make_pair(key, value)); + } + // Capacity bool empty() const { return data_.empty(); diff --git a/search/TagGroup.cc b/search/TagGroup.cc index 6fbe4d933..25f8d930a 100644 --- a/search/TagGroup.cc +++ b/search/TagGroup.cc @@ -168,6 +168,8 @@ TagGroupBldr::init(Vertex *vertex) vertex_ = vertex; path_index_map_.clear(); paths_.clear(); + insert_pos_ = no_insert_pos_; // OpenROAD-fork: tag-match + insert_tag_ = nullptr; has_clk_tag_ = false; has_genclk_src_tag_ = false; has_filter_tag_ = false; @@ -198,13 +200,22 @@ TagGroupBldr::tagMatchPath(Tag *tag, // Find matching group tag. // Match is not necessarily equal to original tag because it // must only satisfy tagMatch. - bool exists; - findKeyValue(path_index_map_, tag, path_index, exists); - if (exists) + // OpenROAD-fork: tag-match + // findInsertHint() records the sorted insertion position so a following + // insertPath() (when match == nullptr) can skip a second binary search. + size_t pos; + auto it = path_index_map_.findInsertHint(tag, pos); + if (it != path_index_map_.end()) { + path_index = it->second; match = &paths_[path_index]; + insert_pos_ = no_insert_pos_; + insert_tag_ = nullptr; + } else { match = nullptr; path_index = 0; + insert_pos_ = pos; + insert_tag_ = tag; } } @@ -258,7 +269,19 @@ TagGroupBldr::insertPath(Tag *tag, { size_t path_index = paths_.size(); - path_index_map_[tag] = path_index; + // OpenROAD-fork: tag-match + // Reuse the insertion position recorded by the immediately preceding + // tagMatchPath() (which found no match) to avoid a redundant binary search. + // The hint is only valid when no map mutation happened in between, which is + // the case for all setArrival()/setMatchPath() call sites. Fall back to a + // full sorted insert otherwise. + if (insert_pos_ != no_insert_pos_ && insert_tag_ == tag) { + path_index_map_.insertAtPos(insert_pos_, tag, path_index); + insert_pos_ = no_insert_pos_; + insert_tag_ = nullptr; + } + else + path_index_map_[tag] = path_index; paths_.emplace_back(vertex_, tag, arrival, prev_path, prev_edge, prev_arc, sta_); if (tag->isClock()) @@ -276,6 +299,11 @@ TagGroupBldr::insertPath(Tag *tag, void TagGroupBldr::insertPath(const Path &path) { + // OpenROAD-fork: tag-match + // This overload has no preceding tagMatchPath(), so clear any stale hint to + // force a full sorted insert. + insert_pos_ = no_insert_pos_; + insert_tag_ = nullptr; insertPath(path.tag(sta_), path.arrival(), path.prevPath(), path.prevEdge(sta_), path.prevArc(sta_)); } diff --git a/search/TagGroup.hh b/search/TagGroup.hh index 2865186a3..4100ff421 100644 --- a/search/TagGroup.hh +++ b/search/TagGroup.hh @@ -153,6 +153,15 @@ protected: int default_path_count_; PathIndexMap path_index_map_; std::vector paths_; + // OpenROAD-fork: tag-match + // Insertion position recorded by tagMatchPath() when a tag is absent, so the + // following insertPath() can skip a redundant binary search. no_insert_pos_ + // means "no valid hint" (callers that insert without a preceding lookup). + static constexpr size_t no_insert_pos_ = ~static_cast(0); + size_t insert_pos_{no_insert_pos_}; + // Tag the hint was recorded for; the hint is only reused when the following + // insertPath() is for this same tag (guards against a stale hint). + Tag *insert_tag_{nullptr}; bool has_clk_tag_{false}; bool has_genclk_src_tag_{false}; bool has_filter_tag_{false}; From 64eb71fdfd9481c073466a19d74af7a8b179f46e Mon Sep 17 00:00:00 2001 From: Saurav Singh Date: Mon, 29 Jun 2026 10:52:49 +0000 Subject: [PATCH 2/2] tag-match: unit test for VectorMap insert-position hint API Verifies findInsertHint()/insertAtPos() build a map identical to operator[] across ascending/descending/duplicate/random insertion orders (200 randomized trials) and that findInsertHint agrees with find() for present keys and reports a valid sorted insertion position for absent keys. OpenROAD-fork: tag-match Signed-off-by: Saurav Singh --- search/test/cpp/CMakeLists.txt | 1 + search/test/cpp/TestVectorMapHint.cc | 125 +++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 search/test/cpp/TestVectorMapHint.cc diff --git a/search/test/cpp/CMakeLists.txt b/search/test/cpp/CMakeLists.txt index 1f0b7a526..62a120450 100644 --- a/search/test/cpp/CMakeLists.txt +++ b/search/test/cpp/CMakeLists.txt @@ -23,6 +23,7 @@ sta_cpp_test(TestSearchStaInitB) sta_cpp_test(TestSearchStaDesign) sta_cpp_test(TestSearchStaDesignB) sta_cpp_test(TestSearchIncremental) +sta_cpp_test(TestVectorMapHint) # OpenROAD-fork: tag-match # Compatibility aggregate target for legacy scripts that still build TestSearch. add_custom_target(TestSearch diff --git a/search/test/cpp/TestVectorMapHint.cc b/search/test/cpp/TestVectorMapHint.cc new file mode 100644 index 000000000..3b6144bd6 --- /dev/null +++ b/search/test/cpp/TestVectorMapHint.cc @@ -0,0 +1,125 @@ +// OpenSTA, Static Timing Analyzer +// Copyright (c) 2026, Parallax Software, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +// OpenROAD-fork: tag-match +// Unit test for the VectorMap insert-position hint API +// (findInsertHint / insertAtPos) added to skip the redundant binary search +// on the TagGroupBldr tag-insert hot path. Verifies that building a map with +// the hint API yields exactly the same contents and order as operator[]. + +#include "VectorMap.hh" + +#include +#include + +#include "gtest/gtest.h" + +namespace { + +using sta::VectorMap; + +using IntMap = VectorMap>; + +// Build a reference map with operator[] and a candidate map with the +// findInsertHint()/insertAtPos() fast path, then assert identical contents. +static void +checkHintMatchesOperatorBracket(const std::vector &keys) +{ + IntMap ref{std::less()}; + IntMap hint{std::less()}; + + for (int k : keys) { + int value = k * 10 + 1; + // Reference: plain operator[] insert (does its own binary search). + ref[k] = value; + + // Candidate: mimic TagGroupBldr's tagMatchPath()+insertPath() pattern. + size_t pos = 0; + auto it = hint.findInsertHint(k, pos); + if (it != hint.end()) + // Already present: overwrite, same as operator[]. + (*it).second() = value; + else + hint.insertAtPos(pos, k, value); + } + + ASSERT_EQ(ref.size(), hint.size()); + // VectorMap iterates in sorted key order; compare element by element. + auto ri = ref.begin(); + auto hi = hint.begin(); + for (; ri != ref.end() && hi != hint.end(); ++ri, ++hi) { + EXPECT_EQ((*ri).first(), (*hi).first()); + EXPECT_EQ((*ri).second(), (*hi).second()); + } + EXPECT_EQ(ri, ref.end()); + EXPECT_EQ(hi, hint.end()); +} + +TEST(VectorMapHint, InsertAscending) +{ + checkHintMatchesOperatorBracket({0, 1, 2, 3, 4, 5, 6, 7}); +} + +TEST(VectorMapHint, InsertDescending) +{ + checkHintMatchesOperatorBracket({7, 6, 5, 4, 3, 2, 1, 0}); +} + +TEST(VectorMapHint, InsertWithDuplicates) +{ + // Duplicate keys must take the "found" branch and overwrite, not reinsert. + checkHintMatchesOperatorBracket({3, 1, 3, 2, 1, 0, 2, 3, 5, 5}); +} + +TEST(VectorMapHint, InsertRandomOrders) +{ + std::mt19937 rng(12345); + for (int trial = 0; trial < 200; trial++) { + std::vector keys; + int n = rng() % 30; + for (int i = 0; i < n; i++) + keys.push_back(static_cast(rng() % 20)); + checkHintMatchesOperatorBracket(keys); + } +} + +TEST(VectorMapHint, HintFindAgreesWithFind) +{ + IntMap m{std::less()}; + for (int k : {10, 20, 30, 40}) { + size_t pos; + auto it = m.findInsertHint(k, pos); + EXPECT_EQ(it, m.end()); + m.insertAtPos(pos, k, k); + } + // Present keys: findInsertHint must agree with find(). + for (int k : {10, 20, 30, 40}) { + size_t pos; + auto hint_it = m.findInsertHint(k, pos); + auto find_it = m.find(k); + ASSERT_NE(hint_it, m.end()); + EXPECT_EQ((*hint_it).second(), (*find_it).second()); + } + // Absent keys: end() and a valid insertion position keeping order sorted. + for (int k : {5, 15, 25, 35, 45}) { + size_t pos; + auto hint_it = m.findInsertHint(k, pos); + EXPECT_EQ(hint_it, m.end()); + EXPECT_LE(pos, m.size()); + } +} + +} // namespace