[ClusterLegalizer] Code Cleanups

[AlexandreSinger]

After cleaning up the ClusterLegalizer code into an API, there are still some small cleanups which are not required for operation but may improve the readability of the code or improve the performance of clustering (reduce memory usage, reduce runtime, improve QoR).

• The ClusterLegalizer now maintains a count of the number of clusters that currently exist. There was a variable that was left behind in the clustering code named total_clb_num which can probably be removed:
  

  vtr-verilog-to-routing/vpr/src/pack/cluster.cpp

  Lines 387 to 390 in e38c0f3

  	// Ensure that we have kept track of the number of clusters correctly.
  	// TODO: The total_clb_num variable could probably just be replaced by
  	// clusters().size().
  	VTR_ASSERT(cluster_legalizer.clusters().size() == (size_t)total_clb_num);

• The constructor of the ClusterLegalizer is very long due to a lot of integer variables being passed in which are only used to allocate C-style dynamic arrays (the original author allocated arrays to a max size). These dynamic arrays should instead be implemented as std::vectors which would be more space efficient and maybe even more performant. Specifically feasible_block_array_size which is a historical barnacle which may no longer be needed.
  

  vtr-verilog-to-routing/vpr/src/pack/cluster_legalizer.h

  Lines 212 to 239 in e38c0f3

  	* @param num_models The total number of models in the architecture.
  	* This is the sum of the number of the user and
  	* library models. Used internally to allocate data
  	* structures.
  	* @param target_external_pin_util_str A string used to initialize the
  	* target external pin utilization of
  	* each cluster type.
  	* @param high_fanout_thresholds An object that stores the thresholds for
  	* a net to be considered high fanout for
  	* different block types.
  	* @param cluster_legalization_strategy The legalization strategy to be
  	* used when creating clusters and
  	* adding molecules to clusters.
  	* Controls the checks that are performed.
  	* @param enable_pin_feasibility_filter A flag to turn on/off the check for
  	* pin usage feasibility.
  	* @param feasible_block_array_size The largest number of feasible blocks
  	* that can be stored in a cluster. Used
  	* to allocate an internal structure.
  	* @param log_verbosity Controls how verbose the log messages will
  	* be within this class.
  	*
  	* TODO: A lot of these arguments are only used to allocate C-style arrays
  	* since the original author was avoiding dynamic allocations. It may
  	* be more space efficient (and cleaner) to make these dynamic arrays
  	* and not pass these arguments in.
  	*/
  	ClusterLegalizer(const AtomNetlist& atom_netlist,

• Pack molecules have a flag in their data structure called valid, which signifies that a molecule has been unclustered. The naming of this variable is very confusing and this flag is really not needed. The ClusterLegalizer has a method called is_atom_clustered which can just check if any of the atoms in the molecule have been clustered (if a molecule has been clustered, all atoms in that molecule should be clustered). This will make the code more readable and would prevent the ClusterLegalizer from modifying the pack molecules themselves (which really should not change during clustering).
  

  vtr-verilog-to-routing/vpr/src/pack/cluster_legalizer.cpp

  Lines 1369 to 1375 in e38c0f3

  	/* invalidate all molecules that share atom block with current molecule */
  	t_pack_molecule* cur_molecule = prepacker_.get_atom_molecule(atom_blk_id);
  	// TODO: This should really be named better. Something like
  	// "is_clustered". and then it should be set to true.
  	// Right now, valid implies "not clustered" which is
  	// confusing.
  	cur_molecule->valid = false;

• The try_pack_molecule method within the ClusterLegalizer is very long and complicated. This method should be split into multiple helper methods. This may also be a good opportunity to organize this method in a more clean way that works well with the ClusterLegalizationStrategy.
  

  vtr-verilog-to-routing/vpr/src/pack/cluster_legalizer.cpp

  Lines 1137 to 1140 in e38c0f3

  	e_block_pack_status ClusterLegalizer::try_pack_molecule(t_pack_molecule* molecule,
  	LegalizationCluster& cluster,
  	LegalizationClusterId cluster_id,
  	const t_ext_pin_util& max_external_pin_util) {

• There are global memory accesses in the ClusterLegalizer which are used for storage which should be cleaned up and stored within the class itself. Most notably is the atom pb lookup in the atom_ctx which is used to store the primitive pb of the given atom. This creates a major limitation of the ClusterLegalizer where it cannot exist at the same time as the ClusteredNetlist and 2 ClusterLegalizers cannot exist at the same time. Instead, this lookup should be stored in the ClusteredNetlist class itself. Removing this global access would make the class significantly more usable.
  

  vtr-verilog-to-routing/vpr/src/pack/cluster_legalizer.cpp

  Lines 577 to 582 in e38c0f3

  	// NOTE: This pb is different from the pb of the cluster. It is the pb
  	// of the actual primitive.
  	// TODO: It would be a good idea to remove the use of this global
  	// variables to prevent external users from modifying this by
  	// mistake.
  	mutable_atom_ctx.lookup.set_atom_pb(blk_id, pb);

• There is currently state leaking out from the ClusterLegalizer from the cluster's pb. Each cluster maintains its own pb during clustering which it frees after the cluster is destroyed. These pbs are being leaked out from the cluster legalizer and used outside of it. This can be unsafe since the user may make changes to these pbs which the ClusterLegalizer is not aware of. These should be changed to some form of accessor methods which manipulate / read the pbs.
  

  vtr-verilog-to-routing/vpr/src/pack/cluster_legalizer.h

  Lines 380 to 385 in e38c0f3

  	/// @brief Gets the top-level pb of the given cluster.
  	inline t_pb* get_cluster_pb(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.pb;
  	}