[Packer] Updated Clustering Algorithm Per Vaughn's Comments

Changed when the clustering progress stats are updated and printed to
make them more accurate.

Changed how the LE block counts are stored to make the code easier to
read.

Fixed some comments.

Author: AlexandreSinger

vpr/src/pack/cluster_legalizer.h

@@ -421,6 +421,13 @@ class ClusterLegalizer {
         return cluster.pr;
     }
 
+    /// @brief Gets the current number of molecules in the cluster.
+    inline size_t get_num_molecules_in_cluster(LegalizationClusterId cluster_id) const {
+        VTR_ASSERT_SAFE(cluster_id.is_valid() && (size_t)cluster_id < legalization_clusters_.size());
+        const LegalizationCluster& cluster = legalization_clusters_[cluster_id];
+        return cluster.molecules.size();
+    }
+
     /// @brief Gets the ID of the cluster that contains the given atom block.
     inline LegalizationClusterId get_atom_cluster(AtomBlockId blk_id) const {
         VTR_ASSERT_SAFE(blk_id.is_valid() && (size_t)blk_id < atom_cluster_.size());

vpr/src/pack/cluster_util.cpp

@@ -172,7 +172,8 @@ void print_pack_status(int tot_num_molecules,
 
     int num_clusters_created = cluster_legalizer.clusters().size();
 
-    if (mols_since_last_print == int_molecule_increment) {
+    if (mols_since_last_print >= int_molecule_increment ||
+        num_molecules_processed == tot_num_molecules) {
         VTR_LOG(
             "%6d/%-6d  %3d%%   "
             "%26d   "
@@ -1493,7 +1494,12 @@ t_pb_type* identify_le_block_type(t_logical_block_type_ptr logic_block_type) {
     return nullptr;
 }
 
-void update_le_count(const t_pb* pb, const t_logical_block_type_ptr logic_block_type, const t_pb_type* le_pb_type, std::array<int, 3>& le_count) {
+void update_le_count(const t_pb* pb,
+                     const t_logical_block_type_ptr logic_block_type,
+                     const t_pb_type* le_pb_type,
+                     int& num_logic_le,
+                     int& num_reg_le,
+                     int& num_logic_and_reg_le) {
     // if this cluster doesn't contain LEs or there
     // are no les in this architecture, ignore it
     if (!logic_block_type || pb->pb_graph_node != logic_block_type->pb_graph_head || !le_pb_type)
@@ -1519,15 +1525,15 @@ void update_le_count(const t_pb* pb, const t_logical_block_type_ptr logic_block_
         auto has_used_adder = pb_used_for_blif_model(&parent_pb->child_pbs[0][ile], adder);
         auto has_used_ff = pb_used_for_blif_model(&parent_pb->child_pbs[0][ile], ff);
 
-        // First type of LEs: used for logic and registers
         if ((has_used_lut || has_used_adder) && has_used_ff) {
-            le_count[0]++;
-            // Second type of LEs: used for logic only
+            // First type of LEs: used for logic and registers
+            num_logic_and_reg_le++;
         } else if (has_used_lut || has_used_adder) {
-            le_count[1]++;
-            // Third type of LEs: used for registers only
+            // Second type of LEs: used for logic only
+            num_logic_le++;
         } else if (has_used_ff) {
-            le_count[2]++;
+            // Third type of LEs: used for registers only
+            num_reg_le++;
         }
     }
 }
@@ -1559,12 +1565,19 @@ bool pb_used_for_blif_model(const t_pb* pb, const std::string& blif_model_name)
     return false;
 }
 
-void print_le_count(const std::array<int, 3>& le_count, const t_pb_type* le_pb_type) {
+void print_le_count(int num_logic_le,
+                    int num_reg_le,
+                    int num_logic_and_reg_le,
+                    const t_pb_type* le_pb_type) {
+    VTR_ASSERT(le_pb_type != nullptr);
+
+    int num_total_le = num_logic_and_reg_le + num_logic_le + num_reg_le;
+
     VTR_LOG("\nLogic Element (%s) detailed count:\n", le_pb_type->name);
-    VTR_LOG("  Total number of Logic Elements used : %d\n", le_count[0] + le_count[1] + le_count[2]);
-    VTR_LOG("  LEs used for logic and registers    : %d\n", le_count[0]);
-    VTR_LOG("  LEs used for logic only             : %d\n", le_count[1]);
-    VTR_LOG("  LEs used for registers only         : %d\n\n", le_count[2]);
+    VTR_LOG("  Total number of Logic Elements used : %d\n", num_total_le);
+    VTR_LOG("  LEs used for logic and registers    : %d\n", num_logic_and_reg_le);
+    VTR_LOG("  LEs used for logic only             : %d\n", num_logic_le);
+    VTR_LOG("  LEs used for registers only         : %d\n\n", num_reg_le);
 }
 
 t_pb* get_top_level_pb(t_pb* pb) {

vpr/src/pack/cluster_util.h

@@ -396,7 +396,12 @@ size_t update_pb_type_count(const t_pb* pb, std::map<t_pb_type*, int>& pb_type_c
  * @brief This function updates the le_count data structure from the given
  *        packed cluster.
  */
-void update_le_count(const t_pb* pb, const t_logical_block_type_ptr logic_block_type, const t_pb_type* le_pb_type, std::array<int, 3>& le_count);
+void update_le_count(const t_pb* pb,
+                     const t_logical_block_type_ptr logic_block_type,
+                     const t_pb_type* le_pb_type,
+                     int& num_logic_le,
+                     int& num_reg_le,
+                     int& num_logic_and_reg_le);
 
 void print_pb_type_count_recurr(t_pb_type* type, size_t max_name_chars, size_t curr_depth, std::map<t_pb_type*, int>& pb_type_count);
 
@@ -430,7 +435,10 @@ bool pb_used_for_blif_model(const t_pb* pb, const std::string& blif_model_name);
 /*
  * @brief Print the LE count data strurture.
  */
-void print_le_count(const std::array<int, 3>& le_count, const t_pb_type* le_pb_type);
+void print_le_count(int num_logic_le,
+                    int num_reg_le,
+                    int num_logic_and_reg_le,
+                    const t_pb_type* le_pb_type);
 
 /*
  * @brief Given a pointer to a pb in a cluster, this routine returns a pointer

vpr/src/pack/greedy_clusterer.cpp

@@ -37,7 +37,6 @@
  */
 
 #include "greedy_clusterer.h"
-#include <array>
 #include <cstdio>
 #include <map>
 #include <string>
@@ -56,6 +55,23 @@
 #include "vtr_math.h"
 #include "vtr_vector.h"
 
+namespace {
+
+/**
+ * @brief Struct to hold statistics on the progress of clustering.
+ */
+struct t_cluster_progress_stats {
+    // The total number of molecules in the design.
+    int num_molecules = 0;
+    // The number of molecules which have been clustered.
+    int num_molecules_processed = 0;
+    // The number of molecules clustered since the last time the status was
+    // logged.
+    int mols_since_last_print = 0;
+};
+
+} // namespace
+
 GreedyClusterer::GreedyClusterer(const t_packer_opts& packer_opts,
                                  const t_analysis_opts& analysis_opts,
                                  const AtomNetlist& atom_netlist,
@@ -93,9 +109,8 @@ GreedyClusterer::do_clustering(ClusterLegalizer& cluster_legalizer,
 
     // The clustering stats holds information used for logging the progress
     // of the clustering to the user.
-    // Reset the clustering stats in case the clusterer is called multiple times.
-    clustering_stats_ = t_cluster_progress_stats();
-    clustering_stats_.num_molecules = prepacker.get_num_molecules();
+    t_cluster_progress_stats clustering_stats;
+    clustering_stats.num_molecules = prepacker.get_num_molecules();
 
     // TODO: Create a ClusteringTimingManager class.
     //       This code relies on the prepacker, once the prepacker is moved to
@@ -124,7 +139,7 @@ GreedyClusterer::do_clustering(ClusterLegalizer& cluster_legalizer,
     alloc_and_init_clustering(max_molecule_stats,
                               prepacker,
                               clustering_data,
-                              clustering_stats_.num_molecules);
+                              clustering_stats.num_molecules);
 
     // Create the greedy seed selector.
     GreedySeedSelector seed_selector(atom_netlist_,
@@ -156,7 +171,8 @@ GreedyClusterer::do_clustering(ClusterLegalizer& cluster_legalizer,
         //    do full legalization for each molecule added to the cluster.
 
         // Try to grow a cluster from the seed molecule without doing intra-lb
-        // route for each molecule.
+        // route for each molecule (i.e. just use faster but not fully
+        // conservative legality checks).
         LegalizationClusterId new_cluster_id = try_grow_cluster(seed_mol,
                                         ClusterLegalizationStrategy::SKIP_INTRA_LB_ROUTE,
                                         cluster_legalizer,
@@ -192,6 +208,21 @@ GreedyClusterer::do_clustering(ClusterLegalizer& cluster_legalizer,
         VTR_ASSERT(new_cluster_id.is_valid());
         VTR_ASSERT(cluster_legalizer.is_mol_clustered(seed_mol));
 
+        // Update the clustering progress stats.
+        size_t num_molecules_in_cluster = cluster_legalizer.get_num_molecules_in_cluster(new_cluster_id);
+        clustering_stats.num_molecules_processed += num_molecules_in_cluster;
+        clustering_stats.mols_since_last_print += num_molecules_in_cluster;
+
+        // Print the current progress of the packing after a cluster has been
+        // successfully created.
+        print_pack_status(clustering_stats.num_molecules,
+                          clustering_stats.num_molecules_processed,
+                          clustering_stats.mols_since_last_print,
+                          mutable_device_ctx.grid.width(),
+                          mutable_device_ctx.grid.height(),
+                          attraction_groups,
+                          cluster_legalizer);
+
         // Pick new seed.
         seed_mol = seed_selector.get_next_seed(prepacker,
                                                cluster_legalizer);
@@ -236,15 +267,6 @@ LegalizationClusterId GreedyClusterer::try_grow_cluster(
                                                                       num_used_type_instances,
                                                                       mutable_device_ctx);
 
-    //initial molecule in cluster has been processed
-    print_pack_status(clustering_stats_.num_molecules,
-                      clustering_stats_.num_molecules_processed,
-                      clustering_stats_.mols_since_last_print,
-                      mutable_device_ctx.grid.width(),
-                      mutable_device_ctx.grid.height(),
-                      attraction_groups,
-                      cluster_legalizer);
-
     int high_fanout_threshold = high_fanout_thresholds_.get_threshold(cluster_legalizer.get_cluster_type(legalization_cluster_id)->name);
     update_cluster_stats(seed_mol,
                          cluster_legalizer,
@@ -302,14 +324,6 @@ LegalizationClusterId GreedyClusterer::try_grow_cluster(
         // If the candidate molecule was clustered successfully, update
         // the cluster stats.
         if (success) {
-            print_pack_status(clustering_stats_.num_molecules,
-                              clustering_stats_.num_molecules_processed,
-                              clustering_stats_.mols_since_last_print,
-                              mutable_device_ctx.grid.width(),
-                              mutable_device_ctx.grid.height(),
-                              attraction_groups,
-                              cluster_legalizer);
-
             update_cluster_stats(candidate_mol,
                                  cluster_legalizer,
                                  is_clock_,  //Set of all clocks
@@ -492,10 +506,6 @@ LegalizationClusterId GreedyClusterer::start_new_cluster(
     //Progress dot for seed-block
     fflush(stdout);
 
-    // Update the clustering progress stats.
-    clustering_stats_.num_molecules_processed++;
-    clustering_stats_.mols_since_last_print++;
-
     // TODO: Below may make more sense in its own method.
 
     // Successfully created cluster
@@ -563,13 +573,6 @@ bool GreedyClusterer::try_add_candidate_mol_to_cluster(t_pack_molecule* candidat
         fflush(stdout);
     }
 
-    // If candidate molecule was successfully added, update the clustering
-    // progress stats.
-    if (pack_status == e_block_pack_status::BLK_PASSED) {
-        clustering_stats_.num_molecules_processed++;
-        clustering_stats_.mols_since_last_print++;
-    }
-
     return pack_status == e_block_pack_status::BLK_PASSED;
 }
 
@@ -584,27 +587,32 @@ void GreedyClusterer::report_le_physical_block_usage(const ClusterLegalizer& clu
     if (le_pb_type == nullptr)
         return;
 
-    // this data structure tracks the number of Logic Elements (LEs) used. It is
-    // populated only for architectures which has LEs. The architecture is assumed
-    // to have LEs only iff it has a logic block that contains LUT primitives and is
-    // the first pb_block to have more than one instance from the top of the hierarchy
-    // (All parent pb_block have one instance only and one mode only). Index 0 holds
-    // the number of LEs that are used for both logic (LUTs/adders) and registers.
-    // Index 1 holds the number of LEs that are used for logic (LUTs/adders) only.
-    // Index 2 holds the number of LEs that are used for registers only.
-    std::array<int, 3> le_count = {0, 0, 0};
+    // Track the number of Logic Elements (LEs) used. This is populated only for
+    // architectures which has LEs. The architecture is assumed to have LEs iff
+    // it has a logic block that contains LUT primitives and is the first
+    // pb_block to have more than one instance from the top of the hierarchy
+    // (All parent pb_block have one instance only and one mode only).
+
+    // The number of LEs that are used for logic (LUTs/adders) only.
+    int num_logic_le = 0;
+    // The number of LEs that are used for registers only.
+    int num_reg_le = 0;
+    // The number of LEs that are used for both logic (LUTs/adders) and registers.
+    int num_logic_and_reg_le = 0;
 
     for (LegalizationClusterId cluster_id : cluster_legalizer.clusters()) {
         // Update the data structure holding the LE counts
         update_le_count(cluster_legalizer.get_cluster_pb(cluster_id),
                         logic_block_type,
                         le_pb_type,
-                        le_count);
+                        num_logic_le,
+                        num_reg_le,
+                        num_logic_and_reg_le);
     }
 
     // if this architecture has LE physical block, report its usage
     if (le_pb_type) {
-        print_le_count(le_count, le_pb_type);
+        print_le_count(num_logic_le, num_reg_le, num_logic_and_reg_le, le_pb_type);
     }
 }
 

vpr/src/pack/greedy_clusterer.h

@@ -28,23 +28,6 @@ struct t_analysis_opts;
 struct t_clustering_data;
 struct t_packer_opts;
 
-/**
- * @brief Struct to hold statistics on the progress of clustering.
- *
- * FIXME: These numbers only ever go up! This is a problem since some clusters
- *        may be reclustered, leading to double counting. This is only a logging
- *        bug, but should be thought about.
- */
-struct t_cluster_progress_stats {
-    // The total number of molecules in the design.
-    int num_molecules = 0;
-    // The number of molecules which have been clustered.
-    int num_molecules_processed = 0;
-    // The number of molecules clustered since the last time the status was
-    // logged.
-    int mols_since_last_print = 0;
-};
-
 /**
  * @brief A clusterer that generates clusters by greedily choosing the clusters
  *        which appear to have the best gain for a given neighbor.
@@ -151,7 +134,9 @@ class GreedyClusterer {
 private:
     /**
      * @brief Given a seed molecule and a legalization strategy, tries to grow
-     *        a cluster greedily. Will return the ID of the cluster created.
+     *        a cluster greedily, starting with the provided seed and adding
+     *        whatever other molecules seem beneficial and legal. Will return
+     *        the ID of the cluster created.
      *
      * If the strategy is set to SKIP_INTRA_LB_ROUTE, the cluster will grow
      * without performing intra-lb route every time a molecule is added to the
@@ -179,16 +164,16 @@ class GreedyClusterer {
     /**
      * @brief Given a seed molecule, starts a new cluster by trying to find a
      *        good logical block type and mode to put it in. This method cannot
-     *        fail (only crash if the seed cannot be clustered), so should
+     *        fail (only crash if the seed cannot be clustered), so it should
      *        always return a valid ID to the cluster created.
      *
      * When balance_block_type_utilization is set to true, this method will try
      * to select less used logical block types if it has the option to in order
      * to balance logical block type utilization.
      *
-     * This method will try to grow the device grid if it find thats more
-     * clusters of specific logical block types have been created than the
-     * device can support.
+     * If the device is to be auto-sized, this method will try to grow the
+     * device grid if it find thats more clusters of specific logical block
+     * types have been created than the device can support.
      */
     LegalizationClusterId start_new_cluster(t_pack_molecule* seed_mol,
                                             ClusterLegalizer& cluster_legalizer,
@@ -254,17 +239,14 @@ class GreedyClusterer {
     /// each molecule.
     const int log_verbosity_;
 
-    /* Does the atom block that drives the output of this atom net also appear as a   *
-     * receiver (input) pin of the atom net?
-     *
-     * This is used in the gain routines to avoid double counting the connections from   *
-     * the current cluster to other blocks (hence yielding better clusterings). *
-     * The only time an atom block should connect to the same atom net *
-     * twice is when one connection is an output and the other is an input, *
-     * so this should take care of all multiple connections.                */
+    /// @brief Does the atom block that drives the output of this atom net also
+    /// appear as a receiver (input) pin of the atom net?
+    ///
+    /// This is used in the gain routines to avoid double counting the
+    /// connections from the current cluster to other blocks (hence yielding
+    /// better clusterings). The only time an atom block should connect to the
+    /// same atom net twice is when one connection is an output and the other
+    /// is an input, so this should take care of all multiple connections.
     const std::unordered_set<AtomNetId> net_output_feeds_driving_block_input_;
-
-    /// @brief The current progress of the clustering. Used for logging.
-    t_cluster_progress_stats clustering_stats_;
 };