first round of review fixes

CMakeLists.txt

@@ -34,7 +34,6 @@ option(USE_RUST "Enable Rust bindings" OFF)
 if (USE_RUST)
     add_subdirectory(3rdparty/corrosion)
     corrosion_import_crate(MANIFEST_PATH rust/Cargo.toml PROFILE "release" IMPORTED_CRATES RUST_CRATES)
-    aux_source_directory(rust RUST_FILES)
 endif()
 
 if (USE_IPO)

rust/nextpnr/src/lib.rs

@@ -157,7 +157,10 @@ impl Context {
     }
 
     /// Bind a given bel to a given cell with the given strength.
-    pub fn bind_bel(&mut self, bel: BelId, cell: *mut CellInfo, strength: PlaceStrength) {
+    ///
+    /// # Safety
+    /// `cell` must be valid and not null.
+    pub unsafe fn bind_bel(&mut self, bel: BelId, cell: *mut CellInfo, strength: PlaceStrength) {
         unsafe { npnr_context_bind_bel(self, bel, cell, strength) }
     }
 
@@ -172,7 +175,10 @@ impl Context {
     }
 
     /// Bind a wire to a net. This method must be used when binding a wire that is driven by a bel pin. Use bindPip() when binding a wire that is driven by a pip.
-    pub fn bind_wire(&mut self, wire: WireId, net: *mut NetInfo, strength: PlaceStrength) {
+    ///
+    /// # Safety
+    /// `net` must be valid and not null.
+    pub unsafe fn bind_wire(&mut self, wire: WireId, net: *mut NetInfo, strength: PlaceStrength) {
         unsafe { npnr_context_bind_wire(self, wire, net, strength) }
     }
 
@@ -181,8 +187,11 @@ impl Context {
         unsafe { npnr_context_unbind_wire(self, wire) }
     }
 
-    /// Bid a pip to a net. This also bind the destination wire of that pip.
+    /// Bind a pip to a net. This also binds the destination wire of that pip.
-    pub fn bind_pip(&mut self, pip: PipId, net: *mut NetInfo, strength: PlaceStrength) {
+    ///
+    /// # Safety
+    /// `net` must be valid and not null.
+    pub unsafe fn bind_pip(&mut self, pip: PipId, net: *mut NetInfo, strength: PlaceStrength) {
         unsafe { npnr_context_bind_pip(self, pip, net, strength) }
     }
 
@@ -218,11 +227,15 @@ impl Context {
         unsafe { npnr_context_delay_epsilon(self) }
     }
 
-    pub fn source_wire(&self, net: *const NetInfo) -> WireId {
+    /// # Safety
+    /// `net` must be valid and not null.
+    pub unsafe fn source_wire(&self, net: *const NetInfo) -> WireId {
         unsafe { npnr_context_get_netinfo_source_wire(self, net) }
     }
 
-    pub fn sink_wires(&self, net: *const NetInfo, sink: *const PortRef) -> Vec<WireId> {
+    /// # Safety
+    /// `net` and `sink` must be valid and not null.
+    pub unsafe fn sink_wires(&self, net: *const NetInfo, sink: *const PortRef) -> Vec<WireId> {
         let mut v = Vec::new();
         let mut n = 0;
         loop {
@@ -269,10 +282,39 @@ impl Context {
     }
 
     pub fn pip_direction(&self, pip: PipId) -> Loc {
-        unsafe { npnr_context_get_pip_direction(self, pip) }
+        let mut src = Loc{x: 0, y: 0, z: 0};
+        let mut dst = Loc{x: 0, y: 0, z: 0};
+
+        let mut pips = 0;
+        for pip in self.get_uphill_pips(self.pip_src_wire(pip)) {
+            let loc = self.pip_location(pip);
+            src.x += loc.x;
+            src.y += loc.y;
+            pips += 1;
+        }
+        if pips != 0 {
+            src.x /= pips;
+            src.y /= pips;
+        }
+
+        let mut pips = 0;
+        for pip in self.get_downhill_pips(self.pip_dst_wire(pip)) {
+            let loc = self.pip_location(pip);
+            dst.x += loc.x;
+            dst.y += loc.y;
+            pips += 1;
+        }
+        if pips != 0 {
+            dst.x /= pips;
+            dst.y /= pips;
+        }
+
+        Loc{x: dst.x - src.x, y: dst.y - src.y, z: 0}
     }
 
-    pub fn pip_avail_for_net(&self, pip: PipId, net: *mut NetInfo) -> bool {
+    /// # Safety
+    /// `net` must be valid and not null.
+    pub unsafe fn pip_avail_for_net(&self, pip: PipId, net: *mut NetInfo) -> bool {
         unsafe { npnr_context_check_pip_avail_for_net(self, pip, net) }
     }
 
@@ -349,7 +391,6 @@ extern "C" {
     fn npnr_context_get_wires_leak(ctx: *const Context, wires: *mut *mut WireId) -> u64;
     fn npnr_context_get_pips_leak(ctx: *const Context, pips: *mut *mut PipId) -> u64;
     fn npnr_context_get_pip_location(ctx: *const Context, pip: PipId) -> Loc;
-    fn npnr_context_get_pip_direction(ctx: *const Context, pip: PipId) -> Loc;
     fn npnr_context_check_pip_avail_for_net(
         ctx: *const Context,
         pip: PipId,
@@ -404,7 +445,6 @@ pub struct Nets<'a> {
     nets: HashMap<IdString, *mut NetInfo>,
     users: HashMap<IdString, &'a [&'a mut PortRef]>,
     index_to_net: Vec<IdString>,
-    net_to_index: HashMap<*mut NetInfo, i32>,
     _data: PhantomData<&'a Context>,
 }
 
@@ -428,7 +468,6 @@ impl<'a> Nets<'a> {
         let mut nets = HashMap::new();
         let mut users = HashMap::new();
         let mut index_to_net = Vec::new();
-        let mut net_to_index = HashMap::new();
         for i in 0..size {
             let name = unsafe { IdString(*names.add(i as usize)) };
             let net = unsafe { *nets_ptr.add(i as usize) };
@@ -443,7 +482,6 @@ impl<'a> Nets<'a> {
             users.insert(name, users_slice);
             let index = index_to_net.len() as i32;
             index_to_net.push(name);
-            net_to_index.insert(net, index);
             unsafe {
                 npnr_netinfo_udata_set(net, NetIndex(index));
             }
@@ -453,7 +491,6 @@ impl<'a> Nets<'a> {
             nets,
             users,
             index_to_net,
-            net_to_index,
             _data: PhantomData,
         }
     }
@@ -468,6 +505,10 @@ impl<'a> Nets<'a> {
         self.nets.len()
     }
 
+    pub fn is_empty(&self) -> bool {
+        self.nets.len() == 0
+    }
+
     pub fn name_from_index(&self, index: NetIndex) -> IdString {
         self.index_to_net[index.0 as usize]
     }

rust/rust.cc

@@ -72,173 +72,81 @@ struct UphillIterWrapper {
 extern "C" {
     USING_NEXTPNR_NAMESPACE;
 
-    /*
+    void npnr_log_info(const char *string) { log_info("%s", string); }
-        DONE:
+    void npnr_log_error(const char *string) { log_error("%s", string); }
-        ctx->bindPip
-        ctx->bindWire
-        ctx->check
-        ctx->debug
-        ctx->estimateDelay
-        ctx->getDelayEpsilon
-        ctx->getPipDstWire
-        ctx->getPipSrcWire
-        ctx->getGridDimX
-        ctx->getGridDimY
-        ctx->id
-        ctx->nameOf
-        ctx->unbindWire
-        ctx->verbose
-
-        UNNECESSARY:
-        ctx->getDelayNS - all FFI calls go through it anyway.
-
-        TODO:
-        ctx->checkPipAvail
-        ctx->checkPipAvailForNet
-        ctx->checkWireAvail
-        ctx->getBelPinType
-        ctx->getBoundPipNet
-        ctx->getBoundWireNet
-        ctx->getNetinfoSinkWire
-        ctx->getNetinfoSinkWires
-        ctx->getNetinfoSourceWire
-        ctx->getPipDelay
-        ctx->getPipLocation
-        ctx->getPipsDownhill
-        ctx->getPipsUphill
-        ctx->getRouteBoundingBox
-        ctx->getWireBelPins
-        ctx->getWireDelay
-        ctx->getWires
-        ctx->getWireType
-        ctx->nameOfPip
-        ctx->nameOfWire
-        ctx->nets
-        ctx->nets.at
-        ctx->nets.size
-        ctx->rng64
-        ctx->setting<bool>
-        ctx->setting<float>
-        ctx->setting<int>
-        ctx->sorted_shuffle
-    */
-
-    void npnr_log_info(const char *const format) { log_info("%s", format); }
-    void npnr_log_error(const char *const format) { log_error("%s", format); }
 
     uint64_t npnr_belid_null() { return wrap(BelId()); }
     uint64_t npnr_wireid_null() { return wrap(WireId()); }
     uint64_t npnr_pipid_null() { return wrap(PipId()); }
 
-    int npnr_context_get_grid_dim_x(const Context *const ctx) { return ctx->getGridDimX(); }
+    int npnr_context_get_grid_dim_x(const Context *ctx) { return ctx->getGridDimX(); }
-    int npnr_context_get_grid_dim_y(const Context *const ctx) { return ctx->getGridDimY(); }
+    int npnr_context_get_grid_dim_y(const Context *ctx) { return ctx->getGridDimY(); }
     void npnr_context_bind_bel(Context *ctx, uint64_t bel, CellInfo* cell, PlaceStrength strength) { return ctx->bindBel(unwrap_bel(bel), cell, strength); }
     void npnr_context_unbind_bel(Context *ctx, uint64_t bel) { return ctx->unbindBel(unwrap_bel(bel)); }
-    bool npnr_context_check_bel_avail(Context *const ctx, uint64_t bel) { return ctx->checkBelAvail(unwrap_bel(bel)); }
+    bool npnr_context_check_bel_avail(Context *ctx, uint64_t bel) { return ctx->checkBelAvail(unwrap_bel(bel)); }
     void npnr_context_bind_wire(Context *ctx, uint64_t wire, NetInfo* net, PlaceStrength strength) { ctx->bindWire(unwrap_wire(wire), net, strength); }
     void npnr_context_unbind_wire(Context *ctx, uint64_t wire) { ctx->unbindWire(unwrap_wire(wire)); }
     void npnr_context_bind_pip(Context *ctx, uint64_t pip, NetInfo* net, PlaceStrength strength) { ctx->bindPip(unwrap_pip(pip), net, strength); }
     void npnr_context_unbind_pip(Context *ctx, uint64_t pip) { ctx->unbindPip(unwrap_pip(pip)); }
-    uint64_t npnr_context_get_pip_src_wire(const Context *const ctx, uint64_t pip) { return wrap(ctx->getPipSrcWire(unwrap_pip(pip))); }
+    uint64_t npnr_context_get_pip_src_wire(const Context *ctx, uint64_t pip) { return wrap(ctx->getPipSrcWire(unwrap_pip(pip))); }
-    uint64_t npnr_context_get_pip_dst_wire(const Context *const ctx, uint64_t pip) { return wrap(ctx->getPipDstWire(unwrap_pip(pip))); }
+    uint64_t npnr_context_get_pip_dst_wire(const Context *ctx, uint64_t pip) { return wrap(ctx->getPipDstWire(unwrap_pip(pip))); }
-    float npnr_context_estimate_delay(const Context *const ctx, uint64_t src, uint64_t dst) { return ctx->getDelayNS(ctx->estimateDelay(unwrap_wire(src), unwrap_wire(dst))); }
+    float npnr_context_estimate_delay(const Context *ctx, uint64_t src, uint64_t dst) { return ctx->getDelayNS(ctx->estimateDelay(unwrap_wire(src), unwrap_wire(dst))); }
-    float npnr_context_get_pip_delay(const Context *const ctx, uint64_t pip) { return ctx->getDelayNS(ctx->getPipDelay(unwrap_pip(pip)).maxDelay()); }
+    float npnr_context_get_pip_delay(const Context *ctx, uint64_t pip) { return ctx->getDelayNS(ctx->getPipDelay(unwrap_pip(pip)).maxDelay()); }
-    float npnr_context_get_wire_delay(const Context *const ctx, uint64_t wire) { return ctx->getDelayNS(ctx->getWireDelay(unwrap_wire(wire)).maxDelay()); }
+    float npnr_context_get_wire_delay(const Context *ctx, uint64_t wire) { return ctx->getDelayNS(ctx->getWireDelay(unwrap_wire(wire)).maxDelay()); }
-    float npnr_context_delay_epsilon(const Context *const ctx) { return ctx->getDelayNS(ctx->getDelayEpsilon()); }
+    float npnr_context_delay_epsilon(const Context *ctx) { return ctx->getDelayNS(ctx->getDelayEpsilon()); }
-    Loc npnr_context_get_pip_location(const Context *const ctx, uint64_t pip) { return ctx->getPipLocation(unwrap_pip(pip)); }
+    Loc npnr_context_get_pip_location(const Context *ctx, uint64_t pip) { return ctx->getPipLocation(unwrap_pip(pip)); }
-    bool npnr_context_check_pip_avail_for_net(const Context *const ctx, uint64_t pip, NetInfo *net) { return ctx->checkPipAvailForNet(unwrap_pip(pip), net); }
+    bool npnr_context_check_pip_avail_for_net(const Context *ctx, uint64_t pip, NetInfo *net) { return ctx->checkPipAvailForNet(unwrap_pip(pip), net); }
 
-    // This method's in C++ temporarily, while I figure out some better way of getting a pip iterator.
+    uint64_t npnr_context_get_pips_leak(const Context *ctx, uint64_t **pips) {
-    Loc npnr_context_get_pip_direction(const Context *const ctx, uint64_t _pip) {
+        auto size = size_t{};
-        auto pip = unwrap_pip(_pip);
+        for (auto _pip : ctx->getPips())
-        auto src_loc = Loc{};
+            size++;
-        auto dst_loc = Loc{};
+        auto *pip_vec = new std::vector<uint64_t>{};
-
+        pip_vec->reserve(size);
-        auto uh_pips = 0;
+        for (auto pip : ctx->getPips())
-        for (auto uh : ctx->getPipsUphill(ctx->getPipSrcWire(pip))) {
+            pip_vec->push_back(wrap(pip));
-            auto loc = ctx->getPipLocation(uh);
+        *pips = pip_vec->data();
-            src_loc.x += loc.x;
+        // Yes, by never deleting pip_vec, we leak memory.
-            src_loc.y += loc.y;
-            uh_pips++;
-        }
-        if (uh_pips > 1) {
-            src_loc.x /= uh_pips;
-            src_loc.y /= uh_pips;
-        }
-
-        auto dh_pips = 0;
-        for (auto dh : ctx->getPipsDownhill(ctx->getPipDstWire(pip))) {
-            auto loc = ctx->getPipLocation(dh);
-            dst_loc.x += loc.x;
-            dst_loc.y += loc.y;
-            dh_pips++;
-        }
-        if (dh_pips > 1) {
-            dst_loc.x /= dh_pips;
-            dst_loc.y /= dh_pips;
-        }
-
-        dst_loc.x -= src_loc.x;
-        dst_loc.y -= src_loc.y;
-        return dst_loc;
-    }
-
-    uint64_t npnr_context_get_pips_leak(const Context *const ctx, PipId **pips) {
-        auto pip_vec = std::vector<PipId>{};
-        for (auto pip : ctx->getPips()) {
-            pip_vec.push_back(pip);
-        }
-        pip_vec.shrink_to_fit();
-        auto size = pip_vec.size();
-        *pips = pip_vec.data();
-        auto dummy = std::vector<PipId>{};
-        // Yes, by memcpying over `pip_vec` we leak memory.
-        std::memcpy(&pip_vec, &dummy, sizeof(dummy));
         return size;
     }
 
-    uint64_t npnr_context_get_wires_leak(const Context *const ctx, WireId **wires) {
+    uint64_t npnr_context_get_wires_leak(const Context *ctx, uint64_t **wires) {
-        auto wire_vec = std::vector<WireId>{};
+        auto size = size_t{};
-        for (auto wire : ctx->getWires()) {
+        for (auto _wire : ctx->getWires())
-            wire_vec.push_back(wire);
+            size++;
-        }
+        auto *wire_vec = new std::vector<uint64_t>{};
-        wire_vec.shrink_to_fit();
+        wire_vec->reserve(size);
-        auto size = wire_vec.size();
+        for (auto wire : ctx->getWires())
-        *wires = wire_vec.data();
+            wire_vec->push_back(wrap(wire));
-        auto dummy = std::vector<WireId>{};
+        *wires = wire_vec->data();
-        // Yes, by memcpying over `wire_vec` we leak memory.
+        // Yes, by never deleting wire_vec, we leak memory.
-        std::memcpy(&wire_vec, &dummy, sizeof(dummy));
         return size;
     }
 
-    void npnr_context_check(const Context *const ctx) { ctx->check(); }
+    void npnr_context_check(const Context *ctx) { ctx->check(); }
-    bool npnr_context_debug(const Context *const ctx) { return ctx->debug; }
+    bool npnr_context_debug(const Context *ctx) { return ctx->debug; }
-    int npnr_context_id(const Context *const ctx, const char *const str) { return ctx->id(str).hash(); }
+    int npnr_context_id(const Context *ctx, const char *str) { return ctx->id(str).hash(); }
-    const char *npnr_context_name_of(const Context *const ctx, IdString str) { return ctx->nameOf(str); }
+    const char *npnr_context_name_of(const Context *ctx, IdString str) { return ctx->nameOf(str); }
-    const char *npnr_context_name_of_pip(const Context *const ctx, uint64_t pip) { return ctx->nameOfPip(unwrap_pip(pip)); }
+    const char *npnr_context_name_of_pip(const Context *ctx, uint64_t pip) { return ctx->nameOfPip(unwrap_pip(pip)); }
-    const char *npnr_context_name_of_wire(const Context *const ctx, uint64_t wire) { return ctx->nameOfWire(unwrap_wire(wire)); }
+    const char *npnr_context_name_of_wire(const Context *ctx, uint64_t wire) { return ctx->nameOfWire(unwrap_wire(wire)); }
-    bool npnr_context_verbose(const Context *const ctx) { return ctx->verbose; }
+    bool npnr_context_verbose(const Context *ctx) { return ctx->verbose; }
 
-    uint64_t npnr_context_get_netinfo_source_wire(const Context *const ctx, const NetInfo *const net) { return wrap(ctx->getNetinfoSourceWire(net)); }
+    uint64_t npnr_context_get_netinfo_source_wire(const Context *ctx, const NetInfo *net) { return wrap(ctx->getNetinfoSourceWire(net)); }
-    uint64_t npnr_context_get_netinfo_sink_wire(const Context *const ctx, const NetInfo *const net, const PortRef *const sink, uint32_t n) { return wrap(ctx->getNetinfoSinkWire(net, *sink, n)); }
+    uint64_t npnr_context_get_netinfo_sink_wire(const Context *ctx, const NetInfo *net, const PortRef *sink, uint32_t n) { return wrap(ctx->getNetinfoSinkWire(net, *sink, n)); }
 
-    uint32_t npnr_context_nets_leak(const Context *const ctx, int **names, NetInfo ***nets) {
+    uint32_t npnr_context_nets_leak(const Context *ctx, int **names, NetInfo ***nets) {
-        auto name_vec = std::vector<int>{};
+        auto size = ctx->nets.size();
-        auto nets_vec = std::vector<NetInfo*>{};
+        auto *name_vec = new std::vector<int>{};
+        auto *nets_vec = new std::vector<NetInfo*>{};
+        name_vec->reserve(size);
+        nets_vec->reserve(size);
         for (auto& item : ctx->nets) {
-            name_vec.push_back(item.first.hash());
+            name_vec->push_back(item.first.hash());
-            nets_vec.push_back(item.second.get());
+            nets_vec->push_back(item.second.get());
         }
-        name_vec.shrink_to_fit();
+        *names = name_vec->data();
-        nets_vec.shrink_to_fit();
+        *nets = nets_vec->data();
-        auto size = name_vec.size();
+        // Yes, by never deleting `name_vec` and `nets_vec` we leak memory.
-        *names = name_vec.data();
-        *nets = nets_vec.data();
-        // Yes, by memcpying over `name_vec` and `nets_vec` we leak memory.
-        auto dummy1 = std::vector<int>{};
-        auto dummy2 = std::vector<NetInfo*>{};
-        std::memcpy(&name_vec, &dummy1, sizeof(dummy1));
-        std::memcpy(&nets_vec, &dummy2, sizeof(dummy2));
         return size;
     }
 
@@ -259,14 +167,14 @@ extern "C" {
         delete iter;
     }
 
-    PortRef* npnr_netinfo_driver(NetInfo *const net) {
+    PortRef* npnr_netinfo_driver(NetInfo *net) {
         if (net == nullptr) {
             return nullptr;
         }
         return &net->driver;
     }
 
-    uint32_t npnr_netinfo_users_leak(NetInfo *const net, PortRef ***users) {
+    uint32_t npnr_netinfo_users_leak(NetInfo *net, PortRef ***users) {
         auto x = std::vector<PortRef*>{};
         for (auto& item : net->users) {
             x.push_back(&item);
@@ -281,16 +189,16 @@ extern "C" {
     }
 
 #ifdef ARCH_ECP5
-    bool npnr_netinfo_is_global(NetInfo *const net) { return net->is_global; }
+    bool npnr_netinfo_is_global(NetInfo *net) { return net->is_global; }
 #else
-    bool npnr_netinfo_is_global(NetInfo *const net) { return false; }
+    bool npnr_netinfo_is_global(NetInfo *net) { return false; }
 #endif
 
-    int32_t npnr_netinfo_udata(NetInfo *const net) { return net->udata; }
+    int32_t npnr_netinfo_udata(NetInfo *net) { return net->udata; }
-    void npnr_netinfo_udata_set(NetInfo *const net, int32_t value) { net->udata = value; }
+    void npnr_netinfo_udata_set(NetInfo *net, int32_t value) { net->udata = value; }
 
-    CellInfo* npnr_portref_cell(const PortRef *const port) { return port->cell; }
+    CellInfo* npnr_portref_cell(const PortRef *port) { return port->cell; }
-    Loc npnr_cellinfo_get_location(const CellInfo *const info) { return info->getLocation(); }
+    Loc npnr_cellinfo_get_location(const CellInfo *info) { return info->getLocation(); }
 
     void npnr_inc_downhill_iter(DownhillIterWrapper *iter) {
         ++iter->current;