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>

3 files changed, 94 insertions, 123 deletions

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;