Skip to content

Re-clustering API bug fixes #2397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 23, 2024
Merged

Re-clustering API bug fixes #2397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f83cd43
Re-clustering API bug fixes
Sep 15, 2023
05a4f85
remove unused variable
Sep 18, 2023
27a6c15
fix formatting
Sep 18, 2023
ad01e48
remove unused parameters and fix formatting
Sep 18, 2023
b95d1e3
dummy commit
May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions vpr/src/pack/re_cluster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ bool move_mol_to_new_cluster(t_pack_molecule* molecule,
//Commit or revert the move
if (is_created) {
commit_mol_move(old_clb, new_clb, during_packing, true);
// Update the clb-->atoms lookup table
helper_ctx.atoms_lookup.resize(helper_ctx.total_clb_num);
for (int i_atom = 0; i_atom < molecule_size; ++i_atom) {
if (molecule->atom_block_ids[i_atom]) {
helper_ctx.atoms_lookup[new_clb].insert(molecule->atom_block_ids[i_atom]);
}
}

VTR_LOGV(verbosity > 4, "Atom:%zu is moved to a new cluster\n", molecule->atom_block_ids[molecule->root]);
} else {
revert_mol_move(old_clb, molecule, old_router_data, during_packing, clustering_data);
Expand Down Expand Up @@ -128,7 +136,7 @@ bool move_mol_to_existing_cluster(t_pack_molecule* molecule,

//Add the atom to the new cluster
t_lb_router_data* new_router_data = nullptr;
is_added = pack_mol_in_existing_cluster(molecule, molecule_size, new_clb, new_clb_atoms, during_packing, false, clustering_data, new_router_data);
is_added = pack_mol_in_existing_cluster(molecule, molecule_size, new_clb, new_clb_atoms, during_packing, clustering_data, new_router_data);

//Commit or revert the move
if (is_added) {
Expand Down Expand Up @@ -157,6 +165,8 @@ bool swap_two_molecules(t_pack_molecule* molecule_1,
bool during_packing,
int verbosity,
t_clustering_data& clustering_data) {
auto& cluster_ctx = g_vpr_ctx.mutable_clustering();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this on Amin's MR, but why is this declared so far away from where it is used? Is this a style thing?


//define local variables
PartitionRegion temp_cluster_pr_1, temp_cluster_pr_2;

Expand Down Expand Up @@ -193,6 +203,11 @@ bool swap_two_molecules(t_pack_molecule* molecule_1,
return false;
}

t_pb* clb_pb_1 = cluster_ctx.clb_nlist.block_pb(clb_1);
std::string clb_pb_1_name = (std::string)clb_pb_1->name;
t_pb* clb_pb_2 = cluster_ctx.clb_nlist.block_pb(clb_2);
std::string clb_pb_2_name = (std::string)clb_pb_2->name;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also put this on Amins PR lol, but I think these should use the std::string's constructor instead of using the c-style casting


//remove the molecule from its current cluster
remove_mol_from_cluster(molecule_1, molecule_1_size, clb_1, clb_1_atoms, false, old_1_router_data);
commit_mol_removal(molecule_1, molecule_1_size, clb_1, during_packing, old_1_router_data, clustering_data);
Expand All @@ -201,31 +216,43 @@ bool swap_two_molecules(t_pack_molecule* molecule_1,
commit_mol_removal(molecule_2, molecule_2_size, clb_2, during_packing, old_2_router_data, clustering_data);

//Add the atom to the new cluster
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_2, clb_2_atoms, during_packing, true, clustering_data, old_2_router_data);
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_2, clb_2_atoms, during_packing, clustering_data, old_2_router_data);
if (!mol_1_success) {
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_1, clb_1_atoms, during_packing, true, clustering_data, old_1_router_data);
mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_2, clb_2_atoms, during_packing, true, clustering_data, old_2_router_data);
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_1, clb_1_atoms, during_packing, clustering_data, old_1_router_data);
mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_2, clb_2_atoms, during_packing, clustering_data, old_2_router_data);

VTR_ASSERT(mol_1_success && mol_2_success);
free_router_data(old_1_router_data);
free_router_data(old_2_router_data);
old_1_router_data = nullptr;
old_2_router_data = nullptr;

free(clb_pb_1->name);
cluster_ctx.clb_nlist.block_pb(clb_1)->name = vtr::strdup(clb_pb_1_name.c_str());
free(clb_pb_2->name);
cluster_ctx.clb_nlist.block_pb(clb_2)->name = vtr::strdup(clb_pb_2_name.c_str());

return false;
}

mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_1, clb_1_atoms, during_packing, true, clustering_data, old_1_router_data);
mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_1, clb_1_atoms, during_packing, clustering_data, old_1_router_data);
if (!mol_2_success) {
remove_mol_from_cluster(molecule_1, molecule_1_size, clb_2, clb_2_atoms, true, old_2_router_data);
commit_mol_removal(molecule_1, molecule_1_size, clb_2, during_packing, old_2_router_data, clustering_data);
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_1, clb_1_atoms, during_packing, true, clustering_data, old_1_router_data);
mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_2, clb_2_atoms, during_packing, true, clustering_data, old_2_router_data);
mol_1_success = pack_mol_in_existing_cluster(molecule_1, molecule_1_size, clb_1, clb_1_atoms, during_packing, clustering_data, old_1_router_data);
mol_2_success = pack_mol_in_existing_cluster(molecule_2, molecule_2_size, clb_2, clb_2_atoms, during_packing, clustering_data, old_2_router_data);

VTR_ASSERT(mol_1_success && mol_2_success);
free_router_data(old_1_router_data);
free_router_data(old_2_router_data);
old_1_router_data = nullptr;
old_2_router_data = nullptr;

free(clb_pb_1->name);
cluster_ctx.clb_nlist.block_pb(clb_1)->name = vtr::strdup(clb_pb_1_name.c_str());
free(clb_pb_2->name);
cluster_ctx.clb_nlist.block_pb(clb_2)->name = vtr::strdup(clb_pb_2_name.c_str());

return false;
}

Expand All @@ -242,6 +269,12 @@ bool swap_two_molecules(t_pack_molecule* molecule_1,
free_router_data(old_2_router_data);
old_1_router_data = nullptr;
old_2_router_data = nullptr;

free(clb_pb_1->name);
cluster_ctx.clb_nlist.block_pb(clb_1)->name = vtr::strdup(clb_pb_1_name.c_str());
free(clb_pb_2->name);
cluster_ctx.clb_nlist.block_pb(clb_2)->name = vtr::strdup(clb_pb_2_name.c_str());

return true;
}
#endif
52 changes: 28 additions & 24 deletions vpr/src/pack/re_cluster_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
#include "read_netlist.h"
#include <cstring>

//The name suffix of the new block (if exists)
// The name suffix of the new block (if exists)
// This suffex is useful in preventing duplicate high-level cluster block names
const char* name_suffix = "_m";

/******************* Static Functions ********************/
//static void set_atom_pin_mapping(const ClusteredNetlist& clb_nlist, const AtomBlockId atom_blk, const AtomPortId atom_port, const t_pb_graph_pin* gpin);
static void load_atom_index_for_pb_pin(t_pb_routes& pb_route, int ipin);
static void load_internal_to_block_net_nums(const t_logical_block_type_ptr type, t_pb_routes& pb_route);
//static bool count_children_pbs(const t_pb* pb);
static void fix_atom_pin_mapping(const AtomBlockId blk);

static void fix_cluster_pins_after_moving(const ClusterBlockId clb_index);
Expand All @@ -42,6 +41,7 @@ static void update_cluster_pb_stats(const t_pack_molecule* molecule,
int molecule_size,
ClusterBlockId clb_index,
bool is_added);

