Merge pull request #605 from litghost/add_placement_sanity_check

Add placement sanity check in placer_heap.

5 files changed, 106 insertions, 11 deletions

diff --git a/common/placer1.cc b/common/placer1.cc
index 619bfbc8..d57a841a 100644
--- a/common/placer1.cc
+++ b/common/placer1.cc
@@ -42,6 +42,7 @@
 #include "fast_bels.h"
 #include "log.h"
 #include "place_common.h"
+#include "scope_lock.h"
 #include "timing.h"
 #include "util.h"
 
@@ -142,7 +143,8 @@ class SAPlacer
     bool place(bool refine = false)
     {
         log_break();
-        ctx->lock();
+
+        nextpnr::ScopeLock<Context> lock(ctx);
 
         size_t placed_cells = 0;
         std::vector<CellInfo *> autoplaced;
@@ -421,7 +423,7 @@ class SAPlacer
                 log_error("constraint satisfaction check failed for cell '%s' at Bel '%s'\n", cell.first.c_str(ctx),
                           ctx->nameOfBel(cell.second->bel));
         timing_analysis(ctx);
-        ctx->unlock();
+
         return true;
     }
 
@@ -1263,9 +1265,10 @@ bool placer1(Context *ctx, Placer1Cfg cfg)
         return true;
     } catch (log_execution_error_exception) {
 #ifndef NDEBUG
+        ctx->lock();
         ctx->check();
-#endif
         ctx->unlock();
+#endif
         return false;
     }
 }
@@ -1284,9 +1287,10 @@ bool placer1_refine(Context *ctx, Placer1Cfg cfg)
         return true;
     } catch (log_execution_error_exception) {
 #ifndef NDEBUG
+        ctx->lock();
         ctx->check();
-#endif
         ctx->unlock();
+#endif
         return false;
     }
 }
diff --git a/common/placer_heap.cc b/common/placer_heap.cc
index 8a3b427f..eb931a37 100644
--- a/common/placer_heap.cc
+++ b/common/placer_heap.cc
@@ -49,6 +49,7 @@
 #include "nextpnr.h"
 #include "place_common.h"
 #include "placer1.h"
+#include "scope_lock.h"
 #include "timing.h"
 #include "util.h"
 
@@ -147,7 +148,7 @@ class HeAPPlacer
     {
         auto startt = std::chrono::high_resolution_clock::now();
 
-        ctx->lock();
+        nextpnr::ScopeLock<Context> lock(ctx);
         place_constraints();
         build_fast_bels();
         seed_placement();
@@ -312,7 +313,24 @@ class HeAPPlacer
                 log_info("AP soln: %s -> %s\n", cell.first.c_str(ctx), ctx->nameOfBel(cell.second->bel));
         }
 
-        ctx->unlock();
+        bool any_bad_placements = false;
+        for (auto bel : ctx->getBels()) {
+            CellInfo *cell = ctx->getBoundBelCell(bel);
+            if (!ctx->isBelLocationValid(bel)) {
+                std::string cell_text = "no cell";
+                if (cell != nullptr)
+                    cell_text = std::string("cell '") + ctx->nameOf(cell) + "'";
+                log_warning("post-placement validity check failed for Bel '%s' "
+                            "(%s)\n",
+                            ctx->nameOfBel(bel), cell_text.c_str());
+                any_bad_placements = true;
+            }
+        }
+
+        if (any_bad_placements) {
+            return false;
+        }
+
         auto endtt = std::chrono::high_resolution_clock::now();
         log_info("HeAP Placer Time: %.02fs\n", std::chrono::duration<double>(endtt - startt).count());
         log_info("  of which solving equations: %.02fs\n", solve_time);
@@ -320,8 +338,11 @@ class HeAPPlacer
         log_info("  of which strict legalisation: %.02fs\n", sl_time);
 
         ctx->check();
+        lock.unlock_early();
 
-        placer1_refine(ctx, Placer1Cfg(ctx));
+        if (!placer1_refine(ctx, Placer1Cfg(ctx))) {
+            return false;
+        }
 
         return true;
     }
