From 7878561970d27ff2fed73fe0909fa64320e34bc8 Mon Sep 17 00:00:00 2001 From: Keith Rothman <537074+litghost@users.noreply.github.com> Date: Fri, 26 Feb 2021 11:26:52 -0800 Subject: Add placement sanity check in placer_heap. Also check return of placer1_refine. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com> --- common/placer_heap.cc | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'common') diff --git a/common/placer_heap.cc b/common/placer_heap.cc index 8a3b427f..df1454e8 100644 --- a/common/placer_heap.cc +++ b/common/placer_heap.cc @@ -321,7 +321,27 @@ class HeAPPlacer ctx->check(); - placer1_refine(ctx, Placer1Cfg(ctx)); + 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; + } + + if (!placer1_refine(ctx, Placer1Cfg(ctx))) { + return false; + } return true; } -- cgit v1.2.3 From 77a5a60a66b0cfc1602edb61aadf392dc651bf46 Mon Sep 17 00:00:00 2001 From: Keith Rothman <537074+litghost@users.noreply.github.com> Date: Fri, 26 Feb 2021 11:37:27 -0800 Subject: Fix latent bug with context locking in placer HeAP. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com> --- common/placer_heap.cc | 22 +++++++++++--------- common/scope_lock.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 common/scope_lock.h (limited to 'common') diff --git a/common/placer_heap.cc b/common/placer_heap.cc index df1454e8..cea862af 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 lock(ctx); place_constraints(); build_fast_bels(); seed_placement(); @@ -312,15 +313,6 @@ class HeAPPlacer log_info("AP soln: %s -> %s\n", cell.first.c_str(ctx), ctx->nameOfBel(cell.second->bel)); } - ctx->unlock(); - auto endtt = std::chrono::high_resolution_clock::now(); - log_info("HeAP Placer Time: %.02fs\n", std::chrono::duration(endtt - startt).count()); - log_info(" of which solving equations: %.02fs\n", solve_time); - log_info(" of which spreading cells: %.02fs\n", cl_time); - log_info(" of which strict legalisation: %.02fs\n", sl_time); - - ctx->check(); - bool any_bad_placements = false; for (auto bel : ctx->getBels()) { CellInfo *cell = ctx->getBoundBelCell(bel); @@ -339,6 +331,16 @@ class HeAPPlacer return false; } + lock.unlock_early(); + + auto endtt = std::chrono::high_resolution_clock::now(); + log_info("HeAP Placer Time: %.02fs\n", std::chrono::duration(endtt - startt).count()); + log_info(" of which solving equations: %.02fs\n", solve_time); + log_info(" of which spreading cells: %.02fs\n", cl_time); + log_info(" of which strict legalisation: %.02fs\n", sl_time); + + ctx->check(); + if (!placer1_refine(ctx, Placer1Cfg(ctx))) { return false; } diff --git a/common/scope_lock.h b/common/scope_lock.h new file mode 100644 index 00000000..35de6bc9 --- /dev/null +++ b/common/scope_lock.h @@ -0,0 +1,56 @@ +/* + * 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 + +namespace nextpnr { + +// Provides a simple RIAA 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 +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() { + locked_ = false; + obj_->unlock(); + } +private: + LockingObject *obj_; + bool locked_; +}; + +}; + +#endif /* SCOPE_LOCK_H */ -- cgit v1.2.3 From 99a2262d61c20019b2a4ce5321df48a9d5d43864 Mon Sep 17 00:00:00 2001 From: Keith Rothman <537074+litghost@users.noreply.github.com> Date: Mon, 1 Mar 2021 09:41:29 -0800 Subject: Use scope in router1/2 and placer1. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com> --- common/placer1.cc | 12 ++++++++---- common/placer_heap.cc | 3 +-- common/router1.cc | 8 ++++---- common/router2.cc | 5 +++++ 4 files changed, 18 insertions(+), 10 deletions(-) (limited to 'common') diff --git a/common/placer1.cc b/common/placer1.cc index 837010fe..5171af19 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 lock(ctx); size_t placed_cells = 0; std::vector 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; } @@ -1251,9 +1253,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; } } @@ -1272,9 +1275,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 cea862af..eb931a37 100644 --- a/common/placer_heap.cc +++ b/common/placer_heap.cc @@ -331,8 +331,6 @@ class HeAPPlacer return false; } - lock.unlock_early(); - auto endtt = std::chrono::high_resolution_clock::now(); log_info("HeAP Placer Time: %.02fs\n", std::chrono::duration(endtt - startt).count()); log_info(" of which solving equations: %.02fs\n", solve_time); @@ -340,6 +338,7 @@ class HeAPPlacer log_info(" of which strict legalisation: %.02fs\n", sl_time); ctx->check(); + lock.unlock_early(); if (!placer1_refine(ctx, Placer1Cfg(ctx))) { return false; 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 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 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)); } }; -- cgit v1.2.3 From 392156c25095aa93fdf847ef51dfa571b5d2ce88 Mon Sep 17 00:00:00 2001 From: Keith Rothman <537074+litghost@users.noreply.github.com> Date: Mon, 1 Mar 2021 10:03:42 -0800 Subject: Correct spelling of RAII and add missing check in unlock_early. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com> --- common/scope_lock.h | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'common') diff --git a/common/scope_lock.h b/common/scope_lock.h index 35de6bc9..7b6c9dcd 100644 --- a/common/scope_lock.h +++ b/common/scope_lock.h @@ -20,37 +20,46 @@ #ifndef SCOPE_LOCK_H #define SCOPE_LOCK_H +#include + namespace nextpnr { -// Provides a simple RIAA locking object. ScopeLock takes a lock when +// 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 -class ScopeLock { -public: - ScopeLock(LockingObject * obj) : obj_(obj), locked_(false) { +template 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_) { + ~ScopeLock() + { + if (locked_) { obj_->unlock(); } } - void unlock_early() { + void unlock_early() + { + if (!locked_) { + throw std::runtime_error("Lock already released?"); + } locked_ = false; obj_->unlock(); } -private: + + private: LockingObject *obj_; bool locked_; }; -}; +}; // namespace nextpnr #endif /* SCOPE_LOCK_H */ -- cgit v1.2.3