/***************** API functions ***********************/
ClusterBlockId atom_to_cluster(const AtomBlockId& atom) {
auto& atom_ctx = g_vpr_ctx.atom();
Expand All @@ -66,19 +66,18 @@ void remove_mol_from_cluster(const t_pack_molecule* molecule,
t_lb_router_data*& router_data) {
auto& helper_ctx = g_vpr_ctx.mutable_cl_helper();

//re-build router_data structure for this cluster
if (!router_data_ready)
router_data = lb_load_router_data(helper_ctx.lb_type_rr_graphs, old_clb, old_clb_atoms);

//remove atom from router_data
for (int i_atom = 0; i_atom < molecule_size; i_atom++) {
if (molecule->atom_block_ids[i_atom]) {
remove_atom_from_target(router_data, molecule->atom_block_ids[i_atom]);
auto it = old_clb_atoms->find(molecule->atom_block_ids[i_atom]);
if (it != old_clb_atoms->end())
old_clb_atoms->erase(molecule->atom_block_ids[i_atom]);
}
}

//re-build router_data structure for this cluster
if (!router_data_ready)
router_data = lb_load_router_data(helper_ctx.lb_type_rr_graphs, old_clb, old_clb_atoms);

update_cluster_pb_stats(molecule, molecule_size, old_clb, false);
}

Expand All @@ -88,7 +87,7 @@ void commit_mol_move(const ClusterBlockId& old_clb,
bool new_clb_created) {
auto& device_ctx = g_vpr_ctx.device();

//Place the new cluster if this function called during placement (after the initial placement is done)
//place the new cluster if this function called during placement (after the initial placement is done)
if (!during_packing && new_clb_created) {
int imacro;
g_vpr_ctx.mutable_placement().block_locs.resize(g_vpr_ctx.placement().block_locs.size() + 1);
Expand All @@ -101,6 +100,7 @@ void commit_mol_move(const ClusterBlockId& old_clb,
t_lb_router_data* lb_load_router_data(std::vector<t_lb_type_rr_node>* lb_type_rr_graphs, const ClusterBlockId& clb_index, const std::unordered_set<AtomBlockId>* clb_atoms) {
//build data structures used by intra-logic block router
auto& cluster_ctx = g_vpr_ctx.clustering();
auto& atom_ctx = g_vpr_ctx.atom();
auto block_type = cluster_ctx.clb_nlist.block_type(clb_index);
t_lb_router_data* router_data = alloc_and_load_router_data(&lb_type_rr_graphs[block_type->index], block_type);

Expand All @@ -110,14 +110,19 @@ t_lb_router_data* lb_load_router_data(std::vector<t_lb_type_rr_node>* lb_type_rr

for (auto atom_id : *clb_atoms) {
add_atom_as_target(router_data, atom_id);
const t_pb* pb = atom_ctx.lookup.atom_pb(atom_id);
while (pb) {
set_reset_pb_modes(router_data, pb, true);
pb = pb->parent_pb;
}
}
return (router_data);
}

bool start_new_cluster_for_mol(t_pack_molecule* molecule,
const t_logical_block_type_ptr& type,
const int mode,
const int feasible_block_array_size,
const int& mode,
const int& feasible_block_array_size,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were these made pass by reference? Did you see any performance increase after changing this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing an int by constant reference isn't going to be faster and might be slower as a pointer is 64-bits and an int is 32, and pointer chasing is slow. After compiler optimization the whole pointer/reference stuff might go away, but generally it is better to pass things smaller than or equal to pointer size by value; use const & for bigger things that aren't changed in a function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I suggest putting these back to const int, although it isn't a huge deal.

bool enable_pin_feasibility_filter,
ClusterBlockId clb_index,
bool during_packing,
Expand Down Expand Up @@ -148,10 +153,11 @@ bool start_new_cluster_for_mol(t_pack_molecule* molecule,

e_block_pack_status pack_result = BLK_STATUS_UNDEFINED;
pb->mode = mode;
reset_cluster_placement_stats(&(helper_ctx.cluster_placement_stats[type->index]));
t_cluster_placement_stats* cluster_placement_stats = &(helper_ctx.cluster_placement_stats[type->index]);
reset_cluster_placement_stats(cluster_placement_stats);
set_mode_cluster_placement_stats(pb->pb_graph_node, mode);

pack_result = try_pack_molecule(&(helper_ctx.cluster_placement_stats[type->index]),
pack_result = try_pack_molecule(cluster_placement_stats,
molecule,
helper_ctx.primitives_list,
pb,
Expand All @@ -177,6 +183,8 @@ bool start_new_cluster_for_mol(t_pack_molecule* molecule,
pb->name = vtr::strdup(new_name.c_str());
clb_index = cluster_ctx.clb_nlist.create_block(new_name.c_str(), pb, type);
helper_ctx.total_clb_num++;
int molecule_size = get_array_size_of_molecule(molecule);
update_cluster_pb_stats(molecule, molecule_size, clb_index, true);

//If you are still in packing, update the clustering data. Otherwise, update the clustered netlist.
if (during_packing) {
Expand All @@ -199,10 +207,9 @@ bool start_new_cluster_for_mol(t_pack_molecule* molecule,

bool pack_mol_in_existing_cluster(t_pack_molecule* molecule,
int molecule_size,
const ClusterBlockId new_clb,
const ClusterBlockId& new_clb,
std::unordered_set<AtomBlockId>* new_clb_atoms,
bool during_packing,
bool is_swap,
t_clustering_data& clustering_data,
t_lb_router_data*& router_data) {
auto& helper_ctx = g_vpr_ctx.mutable_cl_helper();
Expand All @@ -220,8 +227,7 @@ bool pack_mol_in_existing_cluster(t_pack_molecule* molecule,
return false;

//re-build router_data structure for this cluster
if (!is_swap)
router_data = lb_load_router_data(helper_ctx.lb_type_rr_graphs, new_clb, new_clb_atoms);
router_data = lb_load_router_data(helper_ctx.lb_type_rr_graphs, new_clb, new_clb_atoms);

pack_result = try_pack_molecule(&(helper_ctx.cluster_placement_stats[block_type->index]),
molecule,
Expand Down Expand Up @@ -259,11 +265,9 @@ bool pack_mol_in_existing_cluster(t_pack_molecule* molecule,
update_cluster_pb_stats(molecule, molecule_size, new_clb, true);
}

if (!is_swap) {
//Free clustering router data
free_router_data(router_data);
router_data = nullptr;
}
//Free clustering router data
free_router_data(router_data);
router_data = nullptr;

return (pack_result == BLK_PASSED);
}
Expand Down Expand Up @@ -730,4 +734,4 @@ static void update_cluster_pb_stats(const t_pack_molecule* molecule,
cur_pb = cur_pb->parent_pb;
}
}
}
}
35 changes: 28 additions & 7 deletions vpr/src/pack/re_cluster_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ void remove_mol_from_cluster(const t_pack_molecule* molecule,
*/
bool start_new_cluster_for_mol(t_pack_molecule* molecule,
const t_logical_block_type_ptr& type,
const int mode,
const int feasible_block_array_size,
const int& mode,
const int& feasible_block_array_size,
bool enable_pin_feasibility_filter,
ClusterBlockId clb_index,
bool during_packing,
Expand All @@ -102,10 +102,9 @@ bool start_new_cluster_for_mol(t_pack_molecule* molecule,
*/
bool pack_mol_in_existing_cluster(t_pack_molecule* molecule,
int molecule_size,
const ClusterBlockId clb_index,
std::unordered_set<AtomBlockId>* clb_atoms,
const ClusterBlockId& new_clb,
std::unordered_set<AtomBlockId>* new_clb_atoms,
bool during_packing,
bool is_swap,
t_clustering_data& clustering_data,
t_lb_router_data*& router_data);

Expand All @@ -125,29 +124,51 @@ void fix_clustered_netlist(t_pack_molecule* molecule,
/**
* @brief A function that commits the molecule move if it is legal
*
* @during_packing: true if this function is called during packing, false if it is called during placement
* @new_clb_created: true if the move is creating a new cluster (e.g. move_mol_to_new_cluster)
* @params during_packing: true if this function is called during packing, false if it is called during placement
* @params new_clb_created: true if the move is creating a new cluster (e.g. move_mol_to_new_cluster)
*/
void commit_mol_move(const ClusterBlockId& old_clb,
const ClusterBlockId& new_clb,
bool during_packing,
bool new_clb_created);

/**
* @brief A function that reverts the molecule move if it is illegal
*
* @params during_packing: true if this function is called during packing, false if it is called during placement
* @params new_clb_created: true if the move is creating a new cluster (e.g. move_mol_to_new_cluster)
* @params
*/
void revert_mol_move(const ClusterBlockId& old_clb,
t_pack_molecule* molecule,
t_lb_router_data*& old_router_data,
bool during_packing,
t_clustering_data& clustering_data);

/**
*
* @brief A function that checks the legality of a cluster by running the intra-cluster routing
*/
bool is_cluster_legal(t_lb_router_data*& router_data);

/**
* @brief A function that commits the molecule removal if it is legal
*
* @params during_packing: true if this function is called during packing, false if it is called during placement
* @params new_clb_created: true if the move is creating a new cluster (e.g. move_mol_to_new_cluster)
*/
void commit_mol_removal(const t_pack_molecule* molecule,
const int& molecule_size,
const ClusterBlockId& old_clb,
bool during_packing,
t_lb_router_data*& router_data,
t_clustering_data& clustering_data);

/**
*
* @brief A function that check that two clusters are of the same type and in the same mode of operation
*
*/
bool check_type_and_mode_compitability(const ClusterBlockId& old_clb,
const ClusterBlockId& new_clb,
int verbosity);
Expand Down
Loading