diff --git a/common/router1.cc b/common/router1.cc
index efc06b06..bffbc9f9 100644
--- a/common/router1.cc
+++ b/common/router1.cc
@@ -23,6 +23,7 @@
 
 #include "log.h"
 #include "router1.h"
+#include "scope_lock.h"
 #include "timing.h"
 
 namespace {
@@ -805,7 +806,7 @@ bool router1(Context *ctx, const Router1Cfg &cfg)
     try {
         log_break();
         log_info("Routing..\n");
-        ctx->lock();
+        nextpnr::ScopeLock<Context> lock(ctx);
         auto rstart = std::chrono::high_resolution_clock::now();
 
         log_info("Setting up routing queue.\n");
@@ -854,7 +855,6 @@ bool router1(Context *ctx, const Router1Cfg &cfg)
                 router.check();
                 ctx->check();
 #endif
-                ctx->unlock();
                 return false;
             }
         }
@@ -878,13 +878,13 @@ bool router1(Context *ctx, const Router1Cfg &cfg)
         timing_analysis(ctx, true /* slack_histogram */, true /* print_fmax */, true /* print_path */,
                         true /* warn_on_failure */);
 
-        ctx->unlock();
         return true;
     } catch (log_execution_error_exception) {
 #ifndef NDEBUG
+        ctx->lock();
         ctx->check();
-#endif
         ctx->unlock();
+#endif
         return false;
     }
 }
diff --git a/common/router2.cc b/common/router2.cc
index abe5f302..1b7a6fed 100644
--- a/common/router2.cc
+++ b/common/router2.cc
@@ -36,6 +36,7 @@
 #include "log.h"
 #include "nextpnr.h"
 #include "router1.h"
+#include "scope_lock.h"
 #include "timing.h"
 #include "util.h"
 
@@ -1161,6 +1162,8 @@ struct Router2
         ThreadContext st;
         int iter = 1;
 
+        nextpnr::ScopeLock<Context> lock(ctx);
+
         for (size_t i = 0; i < nets_by_udata.size(); i++)
             route_queue.push_back(i);
 
@@ -1237,6 +1240,8 @@ struct Router2
 
         log_info("Running router1 to check that route is legal...\n");
 
+        lock.unlock_early();
+
         router1(ctx, Router1Cfg(ctx));
     }
 };
diff --git a/common/scope_lock.h b/common/scope_lock.h
new file mode 100644
index 00000000..7b6c9dcd
--- /dev/null
+++ b/common/scope_lock.h
@@ -0,0 +1,65 @@
+/*
+ *  nextpnr -- Next Generation Place and Route
+ *
+ *  Copyright (C) 2021 Symbiflow Authors.
+ *
+ *  Permission to use, copy, modify, and/or distribute this software for any
+ *  purpose with or without fee is hereby granted, provided that the above
+ *  copyright notice and this permission notice appear in all copies.
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ *  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ *  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ *  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ *  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ *  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ *  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ */
+
+#ifndef SCOPE_LOCK_H
+#define SCOPE_LOCK_H
+
+#include <stdexcept>
+
+namespace nextpnr {
+
+// Provides a simple RAII locking object.  ScopeLock takes a lock when
+// constructed, and releases the lock on destruction or if "unlock_early" is
+// called.
+//
+// LockingObject must have a method "void lock(void)" and "void unlock(void)".
+template <typename LockingObject> class ScopeLock
+{
+  public:
+    ScopeLock(LockingObject *obj) : obj_(obj), locked_(false)
+    {
+        obj_->lock();
+        locked_ = true;
+    }
+    ScopeLock(const ScopeLock &other) = delete;
+    ScopeLock(const ScopeLock &&other) = delete;
+
+    ~ScopeLock()
+    {
+        if (locked_) {
+            obj_->unlock();
+        }
+    }
+    void unlock_early()
+    {
+        if (!locked_) {
+            throw std::runtime_error("Lock already released?");
+        }
+        locked_ = false;
+        obj_->unlock();
+    }
+
+  private:
+    LockingObject *obj_;
+    bool locked_;
+};
+
+}; // namespace nextpnr
+
+#endif /* SCOPE_LOCK_H */