Merge pull request #114 from YosysHQ/fix_legalise

Fix legalise

3 files changed, 30 insertions, 26 deletions

diff --git a/common/nextpnr.h b/common/nextpnr.h
index 86e781ae..4434c438 100644
--- a/common/nextpnr.h
+++ b/common/nextpnr.h
@@ -191,7 +191,7 @@ struct Loc
     Loc(int x, int y, int z) : x(x), y(y), z(z) {}
 
     bool operator==(const Loc &other) const { return (x == other.x) && (y == other.y) && (z == other.z); }
-    bool operator!=(const Loc &other) const { return (x != other.x) || (y != other.y) || (z == other.z); }
+    bool operator!=(const Loc &other) const { return (x != other.x) || (y != other.y) || (z != other.z); }
 };
 
 NEXTPNR_NAMESPACE_END
diff --git a/common/place_common.cc b/common/place_common.cc
index 1c262c6f..4cb5ae11 100644
--- a/common/place_common.cc
+++ b/common/place_common.cc
@@ -431,12 +431,12 @@ class ConstraintLegaliseWorker
             print_chain(child, depth + 1);
     }
 
-    void print_stats(const char *point)
+    unsigned print_stats(const char *point)
     {
         float distance_sum = 0;
         float max_distance = 0;
-        int moved_cells = 0;
-        int unplaced_cells = 0;
+        unsigned moved_cells = 0;
+        unsigned unplaced_cells = 0;
         for (auto orig : oldLocations) {
             if (ctx->cells.at(orig.first)->bel == BelId()) {
                 unplaced_cells++;
@@ -456,9 +456,10 @@ class ConstraintLegaliseWorker
             log_info("       average distance %f\n", (distance_sum / moved_cells));
             log_info("       maximum distance %f\n", max_distance);
         }
+        return moved_cells + unplaced_cells;
     }
 
-    bool legalise_constraints()
+    int legalise_constraints()
     {
         log_info("Legalising relative constraints...\n");
         for (auto cell : sorted(ctx->cells)) {
@@ -470,27 +471,28 @@ class ConstraintLegaliseWorker
                 if (ctx->verbose)
                     print_chain(cell.second);
                 log_error("failed to place chain starting at cell '%s'\n", cell.first.c_str(ctx));
-                return false;
+                return -1;
             }
         }
-        print_stats("after legalising chains");
+        if (print_stats("legalising chains") == 0)
+            return 0;
         for (auto rippedCell : rippedCells) {
             bool res = place_single_cell(ctx, ctx->cells.at(rippedCell).get(), true);
             if (!res) {
                 log_error("failed to place cell '%s' after relative constraint legalisation\n", rippedCell.c_str(ctx));
-                return false;
+                return -1;
             }
         }
-        print_stats("after replacing ripped up cells");
+        auto score = print_stats("replacing ripped up cells");
         for (auto cell : sorted(ctx->cells))
             if (get_constraints_distance(ctx, cell.second) != 0)
                 log_error("constraint satisfaction check failed for cell '%s' at Bel '%s'\n", cell.first.c_str(ctx),
                           ctx->getBelName(cell.second->bel).c_str(ctx));
-        return true;
+        return score;
     }
 };
 
-bool legalise_relative_constraints(Context *ctx) { return ConstraintLegaliseWorker(ctx).legalise_constraints(); }
+bool legalise_relative_constraints(Context *ctx) { return ConstraintLegaliseWorker(ctx).legalise_constraints() > 0; }
 
 // Get the total distance from satisfied constraints for a cell
 int get_constraints_distance(const Context *ctx, const CellInfo *cell)
diff --git a/common/placer1.cc b/common/placer1.cc
index 0d7c0701..0fd9a227 100644
--- a/common/placer1.cc
+++ b/common/placer1.cc
@@ -244,21 +244,23 @@ class SAPlacer
             }
             // Once cooled below legalise threshold, run legalisation and start requiring
             // legal moves only
-            if (temp < legalise_temp && !require_legal) {
-                legalise_relative_constraints(ctx);
-                require_legal = true;
-                autoplaced.clear();
-                for (auto cell : sorted(ctx->cells)) {
-                    if (cell.second->belStrength < STRENGTH_STRONG)
-                        autoplaced.push_back(cell.second);
-                }
-                temp = post_legalise_temp;
-                diameter *= post_legalise_dia_scale;
-                ctx->shuffle(autoplaced);
+            if (temp < legalise_temp && require_legal) {
+                if (legalise_relative_constraints(ctx)) {
+                    // Only increase temperature if something was moved
+                    autoplaced.clear();
+                    for (auto cell : sorted(ctx->cells)) {
+                        if (cell.second->belStrength < STRENGTH_STRONG)
+                            autoplaced.push_back(cell.second);
+                    }
+                    temp = post_legalise_temp;
+                    diameter *= post_legalise_dia_scale;
+                    ctx->shuffle(autoplaced);
 
-                // Legalisation is a big change so force a slack redistribution here
-                if (ctx->slack_redist_iter > 0)
-                    assign_budget(ctx, true /* quiet */);
+                    // Legalisation is a big change so force a slack redistribution here
+                    if (ctx->slack_redist_iter > 0)
+                        assign_budget(ctx, true /* quiet */);
+                }
+                require_legal = false;
             } else if (ctx->slack_redist_iter > 0 && iter % ctx->slack_redist_iter == 0) {
                 assign_budget(ctx, true /* quiet */);
             }
@@ -486,7 +488,7 @@ class SAPlacer
     std::unordered_map<IdString, int> bel_types;
     std::vector<std::vector<std::vector<std::vector<BelId>>>> fast_bels;
     std::unordered_set<BelId> locked_bels;
-    bool require_legal = false;
+    bool require_legal = true;
     const float legalise_temp = 1;
     const float post_legalise_temp = 10;
     const float post_legalise_dia_scale = 1.5;