Remove isValidBelForCell

This Arch API dates from when we were first working out how to
implement placement validity checking, and in practice is little used by
the core parts of placer1/HeAP and the Arch implementation involves a
lot of duplication with isBelLocationValid.

In the short term; placement validity checking is better served by the
combination of checkBelAvail and isValidBelForCellType before placement;
followed by isBelLocationValid after placement (potentially after
moving/swapping multiple cells).

Longer term, removing this API makes things a bit cleaner for a new
validity checking API.

Signed-off-by: gatecat <gatecat@ds0.me>

24 files changed, 170 insertions, 287 deletions

diff --git a/common/nextpnr.h b/common/nextpnr.h
index b6ee33fe..4ddf8fef 100644
--- a/common/nextpnr.h
+++ b/common/nextpnr.h
@@ -1169,7 +1169,6 @@ template <typename R> struct ArchAPI : BaseCtx
     virtual BelBucketId getBelBucketByName(IdString name) const = 0;
     virtual BelBucketId getBelBucketForBel(BelId bel) const = 0;
     virtual BelBucketId getBelBucketForCellType(IdString cell_type) const = 0;
-    virtual bool isValidBelForCell(CellInfo *cell, BelId bel) const = 0;
     virtual bool isBelLocationValid(BelId bel) const = 0;
     virtual typename R::CellTypeRangeT getCellTypes() const = 0;
     virtual typename R::BelBucketRangeT getBelBuckets() const = 0;
@@ -1420,7 +1419,6 @@ template <typename R> struct BaseArch : ArchAPI<R>
     {
         return getBelBucketByName(cell_type);
     };
