Add documentation and clean up AtomPBBimap

Author: AmirhosseinPoolad

vpr/src/base/atom_lookup.h

@@ -27,15 +27,36 @@ class AtomLookup {
 
     typedef vtr::Range<pin_tnode_iterator> pin_tnode_range;
 
-    bool lock_atom_pb_bimap = false;
+    
 
   public:
     /*
      * PBs
      */
 
-    inline AtomPBBimap &mutable_atom_pb_bimap() {VTR_ASSERT(!lock_atom_pb_bimap); return atom_to_pb_bimap_;}
-    inline const AtomPBBimap &atom_pb_bimap() const {VTR_ASSERT(!lock_atom_pb_bimap); return atom_to_pb_bimap_;}
+
+    /**
+     * @brief Sets the atom to pb bimap access lock to value.
+     * If set to true, access to the bimap is prohibited and will result in failing assertions.
+     * 
+     * @param value Value to set to lock to
+     */
+    inline void set_atom_pb_bimap_lock(bool value) {
+        VTR_ASSERT_SAFE_MSG(lock_atom_pb_bimap_ != value, "Double locking or unlocking the atom pb bimap lock");
+        lock_atom_pb_bimap_ = value;
+      }
+    
+    /// @brief Gets the current atom to pb bimap lock value.
+    inline bool atom_pb_bimap_islocked() {return lock_atom_pb_bimap_;}
+
+
+    // All accesses, mutable or immutable, to the atom to pb bimap
+    // will result in failing assertions if the lock is set to true.
+    // This is done to make sure there is only a single source of
+    // data in places that are supposed to use a local data structure
+    // instead of the global context.
+    inline AtomPBBimap &mutable_atom_pb_bimap() {VTR_ASSERT(!lock_atom_pb_bimap_); return atom_to_pb_bimap_;}
+    inline const AtomPBBimap &atom_pb_bimap() const {VTR_ASSERT(!lock_atom_pb_bimap_); return atom_to_pb_bimap_;}
 
     /**
      * @brief Set atom to pb bimap
@@ -107,6 +128,12 @@ class AtomLookup {
 
   private: //Types
   private:
+    
+  /**
+   * @brief Allows or disallows access to the AtomPBBimap data.
+   * Useful to make sure global context is not accessed in places you don't want it to.
+   */  
+    bool lock_atom_pb_bimap_ = false;
     AtomPBBimap atom_to_pb_bimap_;
 
     vtr::vector_map<AtomPinId, const t_pb_graph_pin*> atom_pin_to_pb_graph_pin_;

vpr/src/pack/atom_pb_bimap.cpp

@@ -1,4 +1,14 @@
+/**
+ * @file 
+ * @author  Amir Poolad
+ * @date    March 2025
+ * @brief   The code for the AtomPBBimap class.
+ * 
+ * This file implements the various functions of the AtomPBBimap class.
+ */
+
 #include "atom_pb_bimap.h"
+#include "atom_netlist.h"
 
 AtomPBBimap::AtomPBBimap(const vtr::bimap<AtomBlockId, const t_pb*, vtr::linear_map, std::unordered_map>& atom_to_pb) {
     atom_to_pb_ = atom_to_pb;
@@ -34,7 +44,6 @@ const t_pb_graph_node* AtomPBBimap::atom_pb_graph_node(const AtomBlockId blk_id)
 void AtomPBBimap::set_atom_pb(const AtomBlockId blk_id, const t_pb* pb) {
     //If either of blk_id or pb are not valid,
     //remove any mapping
-
     if (!blk_id && pb) {
         //Remove
         atom_to_pb_.erase(pb);
@@ -46,3 +55,9 @@ void AtomPBBimap::set_atom_pb(const AtomBlockId blk_id, const t_pb* pb) {
         atom_to_pb_.update(blk_id, pb);
     }
 }
+
+void AtomPBBimap::reset_bimap(const AtomNetlist &netlist) {
+    for (auto blk : netlist.blocks()) {
+        set_atom_pb(blk, nullptr);
+    }
+}

vpr/src/pack/atom_pb_bimap.h

@@ -1,3 +1,13 @@
+/**
+ * @file 
+ * @author  Amir Poolad
+ * @date    March 2025
+ * @brief   The declaration of the AtomPBBimap class.
+ * 
+ * This file declares a class called AtomPBBimap that
+ * contains a two way mapping between AtomBlockIds and pb types.
+ */
+
 #pragma once
 
 #include "vpr_types.h"
@@ -37,6 +47,10 @@ class AtomPBBimap {
      */
     void set_atom_pb(const AtomBlockId blk_id, const t_pb* pb);
 
+    /// @brief Sets the pb for all blocks in the netlist to nullptr.
+    void reset_bimap(const AtomNetlist &netlist);
+
     private:
+        /// @brief Two way map between AtomBlockIds and t_pb
         vtr::bimap<AtomBlockId, const t_pb*, vtr::linear_map, std::unordered_map> atom_to_pb_;
     };

vpr/src/pack/cluster_legalizer.cpp

@@ -39,10 +39,10 @@
 #include "vtr_vector_map.h"
 
 /*
-* @brief Allocates the stats stored within the pb of a cluster.
-*
-* Used to store information used during clustering.
-*/
+ * @brief Allocates the stats stored within the pb of a cluster.
+ *
+ * Used to store information used during clustering.
+ */
 static void alloc_and_load_pb_stats(t_pb* pb) {
      /* Call this routine when starting to fill up a new cluster.  It resets *
       * the gain vector, etc.                                                */
@@ -58,8 +58,8 @@ static void alloc_and_load_pb_stats(t_pb* pb) {
 }
 
 /*
-* @brief Check the atom blocks of a cluster pb. Used in the verify method.
-*/
+ * @brief Check the atom blocks of a cluster pb. Used in the verify method.
+ */
 /* TODO: May want to check that all atom blocks are actually reached */
 static void check_cluster_atom_blocks(t_pb* pb, std::unordered_set<AtomBlockId>& blocks_checked, const AtomPBBimap &atom_pb_lookup) {
     const AtomContext& atom_ctx = g_vpr_ctx.atom();
@@ -118,7 +118,6 @@ static void free_pb_stats_recursive(t_pb* pb) {
     }
 }
 
-
 /**
  * @brief Checks whether an atom block can be added to a clustered block
  *        without violating floorplanning constraints. It also updates the
@@ -244,11 +243,12 @@ static bool check_cluster_noc_group(AtomBlockId atom_blk_id,
     // If the cluster belongs to a different NoC group than the atom's group,
     // they are incompatible.
     VTR_LOGV(log_verbosity > 3,
-            "\t\t\t NoC Group: Atom block %d failed NoC group check for cluster. Cluster's NoC group: %d, atom's NoC group: %d\n",
-            atom_blk_id, (size_t)cluster_noc_grp_id, (size_t)atom_noc_grp_id);
-    return false;
+        "\t\t\t NoC Group: Atom block %d failed NoC group check for cluster. Cluster's NoC group: %d, atom's NoC group: %d\n",
+        atom_blk_id, (size_t)cluster_noc_grp_id, (size_t)atom_noc_grp_id);
+return false;
 }
 
+
 /**
  * @brief This function takes the root block of a chain molecule and a proposed
  *        placement primitive for this block. The function then checks if this
@@ -260,7 +260,7 @@ static enum e_block_pack_status check_chain_root_placement_feasibility(const t_p
                                                                        const t_clustering_chain_info& clustering_chain_info,
                                                                        t_pack_patterns* mol_pack_patterns,
                                                                        const AtomBlockId blk_id) {
-    const AtomNetlist& netlist_context = g_vpr_ctx.atom().netlist();
+    const AtomNetlist& atom_netlist = g_vpr_ctx.atom().netlist();
 
     enum e_block_pack_status block_pack_status = e_block_pack_status::BLK_PASSED;
 
@@ -270,10 +270,10 @@ static enum e_block_pack_status check_chain_root_placement_feasibility(const t_p
 
     t_model_ports* root_port = chain_root_pins[0][0]->port->model_port;
     AtomNetId chain_net_id;
-    auto port_id = netlist_context.find_atom_port(blk_id, root_port);
+    auto port_id = atom_netlist.find_atom_port(blk_id, root_port);
 
     if (port_id) {
-        chain_net_id = netlist_context.port_net(port_id, chain_root_pins[0][0]->pin_number);
+        chain_net_id = atom_netlist.port_net(port_id, chain_root_pins[0][0]->pin_number);
     }
 
     // if this block is part of a long chain or it is driven by a cluster

vpr/src/pack/cluster_legalizer.h

@@ -29,7 +29,6 @@ class Prepacker;
 class t_intra_cluster_placement_stats;
 class t_pb_graph_node;
 struct t_lb_router_data;
-class UserPlaceConstraints;
 
 // A special ID to identify the legalization clusters. This is separate from the
 // ClusterBlockId since this legalizer is not necessarily tied to the Clustered
@@ -192,7 +191,6 @@ struct LegalizationCluster {
  *
  * // new_cluster_id now contains a fully legalized cluster.
  */
-
 class ClusterLegalizer {
   public:
     // Iterator for the legalization cluster IDs
@@ -593,6 +591,9 @@ class ClusterLegalizer {
     /// @brief The prepacker object that stores the molecules which will be
     ///        legalized into clusters.
     const Prepacker& prepacker_;
+
+    /// @brief A two way map between AtomBlockIds and pb types. This is a copy
+    /// of the AtomPBBimap in the global context's AtomLookup
     AtomPBBimap atom_pb_lookup_;
 
 };

vpr/src/pack/pack.cpp

@@ -175,7 +175,7 @@ bool try_pack(t_packer_opts* packer_opts,
                               is_global,
                               flat_placement_info);
 
-    g_vpr_ctx.mutable_atom().mutable_lookup().lock_atom_pb_bimap = true;
+    g_vpr_ctx.mutable_atom().mutable_lookup().set_atom_pb_bimap_lock(true);
 
     while (true) {
         //Cluster the netlist
@@ -288,11 +288,8 @@ bool try_pack(t_packer_opts* packer_opts,
         }
 
         //Reset clustering for re-packing
-        // g_vpr_ctx.mutable_atom().mutable_lookup().lock_atom_pb = false;
-        for (auto blk : g_vpr_ctx.atom().netlist().blocks()) {
-            g_vpr_ctx.mutable_atom().mutable_lookup().set_atom_clb(blk, ClusterBlockId::INVALID());
-            cluster_legalizer.mutable_atom_pb_lookup().set_atom_pb(blk, nullptr);
-        }
+        cluster_legalizer.mutable_atom_pb_lookup().reset_bimap(g_vpr_ctx.atom().netlist());
+        
         for (auto net : g_vpr_ctx.atom().netlist().nets()) {
             g_vpr_ctx.mutable_atom().mutable_lookup().remove_atom_net(net);
         }
@@ -301,7 +298,6 @@ bool try_pack(t_packer_opts* packer_opts,
 
         // Reset the cluster legalizer for re-clustering.
         cluster_legalizer.reset();
-        // g_vpr_ctx.mutable_atom().mutable_lookup().lock_atom_pb = true;
         ++pack_iteration;
     }
 
@@ -319,7 +315,7 @@ bool try_pack(t_packer_opts* packer_opts,
      * }
      */
     /******************** End **************************/
-    g_vpr_ctx.mutable_atom().mutable_lookup().lock_atom_pb_bimap = false;
+    g_vpr_ctx.mutable_atom().mutable_lookup().set_atom_pb_bimap_lock(false);
     g_vpr_ctx.mutable_atom().mutable_lookup().set_atom_to_pb_bimap(cluster_legalizer.atom_pb_lookup());
     //check clustering and output it
     check_and_output_clustering(cluster_legalizer, *packer_opts, is_clock, &arch);