From 748d46c12420d7cb3b7b2805283fd52abdbe158d Mon Sep 17 00:00:00 2001
From: Justin Viiret <justin.viiret@intel.com>
Date: Thu, 3 Dec 2015 09:27:57 +1100
Subject: [PATCH] CastleProto: track next top explicitly

Repeats may be removed (e.g. by pruning in role aliasing passes)
leaving "holes" in the top map. Track the next top to use explicitly,
rather than using repeats.size().
---
 src/nfa/castlecompile.cpp             | 21 ++++++++++++---------
 src/nfa/castlecompile.h               | 10 ++++++++++
 src/rose/rose_build_role_aliasing.cpp | 17 ++++++++++-------
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/src/nfa/castlecompile.cpp b/src/nfa/castlecompile.cpp
index 8d7f3913..cc5c599b 100644
--- a/src/nfa/castlecompile.cpp
+++ b/src/nfa/castlecompile.cpp
@@ -740,19 +740,13 @@ const CharReach &CastleProto::reach() const {
     return repeats.begin()->second.reach;
 }
 
-static
-u32 find_next_top(const map<u32, PureRepeat> &repeats) {
-    u32 top = verify_u32(repeats.size());
-    assert(!contains(repeats, top));
-    return top;
-}
-
 u32 CastleProto::add(const PureRepeat &pr) {
     assert(repeats.size() < max_occupancy);
     assert(pr.reach == reach());
     assert(pr.reports.size() == 1);
-    u32 top = find_next_top(repeats);
+    u32 top = next_top++;
     DEBUG_PRINTF("selected unused top %u\n", top);
+    assert(!contains(repeats, top));
     repeats.emplace(top, pr);
     for (const auto &report : pr.reports) {
         report_map[report].insert(top);
@@ -760,6 +754,15 @@ u32 CastleProto::add(const PureRepeat &pr) {
     return top;
 }
 
+void CastleProto::erase(u32 top) {
+    DEBUG_PRINTF("erase top %u\n", top);
+    assert(contains(repeats, top));
+    repeats.erase(top);
+    for (auto &m : report_map) {
+        m.second.erase(top);
+    }
+}
+
 u32 CastleProto::merge(const PureRepeat &pr) {
     assert(repeats.size() <= max_occupancy);
     assert(pr.reach == reach());
@@ -820,7 +823,7 @@ void remapCastleTops(CastleProto &proto, map<u32, u32> &top_map) {
     for (const auto &m : proto.repeats) {
         const u32 top = m.first;
         const PureRepeat &pr = m.second;
-        u32 new_top = find_next_top(out);
+        u32 new_top = out.size();
         out.emplace(new_top, pr);
         top_map[top] = new_top;
     }
diff --git a/src/nfa/castlecompile.h b/src/nfa/castlecompile.h
index 8ca3581c..fc4bb991 100644
--- a/src/nfa/castlecompile.h
+++ b/src/nfa/castlecompile.h
@@ -68,8 +68,12 @@ struct CastleProto {
     explicit CastleProto(const PureRepeat &pr);
     const CharReach &reach() const;
 
+    /** \brief Add a new repeat. */
     u32 add(const PureRepeat &pr);
 
+    /** \brief Remove a repeat. */
+    void erase(u32 top);
+
     /**
      * \brief Merge in the given repeat, returning the top used.
      *
@@ -84,6 +88,12 @@ struct CastleProto {
 
     /** \brief Mapping from report to associated tops. */
     ue2::unordered_map<ReportID, flat_set<u32>> report_map;
+
+    /**
+     * \brief Next top id to use. Repeats may be removed without top remapping,
+     * so we track this explicitly instead of using repeats.size().
+     */
+    u32 next_top = 1;
 };
 
 std::set<ReportID> all_reports(const CastleProto &proto);
diff --git a/src/rose/rose_build_role_aliasing.cpp b/src/rose/rose_build_role_aliasing.cpp
index 62db5b2e..88deaa25 100644
--- a/src/rose/rose_build_role_aliasing.cpp
+++ b/src/rose/rose_build_role_aliasing.cpp
@@ -751,14 +751,17 @@ void pruneReportIfUnused(const RoseBuildImpl &tbi, shared_ptr<NGHolder> h,
  * Castle. */
 static
 void pruneCastle(CastleProto &castle, ReportID report) {
-    for (map<u32, PureRepeat>::iterator it = castle.repeats.begin();
-         it != castle.repeats.end(); /* incr inside */) {
-        if (contains(it->second.reports, report)) {
-            ++it;
-        } else {
-            castle.repeats.erase(it++);
+    unordered_set<u32> dead; // tops to remove.
+    for (const auto &m : castle.repeats) {
+        if (!contains(m.second.reports, report)) {
+            dead.insert(m.first);
         }
     }
+
+    for (const auto &top : dead) {
+        castle.erase(top);
+    }
+
     assert(!castle.repeats.empty());
 }
 
@@ -798,7 +801,7 @@ void pruneUnusedTops(CastleProto &castle, const RoseGraph &g,
     for (u32 top : assoc_keys(castle.repeats)) {
         if (!contains(used_tops, top)) {
             DEBUG_PRINTF("removing unused top %u\n", top);
-            castle.repeats.erase(top);
+            castle.erase(top);
         }
     }
 }