-    virtual bool isValidBelForCell(CellInfo *cell, BelId bel) const override { return true; }
     virtual bool isBelLocationValid(BelId bel) const override { return true; }
     virtual typename R::CellTypeRangeT getCellTypes() const override
     {
diff --git a/common/place_common.cc b/common/place_common.cc
index 6526c38e..e5b48ffb 100644
--- a/common/place_common.cc
+++ b/common/place_common.cc
@@ -118,8 +118,7 @@ bool place_single_cell(Context *ctx, CellInfo *cell, bool require_legality)
         }
         IdString targetType = cell->type;
         for (auto bel : ctx->getBels()) {
-            if (ctx->isValidBelForCellType(targetType, bel) &&
-                (!require_legality || ctx->isValidBelForCell(cell, bel))) {
+            if (ctx->isValidBelForCellType(targetType, bel)) {
                 if (ctx->checkBelAvail(bel)) {
                     wirelen_t wirelen = get_cell_metric_at_bel(ctx, cell, bel, MetricType::COST);
                     if (iters >= 4)
@@ -155,12 +154,20 @@ bool place_single_cell(Context *ctx, CellInfo *cell, bool require_legality)
             ctx->unbindBel(ripup_target->bel);
             best_bel = ripup_bel;
         } else {
+            ripup_target = nullptr;
             all_placed = true;
         }
+        ctx->bindBel(best_bel, cell, STRENGTH_WEAK);
+        if (require_legality && !ctx->isBelLocationValid(best_bel)) {
+            ctx->unbindBel(best_bel);
+            if (ripup_target != nullptr) {
+                ctx->bindBel(best_bel, ripup_target, STRENGTH_WEAK);
+            }
+            all_placed = false;
+            continue;
+        }
         if (ctx->verbose)
             log_info("   placed single cell '%s' at '%s'\n", cell->name.c_str(ctx), ctx->nameOfBel(best_bel));
-        ctx->bindBel(best_bel, cell, STRENGTH_WEAK);
-
         cell = ripup_target;
     }
     return true;
@@ -387,7 +394,7 @@ class ConstraintLegaliseWorker
                         for (auto bel : ctx->getBelsByTile(cp.second.x, cp.second.y)) {
                             CellInfo *belCell = ctx->getBoundBelCell(bel);
                             if (belCell != nullptr && !solution.count(belCell->name)) {
-                                if (!ctx->isValidBelForCell(belCell, bel)) {
+                                if (!ctx->isBelLocationValid(bel)) {
                                     NPNR_ASSERT(belCell->belStrength < STRENGTH_STRONG);
                                     ctx->unbindBel(bel);
                                     rippedCells.insert(belCell->name);
diff --git a/common/placer1.cc b/common/placer1.cc
index 280dd02e..270430e9 100644
--- a/common/placer1.cc
+++ b/common/placer1.cc
@@ -167,13 +167,6 @@ class SAPlacer
                                   "\'%s\' of type \'%s\'\n",
                                   loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
                     }
-                    if (!ctx->isValidBelForCell(cell, bel)) {
-                        IdString bel_type = ctx->getBelType(bel);
-                        log_error("Bel \'%s\' of type \'%s\' is not valid for cell "
-                                  "\'%s\' of type \'%s\'\n",
-                                  loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
-                    }
-
                     auto bound_cell = ctx->getBoundBelCell(bel);
                     if (bound_cell) {
                         log_error(
@@ -182,6 +175,12 @@ class SAPlacer
                     }
 
                     ctx->bindBel(bel, cell, STRENGTH_USER);
+                    if (!ctx->isBelLocationValid(bel)) {
+                        IdString bel_type = ctx->getBelType(bel);
+                        log_error("Bel \'%s\' of type \'%s\' is not valid for cell "
+                                  "\'%s\' of type \'%s\'\n",
+                                  loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
+                    }
                     locked_bels.insert(bel);
                     placed_cells++;
                 }
@@ -444,7 +443,7 @@ class SAPlacer
             IdString targetType = cell->type;
 
             auto proc_bel = [&](BelId bel) {
-                if (ctx->isValidBelForCellType(targetType, bel) && ctx->isValidBelForCell(cell, bel)) {
+                if (ctx->isValidBelForCellType(targetType, bel)) {
                     if (ctx->checkBelAvail(bel)) {
                         uint64_t score = ctx->rng64();
                         if (score <= best_score) {
@@ -480,10 +479,20 @@ class SAPlacer
                 ctx->unbindBel(ripup_target->bel);
                 best_bel = ripup_bel;
             } else {
+                ripup_target = nullptr;
                 all_placed = true;
             }
             ctx->bindBel(best_bel, cell, STRENGTH_WEAK);
 
+            if (!ctx->isBelLocationValid(best_bel)) {
+                ctx->unbindBel(best_bel);
+                if (ripup_target != nullptr) {
+                    ctx->bindBel(best_bel, ripup_target, STRENGTH_WEAK);
+                }
+                all_placed = false;
+                continue;
+            }
+
             // Back annotate location
             cell->attrs[ctx->id("BEL")] = ctx->getBelName(cell->bel).str(ctx);
             cell = ripup_target;
diff --git a/common/placer_heap.cc b/common/placer_heap.cc
index 7882c8da..7d529401 100644
--- a/common/placer_heap.cc
+++ b/common/placer_heap.cc
@@ -392,13 +392,6 @@ class HeAPPlacer
                               "\'%s\' of type \'%s\'\n",
                               loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
                 }
-                if (!ctx->isValidBelForCell(cell, bel)) {
-                    IdString bel_type = ctx->getBelType(bel);
-                    log_error("Bel \'%s\' of type \'%s\' is not valid for cell "
-                              "\'%s\' of type \'%s\'\n",
-                              loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
-                }
-
                 auto bound_cell = ctx->getBoundBelCell(bel);
                 if (bound_cell) {
                     log_error("Cell \'%s\' cannot be bound to bel \'%s\' since it is already bound to cell \'%s\'\n",
@@ -406,6 +399,12 @@ class HeAPPlacer
                 }
 
                 ctx->bindBel(bel, cell, STRENGTH_USER);
+                if (!ctx->isBelLocationValid(bel)) {
+                    IdString bel_type = ctx->getBelType(bel);
+                    log_error("Bel \'%s\' of type \'%s\' is not valid for cell "
+                              "\'%s\' of type \'%s\'\n",
+                              loc_name.c_str(), bel_type.c_str(ctx), cell->name.c_str(ctx), cell->type.c_str(ctx));
+                }
                 placed_cells++;
             }
         }
@@ -571,12 +570,13 @@ class HeAPPlacer
                         place_cells.push_back(ci);
                         placed = true;
                     } else {
-                        if (ctx->isValidBelForCell(ci, bel)) {
-                            ctx->bindBel(bel, ci, STRENGTH_STRONG);
+                        ctx->bindBel(bel, ci, STRENGTH_STRONG);
+                        if (ctx->isBelLocationValid(bel)) {
                             cell_locs[cell.first].locked = true;
                             placed = true;
                             bels_used.insert(bel);
                         } else {
+                            ctx->unbindBel(bel);
                             available_bels.at(ci->type).push_front(bel);
                         }
                     }
diff --git a/docs/archapi.md b/docs/archapi.md
index 0f0e6181..fc943dfb 100644
--- a/docs/archapi.md
+++ b/docs/archapi.md
@@ -652,8 +652,8 @@ strict legality enforcement.  It is not required that all bels within a bucket
 are strictly equivelant.
 
 Strict legality step will enforce those differences, along with additional
-local constraints.  `isValidBelForCell`, `isValidBelForCellType`, and
-`isBelLocationValid` are used to enforce strict legality checks.
+local constraints.  `isValidBelForCellType`, and `isBelLocationValid` are used
+to enforce strict legality checks.
 
 ### BelBucketRangeT getBelBuckets() const
 
@@ -702,18 +702,10 @@ return the same value regardless if other cells are placed within the fabric.
 
 *BaseArch default: returns `cell_type == getBelType(bel)`*
 
-### bool isValidBelForCell(CellInfo \*cell, BelId bel) const
-
-Returns true if the given cell can be bound to the given bel, considering
-other bound resources. For example, this can be used if there is only
-a certain number of different clock signals allowed for a group of bels.
-
-*BaseArch default: returns true*
-
 ### bool isBelLocationValid(BelId bel) const
 
-Returns true if a bell in the current configuration is valid, i.e. if
-`isValidBelForCell()` would return true for the current mapping.
+Returns true if a bel in the current configuration is legal (for example,
+a flipflop's clock signal is correctly shared with all bels in a slice.)
 
 *BaseArch default: returns true*
 
diff --git a/docs/coding.md b/docs/coding.md
index 355fe457..dc09bb3f 100644
--- a/docs/coding.md
+++ b/docs/coding.md
@@ -75,7 +75,7 @@ The job of the placer in nextpnr is to find a suitable bel for each cell in the
 
 Placers might want to create their own indices of bels (for example, bels by type and location) to speed up the search.
 
-As nextpnr allows arbitrary constraints on bels for more advanced packer-free flows and complex real-world architectures; placements must be checked for legality using `isValidBelForCell` (before placement) or `isBelLocationValid` (after placement) and the placement rejected if invalid. For analytical placement algorithms; after creating a spread-out AP solution the legality of placing each cell needs to be checked. In practice, the cost of this is fairly low as the architecture should ensure these functions are as fast as possible.
+As nextpnr allows arbitrary constraints on bels for more advanced packer-free flows and complex real-world architectures; placements must be checked for legality using `isBelLocationValid` (after placement) and the placement rejected if invalid. For analytical placement algorithms; after creating a spread-out AP solution the legality of placing each cell needs to be checked. In practice, the cost of this is fairly low as the architecture should ensure these functions are as fast as possible.
 
 There are several routes for timing information in the placer:
     - sink `PortRef`s have a `budget` value annotated by calling `assign_budget` which is an estimate of the maximum delay that an arc may have
diff --git a/ecp5/arch.h b/ecp5/arch.h
index d5edd88e..83928256 100644
--- a/ecp5/arch.h
+++ b/ecp5/arch.h
@@ -864,7 +864,6 @@ struct Arch : BaseArch<ArchRanges>
 
     // -------------------------------------------------
     // Placement validity checks
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override;
     bool isBelLocationValid(BelId bel) const override;
 
     // Helper function for above
diff --git a/ecp5/arch_place.cc b/ecp5/arch_place.cc
index 668b3141..5565a01c 100644
--- a/ecp5/arch_place.cc
+++ b/ecp5/arch_place.cc
@@ -82,38 +82,14 @@ bool Arch::isBelLocationValid(BelId bel) const
         return slices_compatible(bel_cells);
     } else {
         CellInfo *cell = getBoundBelCell(bel);
-        if (cell == nullptr)
+        if (cell == nullptr) {
+            return true;
+        } else if (cell->type == id_DCUA || cell->type == id_EXTREFB || cell->type == id_PCSCLKDIV) {
+            return args.type != ArchArgs::LFE5U_25F && args.type != ArchArgs::LFE5U_45F &&
+                   args.type != ArchArgs::LFE5U_85F;
+        } else {
             return true;
-        else
-            return isValidBelForCell(cell, bel);
-    }
-}
-
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
-{
-    if (cell->type == id_TRELLIS_SLICE) {
-        NPNR_ASSERT(getBelType(bel) == id_TRELLIS_SLICE);
-
-        std::vector<const CellInfo *> bel_cells;
-        Loc bel_loc = getBelLocation(bel);
-
-        if (cell->sliceInfo.has_l6mux && ((bel_loc.z % 2) == 1))
-            return false;
-
-        for (auto bel_other : getBelsByTile(bel_loc.x, bel_loc.y)) {
-            CellInfo *cell_other = getBoundBelCell(bel_other);
-            if (cell_other != nullptr && bel_other != bel) {
-                bel_cells.push_back(cell_other);
-            }
         }
-
-        bel_cells.push_back(cell);
-        return slices_compatible(bel_cells);
-    } else if (cell->type == id_DCUA || cell->type == id_EXTREFB || cell->type == id_PCSCLKDIV) {
-        return args.type != ArchArgs::LFE5U_25F && args.type != ArchArgs::LFE5U_45F && args.type != ArchArgs::LFE5U_85F;
-    } else {
-        // other checks
-        return true;
     }
 }
 
diff --git a/ecp5/arch_pybindings.cc b/ecp5/arch_pybindings.cc
index e1adbb46..5218ce36 100644
--- a/ecp5/arch_pybindings.cc
+++ b/ecp5/arch_pybindings.cc
@@ -44,9 +44,6 @@ void arch_wrap_python(py::module &m)
                            .def("place", &Context::place)
                            .def("route", &Context::route);
 
-    fn_wrapper_2a<Context, decltype(&Context::isValidBelForCell), &Context::isValidBelForCell, pass_through<bool>,
-                  addr_and_unwrap<CellInfo>, conv_from_str<BelId>>::def_wrap(ctx_cls, "isValidBelForCell");
-
     typedef std::unordered_map<IdString, std::unique_ptr<CellInfo>> CellMap;
     typedef std::unordered_map<IdString, std::unique_ptr<NetInfo>> NetMap;
     typedef std::unordered_map<IdString, IdString> AliasMap;
diff --git a/ecp5/globals.cc b/ecp5/globals.cc
index 1a0b7f2f..36bec64c 100644
--- a/ecp5/globals.cc
+++ b/ecp5/globals.cc
@@ -403,33 +403,35 @@ class Ecp5GlobalRouter
         bool dedicated_routing = false;
         for (auto bel : ctx->getBels()) {
             if (ctx->getBelType(bel) == id_DCCA && ctx->checkBelAvail(bel)) {
-                if (ctx->isValidBelForCell(dcc, bel)) {
-                    std::string belname = ctx->loc_info(bel)->bel_data[bel.index].name.get();
-                    if (belname.at(0) == 'D' && using_ce)
-                        continue; // don't allow DCCs with CE at center
-                    ctx->bindBel(bel, dcc, STRENGTH_LOCKED);
-                    wirelen_t wirelen = get_dcc_wirelen(dcc, dedicated_routing);
-                    if (wirelen < best_wirelen) {
-                        if (dedicated_routing) {
-                            best_bel_pclkcib = WireId();
-                        } else {
-                            bool found_pclkcib = false;
-                            for (WireId pclkcib : get_candidate_pclkcibs(bel)) {
-                                if (used_pclkcib.count(pclkcib))
-                                    continue;
-                                found_pclkcib = true;
-                                best_bel_pclkcib = pclkcib;
-                                break;
-                            }
-                            if (!found_pclkcib)
-                                goto pclkcib_fail;
+                std::string belname = ctx->loc_info(bel)->bel_data[bel.index].name.get();
+                if (belname.at(0) == 'D' && using_ce)
+                    continue; // don't allow DCCs with CE at center
+                ctx->bindBel(bel, dcc, STRENGTH_LOCKED);
+                if (!ctx->isBelLocationValid(bel)) {
+                    ctx->unbindBel(bel);
+                    continue;
+                }
+                wirelen_t wirelen = get_dcc_wirelen(dcc, dedicated_routing);
+                if (wirelen < best_wirelen) {
+                    if (dedicated_routing) {
+                        best_bel_pclkcib = WireId();
+                    } else {
+                        bool found_pclkcib = false;
+                        for (WireId pclkcib : get_candidate_pclkcibs(bel)) {
+                            if (used_pclkcib.count(pclkcib))
+                                continue;
+                            found_pclkcib = true;
+                            best_bel_pclkcib = pclkcib;
+                            break;
                         }
-                        best_bel = bel;
-                        best_wirelen = wirelen;
+                        if (!found_pclkcib)
+                            goto pclkcib_fail;
                     }
-                pclkcib_fail:
-                    ctx->unbindBel(bel);
+                    best_bel = bel;
+                    best_wirelen = wirelen;
                 }
+            pclkcib_fail:
+                ctx->unbindBel(bel);
             }
         }
         NPNR_ASSERT(best_bel != BelId());
diff --git a/fpga_interchange/arch.h b/fpga_interchange/arch.h
index b152bd0e..80eada4e 100644
--- a/fpga_interchange/arch.h
+++ b/fpga_interchange/arch.h
@@ -1280,17 +1280,6 @@ struct Arch : ArchAPI<ArchRanges>
         return bel_info(chip_info, bel).valid_cells[get_cell_type_index(cell_type)];
     }
 
-    // Whether or not a given cell can be placed at a given Bel
-    // This is not intended for Bel type checks, but finer-grained constraints
-    // such as conflicting set/reset signals, etc
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override
-    {
-        NPNR_ASSERT(isValidBelForCellType(cell->type, bel));
-
-        // FIXME: Implement this
-        return true;
-    }
-
     // Return true whether all Bels at a given location are valid
     bool isBelLocationValid(BelId bel) const override
     {
diff --git a/fpga_interchange/arch_pybindings.cc b/fpga_interchange/arch_pybindings.cc
index 416a015a..4fddad93 100644
--- a/fpga_interchange/arch_pybindings.cc
+++ b/fpga_interchange/arch_pybindings.cc
@@ -44,9 +44,6 @@ void arch_wrap_python(py::module &m)
                            .def("place", &Context::place)
                            .def("route", &Context::route);
 
-    fn_wrapper_2a<Context, decltype(&Context::isValidBelForCell), &Context::isValidBelForCell, pass_through<bool>,
-                  addr_and_unwrap<CellInfo>, conv_from_str<BelId>>::def_wrap(ctx_cls, "isValidBelForCell");
-
     typedef std::unordered_map<IdString, std::unique_ptr<CellInfo>> CellMap;
     typedef std::unordered_map<IdString, std::unique_ptr<NetInfo>> NetMap;
     typedef std::unordered_map<IdString, IdString> AliasMap;
diff --git a/generic/arch.cc b/generic/arch.cc
index 999e5033..a87c4392 100644
--- a/generic/arch.cc
+++ b/generic/arch.cc
@@ -650,21 +650,6 @@ TimingClockingInfo Arch::getPortClockingInfo(const CellInfo *cell, IdString port
     return tmg.clockingInfo.at(port).at(index);
 }
 
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
-{
-    std::vector<const CellInfo *> cells;
-    cells.push_back(cell);
-    Loc loc = getBelLocation(bel);
-    for (auto tbel : getBelsByTile(loc.x, loc.y)) {
-        if (tbel == bel)
-            continue;
-        CellInfo *bound = getBoundBelCell(tbel);
-        if (bound != nullptr)
-            cells.push_back(bound);
-    }
-    return cellsCompatible(cells.data(), int(cells.size()));
-}
-
 bool Arch::isBelLocationValid(BelId bel) const
 {
     std::vector<const CellInfo *> cells;
diff --git a/generic/arch.h b/generic/arch.h
index 007478eb..f3a6dccd 100644
--- a/generic/arch.h
+++ b/generic/arch.h
@@ -357,7 +357,6 @@ struct Arch : ArchAPI<ArchRanges>
     TimingClockingInfo getPortClockingInfo(const CellInfo *cell, IdString port, int index) const override;
 
     bool isValidBelForCellType(IdString cell_type, BelId bel) const override { return cell_type == getBelType(bel); }
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override;
     bool isBelLocationValid(BelId bel) const override;
 
     static const std::string defaultPlacer;
diff --git a/gowin/arch.cc b/gowin/arch.cc
index 8aeccfba..58df773c 100644
--- a/gowin/arch.cc
+++ b/gowin/arch.cc
@@ -1092,21 +1092,6 @@ TimingClockingInfo Arch::getPortClockingInfo(const CellInfo *cell, IdString port
     return tmg.clockingInfo.at(port).at(index);
 }
 
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
-{
-    std::vector<const CellInfo *> cells;
-    cells.push_back(cell);
-    Loc loc = getBelLocation(bel);
-    for (auto tbel : getBelsByTile(loc.x, loc.y)) {
-        if (tbel == bel)
-            continue;
-        CellInfo *bound = getBoundBelCell(tbel);
-        if (bound != nullptr)
-            cells.push_back(bound);
-    }
-    return cellsCompatible(cells.data(), int(cells.size()));
-}
-
 bool Arch::isBelLocationValid(BelId bel) const
 {
     std::vector<const CellInfo *> cells;
diff --git a/gowin/arch.h b/gowin/arch.h
index eab75899..3a1b2534 100644
--- a/gowin/arch.h
+++ b/gowin/arch.h
@@ -449,7 +449,6 @@ struct Arch : BaseArch<ArchRanges>
     // Get the TimingClockingInfo of a port
     TimingClockingInfo getPortClockingInfo(const CellInfo *cell, IdString port, int index) const;
 
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const;
     bool isBelLocationValid(BelId bel) const;
 
     static const std::string defaultPlacer;
diff --git a/ice40/arch.h b/ice40/arch.h
index 30b5f871..7280d2fe 100644
--- a/ice40/arch.h
+++ b/ice40/arch.h
@@ -834,11 +834,6 @@ struct Arch : BaseArch<ArchRanges>
     // Perform placement validity checks, returning false on failure (all
     // implemented in arch_place.cc)
 
-    // Whether or not a given cell can be placed at a given Bel
-    // This is not intended for Bel type checks, but finer-grained constraints
-    // such as conflicting set/reset signals, etc
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override;
-
     // Return true whether all Bels at a given location are valid
     bool isBelLocationValid(BelId bel) const override;
 
diff --git a/ice40/arch_place.cc b/ice40/arch_place.cc
index a450a7df..cc423a53 100644
--- a/ice40/arch_place.cc
+++ b/ice40/arch_place.cc
@@ -70,27 +70,6 @@ bool Arch::logic_cells_compatible(const CellInfo **it, const size_t size) const
     return locals_count <= 32;
 }
 
-bool Arch::isBelLocationValid(BelId bel) const
-{
-    if (getBelType(bel) == id_ICESTORM_LC) {
-        std::array<const CellInfo *, 8> bel_cells;
-        size_t num_cells = 0;
-        Loc bel_loc = getBelLocation(bel);
-        for (auto bel_other : getBelsByTile(bel_loc.x, bel_loc.y)) {
-            CellInfo *ci_other = getBoundBelCell(bel_other);
-            if (ci_other != nullptr)
-                bel_cells[num_cells++] = ci_other;
-        }
-        return logic_cells_compatible(bel_cells.data(), num_cells);
-    } else {
-        CellInfo *ci = getBoundBelCell(bel);
-        if (ci == nullptr)
-            return true;
-        else
-            return isValidBelForCell(ci, bel);
-    }
-}
-
 static inline bool _io_pintype_need_clk_in(unsigned pin_type) { return (pin_type & 0x01) == 0x00; }
 
 static inline bool _io_pintype_need_clk_out(unsigned pin_type)
@@ -103,116 +82,116 @@ static inline bool _io_pintype_need_clk_en(unsigned pin_type)
     return _io_pintype_need_clk_in(pin_type) || _io_pintype_need_clk_out(pin_type);
 }
 
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
+bool Arch::isBelLocationValid(BelId bel) const
 {
-    if (cell->type == id_ICESTORM_LC) {
-        NPNR_ASSERT(getBelType(bel) == id_ICESTORM_LC);
-
+    if (getBelType(bel) == id_ICESTORM_LC) {
         std::array<const CellInfo *, 8> bel_cells;
         size_t num_cells = 0;
-
         Loc bel_loc = getBelLocation(bel);
         for (auto bel_other : getBelsByTile(bel_loc.x, bel_loc.y)) {
             CellInfo *ci_other = getBoundBelCell(bel_other);
-            if (ci_other != nullptr && bel_other != bel)
+            if (ci_other != nullptr)
                 bel_cells[num_cells++] = ci_other;
         }
-
-        bel_cells[num_cells++] = cell;
         return logic_cells_compatible(bel_cells.data(), num_cells);
-    } else if (cell->type == id_SB_IO) {
-        // Do not allow placement of input SB_IOs on blocks where there a PLL is outputting to.
-
-        // Find shared PLL by looking for driving bel siblings from D_IN_0
-        // that are a PLL clock output.
-        auto wire = getBelPinWire(bel, id_D_IN_0);
-        for (auto pin : getWireBelPins(wire)) {
-            if (pin.pin == id_PLLOUT_A || pin.pin == id_PLLOUT_B) {
-                // Is there a PLL there ?
-                auto pll_cell = getBoundBelCell(pin.bel);
-                if (pll_cell == nullptr)
-                    break;
-
-                // Is that port actually used ?
-                if ((pin.pin == id_PLLOUT_B) && !is_sb_pll40_dual(this, pll_cell))
-                    break;
-
-                // Is that SB_IO used at an input ?
-                if ((cell->ports[id_D_IN_0].net == nullptr) && (cell->ports[id_D_IN_1].net == nullptr))
-                    break;
-
-                // Are we perhaps a PAD INPUT Bel that can be placed here?
-                if (pll_cell->attrs[id("BEL_PAD_INPUT")] == getBelName(bel).str(getCtx()))
-                    return true;
-
-                // Conflict
-                return false;
+    } else {
+        CellInfo *cell = getBoundBelCell(bel);
+        if (cell == nullptr)
+            return true;
+        else if (cell->type == id_SB_IO) {
+            // Do not allow placement of input SB_IOs on blocks where there a PLL is outputting to.
+
+            // Find shared PLL by looking for driving bel siblings from D_IN_0
+            // that are a PLL clock output.
+            auto wire = getBelPinWire(bel, id_D_IN_0);
+            for (auto pin : getWireBelPins(wire)) {
+                if (pin.pin == id_PLLOUT_A || pin.pin == id_PLLOUT_B) {
+                    // Is there a PLL there ?
+                    auto pll_cell = getBoundBelCell(pin.bel);
+                    if (pll_cell == nullptr)
+                        break;
+
+                    // Is that port actually used ?
+                    if ((pin.pin == id_PLLOUT_B) && !is_sb_pll40_dual(this, pll_cell))
+                        break;
+
+                    // Is that SB_IO used at an input ?
+                    if ((cell->ports[id_D_IN_0].net == nullptr) && (cell->ports[id_D_IN_1].net == nullptr))
+                        break;
+
+                    // Are we perhaps a PAD INPUT Bel that can be placed here?
+                    if (pll_cell->attrs[id("BEL_PAD_INPUT")] == getBelName(bel).str(getCtx()))
+                        return true;
+
+                    // Conflict
+                    return false;
+                }
             }
-        }
 
-        Loc ioLoc = getBelLocation(bel);
-        Loc compLoc = ioLoc;
-        compLoc.z = 1 - compLoc.z;
+            Loc ioLoc = getBelLocation(bel);
+            Loc compLoc = ioLoc;
+            compLoc.z = 1 - compLoc.z;
 
-        // Check LVDS pairing
-        if (cell->ioInfo.lvds) {
-            // Check correct z and complement location is free
-            if (ioLoc.z != 0)
-                return false;
-            BelId compBel = getBelByLocation(compLoc);
-            CellInfo *compCell = getBoundBelCell(compBel);
-            if (compCell)
-                return false;
-        } else {
-            // Check LVDS IO is not placed at complement location
-            BelId compBel = getBelByLocation(compLoc);
-            CellInfo *compCell = getBoundBelCell(compBel);
-            if (compCell && compCell->ioInfo.lvds)
-                return false;
+            // Check LVDS pairing
+            if (cell->ioInfo.lvds) {
+                // Check correct z and complement location is free
+                if (ioLoc.z != 0)
+                    return false;
+                BelId compBel = getBelByLocation(compLoc);
+                CellInfo *compCell = getBoundBelCell(compBel);
+                if (compCell)
+                    return false;
+            } else {
+                // Check LVDS IO is not placed at complement location
+                BelId compBel = getBelByLocation(compLoc);
+                CellInfo *compCell = getBoundBelCell(compBel);
+                if (compCell && compCell->ioInfo.lvds)
+                    return false;
 
-            // Check for conflicts on shared nets
-            // - CLOCK_ENABLE
-            // - OUTPUT_CLK
-            // - INPUT_CLK
-            if (compCell) {
-                bool use[6] = {
-                        _io_pintype_need_clk_in(cell->ioInfo.pintype),
-                        _io_pintype_need_clk_in(compCell->ioInfo.pintype),
-                        _io_pintype_need_clk_out(cell->ioInfo.pintype),
-                        _io_pintype_need_clk_out(compCell->ioInfo.pintype),
-                        _io_pintype_need_clk_en(cell->ioInfo.pintype),
-                        _io_pintype_need_clk_en(compCell->ioInfo.pintype),
-                };
-                NetInfo *nets[] = {
-                        cell->ports[id_INPUT_CLK].net,    compCell->ports[id_INPUT_CLK].net,
-                        cell->ports[id_OUTPUT_CLK].net,   compCell->ports[id_OUTPUT_CLK].net,
-                        cell->ports[id_CLOCK_ENABLE].net, compCell->ports[id_CLOCK_ENABLE].net,
-                };
-
-                for (int i = 0; i < 6; i++)
-                    if (use[i] && (nets[i] != nets[i ^ 1]) && (use[i ^ 1] || (nets[i ^ 1] != nullptr)))
-                        return false;
+                // Check for conflicts on shared nets
+                // - CLOCK_ENABLE
+                // - OUTPUT_CLK
+                // - INPUT_CLK
+                if (compCell) {
+                    bool use[6] = {
+                            _io_pintype_need_clk_in(cell->ioInfo.pintype),
+                            _io_pintype_need_clk_in(compCell->ioInfo.pintype),
+                            _io_pintype_need_clk_out(cell->ioInfo.pintype),
+                            _io_pintype_need_clk_out(compCell->ioInfo.pintype),
+                            _io_pintype_need_clk_en(cell->ioInfo.pintype),
+                            _io_pintype_need_clk_en(compCell->ioInfo.pintype),
+                    };
+                    NetInfo *nets[] = {
+                            cell->ports[id_INPUT_CLK].net,    compCell->ports[id_INPUT_CLK].net,
+                            cell->ports[id_OUTPUT_CLK].net,   compCell->ports[id_OUTPUT_CLK].net,
+                            cell->ports[id_CLOCK_ENABLE].net, compCell->ports[id_CLOCK_ENABLE].net,
+                    };
+
+                    for (int i = 0; i < 6; i++)
+                        if (use[i] && (nets[i] != nets[i ^ 1]) && (use[i ^ 1] || (nets[i ^ 1] != nullptr)))
+                            return false;
+                }
             }
-        }
 
-        return get_bel_package_pin(bel) != "";
-    } else if (cell->type == id_SB_GB) {
-        if (cell->gbInfo.forPadIn)
-            return true;
-        NPNR_ASSERT(cell->ports.at(id_GLOBAL_BUFFER_OUTPUT).net != nullptr);
-        const NetInfo *net = cell->ports.at(id_GLOBAL_BUFFER_OUTPUT).net;
-        int glb_id = get_driven_glb_netwk(bel);
-        if (net->is_reset && net->is_enable)
-            return false;
-        else if (net->is_reset)
-            return (glb_id % 2) == 0;
-        else if (net->is_enable)
-            return (glb_id % 2) == 1;
-        else
+            return get_bel_package_pin(bel) != "";
+        } else if (cell->type == id_SB_GB) {
+            if (cell->gbInfo.forPadIn)
+                return true;
+            NPNR_ASSERT(cell->ports.at(id_GLOBAL_BUFFER_OUTPUT).net != nullptr);
+            const NetInfo *net = cell->ports.at(id_GLOBAL_BUFFER_OUTPUT).net;
+            int glb_id = get_driven_glb_netwk(bel);
+            if (net->is_reset && net->is_enable)
+                return false;
+            else if (net->is_reset)
+                return (glb_id % 2) == 0;
+            else if (net->is_enable)
+                return (glb_id % 2) == 1;
+            else
+                return true;
+        } else {
+            // TODO: IO cell clock checks
             return true;
-    } else {
-        // TODO: IO cell clock checks
-        return true;
+        }
     }
 }
 
diff --git a/ice40/arch_pybindings.cc b/ice40/arch_pybindings.cc
index 76ce7590..6922887d 100644
--- a/ice40/arch_pybindings.cc
+++ b/ice40/arch_pybindings.cc
@@ -60,9 +60,6 @@ void arch_wrap_python(py::module &m)
                            .def("place", &Context::place)
                            .def("route", &Context::route);
 
-    fn_wrapper_2a<Context, decltype(&Context::isValidBelForCell), &Context::isValidBelForCell, pass_through<bool>,
-                  addr_and_unwrap<CellInfo>, conv_from_str<BelId>>::def_wrap(ctx_cls, "isValidBelForCell");
-
     typedef std::unordered_map<IdString, std::unique_ptr<CellInfo>> CellMap;
     typedef std::unordered_map<IdString, std::unique_ptr<NetInfo>> NetMap;
     typedef std::unordered_map<IdString, IdString> AliasMap;
diff --git a/machxo2/arch.cc b/machxo2/arch.cc
index 2938f1ba..74bfc598 100644
--- a/machxo2/arch.cc
+++ b/machxo2/arch.cc
@@ -448,14 +448,6 @@ bool Arch::route()
 }
 
 // ---------------------------------------------------------------
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
-{
-    // FIXME: Unlike ECP5, SLICEs in a given tile do not share a clock, so
-    // any SLICE Cell is valid for any BEL, even if some cells are already
-    // bound to BELs in the tile. However, this may need to be filled in once
-    // more than one LUT4 and DFF type is supported.
-    return true;
-}
 
 bool Arch::isBelLocationValid(BelId bel) const
 {
diff --git a/machxo2/arch.h b/machxo2/arch.h
index f1642490..adc6e25c 100644
--- a/machxo2/arch.h
+++ b/machxo2/arch.h
@@ -650,7 +650,6 @@ struct Arch : BaseArch<ArchRanges>
     bool route() override;
 
     // Placer
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override;
     bool isBelLocationValid(BelId bel) const override;
 
     static const std::string defaultPlacer;
diff --git a/machxo2/arch_pybindings.cc b/machxo2/arch_pybindings.cc
index fa0f9535..07e25437 100644
--- a/machxo2/arch_pybindings.cc
+++ b/machxo2/arch_pybindings.cc
@@ -1,3 +1,4 @@
+
 /*
  *  nextpnr -- Next Generation Place and Route
  *
@@ -44,9 +45,6 @@ void arch_wrap_python(py::module &m)
                            .def("place", &Context::place)
                            .def("route", &Context::route);
 
-    fn_wrapper_2a<Context, decltype(&Context::isValidBelForCell), &Context::isValidBelForCell, pass_through<bool>,
-                  addr_and_unwrap<CellInfo>, conv_from_str<BelId>>::def_wrap(ctx_cls, "isValidBelForCell");
-
     typedef std::unordered_map<IdString, std::unique_ptr<CellInfo>> CellMap;
     typedef std::unordered_map<IdString, std::unique_ptr<NetInfo>> NetMap;
     typedef std::unordered_map<IdString, IdString> AliasMap;
diff --git a/nexus/arch.h b/nexus/arch.h
index 963b5b2f..f1332a42 100644
--- a/nexus/arch.h
+++ b/nexus/arch.h
@@ -1209,11 +1209,6 @@ struct Arch : BaseArch<ArchRanges>
     // Perform placement validity checks, returning false on failure (all
     // implemented in arch_place.cc)
 
-    // Whether or not a given cell can be placed at a given Bel
-    // This is not intended for Bel type checks, but finer-grained constraints
-    // such as conflicting set/reset signals, etc
-    bool isValidBelForCell(CellInfo *cell, BelId bel) const override;
-
     // Return true whether all Bels at a given location are valid
     bool isBelLocationValid(BelId bel) const override;
 
diff --git a/nexus/arch_place.cc b/nexus/arch_place.cc
index 0d141013..dab1c0a2 100644
--- a/nexus/arch_place.cc
+++ b/nexus/arch_place.cc
@@ -96,12 +96,6 @@ bool Arch::nexus_logic_tile_valid(LogicTileStatus &lts) const
     return true;
 }
 
-bool Arch::isValidBelForCell(CellInfo *cell, BelId bel) const
-{
-    // FIXME
-    return true;
-}
-
 bool Arch::isBelLocationValid(BelId bel) const
 {
     if (bel_tile_is(bel, LOC_LOGIC)) {