Skip to content
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.

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

Remove accesses to global placement state during placement stage #2669

Merged
merged 81 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
81 commits
Select commit Hold shift + click to select a range
ccad435
moved ERROR_TOL to net_cost_handler.h
soheilshahrouz Jul 16, 2024
fb01f19
Merge remote-tracking branch 'origin/master' into temp_place_ref
soheilshahrouz Jul 22, 2024
476d5b6
add t_swap_stats
soheilshahrouz Jul 22, 2024
4862a91
changed the signitaure of create_move_generators()
soheilshahrouz Jul 22, 2024
6d54b14
don't access global place context in get_coordinate_of_pin()
soheilshahrouz Jul 22, 2024
05f4eb9
remove place_ctx.block_locs from noc_place_utils
soheilshahrouz Jul 22, 2024
8cf3b06
remove place_ctx.block_locs from initial placement
soheilshahrouz Jul 22, 2024
8b1b568
remove place_ctx.block_locs from move generators
soheilshahrouz Jul 22, 2024
db52147
remove place_ctx.block_locs from directed_moves_utils
soheilshahrouz Jul 22, 2024
dd99e60
remove a few more place_ctx.block_locs
soheilshahrouz Jul 22, 2024
e4e7b8b
remove place_ctx.block_locs from move_transactions.cpp
soheilshahrouz Jul 22, 2024
3febe66
remove place_ctx.block_locs from noc_routing_has_cycle()
soheilshahrouz Jul 23, 2024
e68dd7a
remove place_ctx.block_locs from placement_checkpoint
soheilshahrouz Jul 23, 2024
0b9851c
remove place_ctx.block_locs from read_place.cpp
soheilshahrouz Jul 23, 2024
a839891
replace some accesses outside the placement stage to place_ctx.block_…
soheilshahrouz Jul 23, 2024
5151eb4
remove some accesses to place_ctx.block_locs
soheilshahrouz Jul 24, 2024
bc22a20
remove place_ctx.block_locs from timing graph resolver
soheilshahrouz Jul 24, 2024
f73092f
remove more accesses to place_ctx.block_locs
soheilshahrouz Jul 24, 2024
d15526d
an attempt to remove calls to physical_tile_type(ClusterBlockId blk)
soheilshahrouz Jul 24, 2024
9192990
remove remaining place_ctx.block_locs from vpr_utils.cpp
soheilshahrouz Jul 26, 2024
cfb147e
remove g_placer_ctx
soheilshahrouz Jul 26, 2024
ea8c972
remove placer_globals files
soheilshahrouz Jul 26, 2024
d3f4ca7
trying to remove remaining place_ctx.block_locs
soheilshahrouz Jul 28, 2024
1a4f52a
add PlaceLocVars
soheilshahrouz Jul 28, 2024
816d44e
remove all remaining place_ctx.block_locs accesses
soheilshahrouz Jul 28, 2024
519546b
rename block_locs() getters in the placement context
soheilshahrouz Jul 28, 2024
a208dc7
remove place_ctx.grid_blocks access in initial_noc_placement
soheilshahrouz Jul 28, 2024
017b08d
remove place_ctx.grid_blocks accesses from move_utils and move_transa…
soheilshahrouz Jul 28, 2024
1f84809
remove place_ctx.grid_block from find_empty_compatible_subtile
soheilshahrouz Jul 28, 2024
ca5f796
remove place_ctx.grid_block from find_free_layer
soheilshahrouz Jul 28, 2024
613b7b6
remove place_ctx.grid_blocks from place_util and a few other files
soheilshahrouz Jul 28, 2024
68f6547
remove place_ctx.grid_blocks
soheilshahrouz Jul 28, 2024
b94e00c
remove place_ctx.physical_pins
soheilshahrouz Jul 28, 2024
869db57
initialize block_locs and grid_blocks in placer_ctx
soheilshahrouz Jul 29, 2024
578b335
copy local placement loc vars to global state at the end of placement…
soheilshahrouz Jul 29, 2024
7018bc3
add place_loc_vars_ref to graphics
soheilshahrouz Jul 29, 2024
72f830a
replace accesses to placement context in graphics with accesses to it…
soheilshahrouz Jul 29, 2024
67fe9c3
update the graphic place loc vars reference after the placement stage
soheilshahrouz Jul 29, 2024
b39cfe8
unlock place_loc_vars before accessing it
soheilshahrouz Jul 29, 2024
0fa2cfd
enum class e_reward_function
soheilshahrouz Jul 29, 2024
34a680a
replace accesses to global block_locs and grid_blocks with a referenc…
soheilshahrouz Jul 29, 2024
15fab24
rename getter methods of PlacerContext
soheilshahrouz Jul 29, 2024
abbd495
add comments and code cleaning
soheilshahrouz Jul 29, 2024
43c3fd8
fix the compilation error when VTR_ASSERT_SAFE_ENABLED is defined
soheilshahrouz Jul 29, 2024
da61fdb
fix unused arg warning
soheilshahrouz Jul 29, 2024
25e803c
add comments
soheilshahrouz Jul 29, 2024
a85ecc4
enum class e_pad_loc_type and more comments
soheilshahrouz Jul 29, 2024
f574059
fix compilation error in place_loc_vars()
soheilshahrouz Jul 29, 2024
6b80de0
add forward decls and include needed header files in read_place.h
soheilshahrouz Jul 30, 2024
9bfb2a0
Merge branch 'master' into temp_place_ref
soheilshahrouz Jul 30, 2024
56f6d8b
enum class e_cost_methods
soheilshahrouz Jul 30, 2024
946a235
fix compilation errors
soheilshahrouz Jul 31, 2024
7bb781b
use VTR_ASSERT_SAFE instead of VTR_ASSERT in placement context getters
soheilshahrouz Jul 31, 2024
a7c203c
rename PlaceLocVars to BlkLocRegistry
soheilshahrouz Aug 3, 2024
ae9dc10
rename getter methods for BlkLocRegistry objects
soheilshahrouz Aug 3, 2024
dc9c2e2
Merge branch 'master' into temp_place_ref
soheilshahrouz Aug 4, 2024
ee1448e
pass grid_blocks to alloc_and_load_legal_placement_locations()
soheilshahrouz Aug 5, 2024
1ed4704
rename place_loc_vars args to blk_loc_registry
soheilshahrouz Aug 5, 2024
6db42e9
added const BlkLocRegistry& member variable to VprTimingGraphResolver
soheilshahrouz Aug 5, 2024
0c576c5
rename remaining place_loc_vars to blk_loc_registry
soheilshahrouz Aug 5, 2024
ec507de
fix test_fasm failure
soheilshahrouz Aug 5, 2024
d2a1632
no need to manually destruct PlacerContext as it is a local object
soheilshahrouz Aug 5, 2024
75d1ef9
applied Alex's comments
soheilshahrouz Aug 5, 2024
0902eaf
move place_sync_external_block_connections() from vpr_utils to place_…
soheilshahrouz Aug 10, 2024
f19f451
added some doxygen comments
soheilshahrouz Aug 10, 2024
0a8474c
rename placer_context.h to placer_state.h
soheilshahrouz Aug 10, 2024
1b40e0c
fix missing header inclusion
soheilshahrouz Aug 10, 2024
df53193
rename PlacerContext to PlacerState
soheilshahrouz Aug 10, 2024
a6ffb7b
rename a few remaining placer_loc_vars to blk_loc_registry
soheilshahrouz Aug 10, 2024
1ecf07c
add blk_loc_registry.cpp/.h
soheilshahrouz Aug 12, 2024
3b67601
add grid_block.h
soheilshahrouz Aug 12, 2024
8084079
remove INVALID_BLOCK_ID
soheilshahrouz Aug 12, 2024
f7b183a
remove EMPTY_BLOCK_ID
soheilshahrouz Aug 12, 2024
ac30b4b
added some comments
soheilshahrouz Aug 12, 2024
fbd992d
make place_sync_external_block_connections() and set_block_location()…
soheilshahrouz Aug 12, 2024
155efc6
add GridBlock::zero_initialize() method
soheilshahrouz Aug 12, 2024
46434be
add increment_usage() and decrement_usage() to GridBlock
soheilshahrouz Aug 12, 2024
fe1ee45
add load_from_block_locs() to GridBlock
soheilshahrouz Aug 12, 2024
ad7d6cb
add a few more doxygen comments. PR comments are all addressed
soheilshahrouz Aug 12, 2024
2e64e8a
fix compilation warning with no graphics
soheilshahrouz Aug 12, 2024
609bb78
Merge branch 'master' into temp_place_ref
vaughnbetz Aug 15, 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
4 changes: 2 additions & 2 deletions libs/EXTERNAL/libtatum/libtatum/tatum/TimingReporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ Time TimingReporter::report_timing_data_arrival_subpath(std::ostream& os,

{
//Input constraint
TATUM_ASSERT(subpath.elements().size() > 0);
TATUM_ASSERT(!subpath.elements().empty());
const TimingPathElem& path_elem = *(subpath.elements().begin());

Time input_constraint;
Expand Down Expand Up @@ -712,7 +712,7 @@ bool TimingReporter::nearly_equal(const Time& lhs, const Time& rhs) const {

size_t TimingReporter::estimate_point_print_width(const TimingPath& path) const {
size_t width = 60; //default
for(auto subpath : {path.clock_launch_path(), path.data_arrival_path(), path.clock_capture_path()}) {
for(const auto& subpath : {path.clock_launch_path(), path.data_arrival_path(), path.clock_capture_path()}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Files located in libs/EXTERNAL are subtrees and modifying them should be avoided unless needed. If modifying them is needed you need to remember to push your changes back to the libtatum repository. See my instructions here: https://docs.verilogtorouting.org/en/latest/README.developers/#pushing-vtr-changes-back-to-upstream-subtree

This prevents our version of libtatum from diverging from the libtatum repo. This is a consequence of using subtrees.

In my opinion, these changes may not be worth the effort. Especially because I do not think making this a const reference actually improves performance (since its iterating over a statically initialized list).

Copy link
Contributor

Choose a reason for hiding this comment

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

Pushing back to tatum is OK; it's leading user is VTR. But I agree this should be pushed back if you change it.

for(auto elem : subpath.elements()) {
//Take the longest typical point name
std::string point = name_resolver_.node_name(elem.node()) + " (" + name_resolver_.node_type_name(elem.node()) + ")";
Expand Down
10 changes: 5 additions & 5 deletions utils/fasm/src/fasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ void FasmWriterVisitor::visit_top_impl(const char* top_level_name) {
}

void FasmWriterVisitor::visit_clb_impl(ClusterBlockId blk_id, const t_pb* clb) {
auto& place_ctx = g_vpr_ctx.placement();
auto& device_ctx = g_vpr_ctx.device();
auto& cluster_ctx = g_vpr_ctx.clustering();
auto& block_locs = g_vpr_ctx.placement().block_locs();

current_blk_id_ = blk_id;

Expand All @@ -48,10 +48,10 @@ void FasmWriterVisitor::visit_clb_impl(ClusterBlockId blk_id, const t_pb* clb) {

root_clb_ = clb->pb_graph_node;

int x = place_ctx.block_locs[blk_id].loc.x;
int y = place_ctx.block_locs[blk_id].loc.y;
int layer_num = place_ctx.block_locs[blk_id].loc.layer;
int sub_tile = place_ctx.block_locs[blk_id].loc.sub_tile;
int x = block_locs[blk_id].loc.x;
int y = block_locs[blk_id].loc.y;
int layer_num = block_locs[blk_id].loc.layer;
int sub_tile = block_locs[blk_id].loc.sub_tile;
physical_tile_ = device_ctx.grid.get_physical_type({x, y, layer_num});
logical_block_ = cluster_ctx.clb_nlist.block_type(blk_id);
const auto& grid_meta = device_ctx.grid.get_metadata({x, y, layer_num});
Expand Down
2 changes: 1 addition & 1 deletion utils/fasm/test/test_fasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ TEST_CASE("fasm_integration_test", "[fasm]") {

// Verify occupied grid LOCs
const auto & place_ctx = g_vpr_ctx.placement();
for (const auto& loc: place_ctx.block_locs) {
for (const auto& loc: place_ctx.block_locs()) {

// Do not consider "IOB" tiles. They do not have fasm features
// defined in the arch.
Expand Down
4 changes: 2 additions & 2 deletions vpr/src/base/ShowSetup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,10 @@ static void ShowPlacerOpts(const t_placer_opts& PlacerOpts,

VTR_LOG("PlacerOpts.pad_loc_type: ");
switch (PlacerOpts.pad_loc_type) {
case FREE:
case e_pad_loc_type::FREE:
VTR_LOG("FREE\n");
break;
case RANDOM:
case e_pad_loc_type::RANDOM:
VTR_LOG("RANDOM\n");
break;
default:
Expand Down
8 changes: 4 additions & 4 deletions vpr/src/base/load_flat_place.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ static void print_flat_cluster(FILE* fp, ClusterBlockId iblk,

static void print_flat_cluster(FILE* fp, ClusterBlockId iblk,
std::vector<AtomBlockId>& atoms) {
const auto& atom_ctx = g_vpr_ctx.atom();
const auto& block_locs = g_vpr_ctx.placement().block_locs();

auto& atom_ctx = g_vpr_ctx.atom();
t_pl_loc loc = g_vpr_ctx.placement().block_locs[iblk].loc;
t_pl_loc loc = block_locs[iblk].loc;
size_t bnum = size_t(iblk);

for (auto atom : atoms) {
Expand All @@ -26,13 +27,12 @@ static void print_flat_cluster(FILE* fp, ClusterBlockId iblk,

/* prints a flat placement file */
void print_flat_placement(const char* flat_place_file) {

FILE* fp;

ClusterAtomsLookup atoms_lookup;
auto& cluster_ctx = g_vpr_ctx.clustering();

if (!g_vpr_ctx.placement().block_locs.empty()) {
if (!g_vpr_ctx.placement().block_locs().empty()) {
soheilshahrouz marked this conversation as resolved.
Show resolved Hide resolved
fp = fopen(flat_place_file, "w");
for (auto iblk : cluster_ctx.clb_nlist.blocks()) {
auto atoms = atoms_lookup.atoms_in_cluster(iblk);
Expand Down
25 changes: 14 additions & 11 deletions vpr/src/base/place_and_route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ int binary_search_place_and_route(const Netlist<>& placement_net_list,
ScreenUpdatePriority::MINOR,
is_flat);

if (success && Fc_clipped == false) {
if (success && !Fc_clipped) {
final = current;
save_routing(best_routing,
route_ctx.clb_opins_used_locally,
Expand All @@ -357,8 +357,9 @@ int binary_search_place_and_route(const Netlist<>& placement_net_list,
if (placer_opts.place_freq == PLACE_ALWAYS) {
auto& cluster_ctx = g_vpr_ctx.clustering();
// Cluster-based net_list is used for placement
print_place(filename_opts.NetFile.c_str(), cluster_ctx.clb_nlist.netlist_id().c_str(),
filename_opts.PlaceFile.c_str());
std::string placement_id = print_place(filename_opts.NetFile.c_str(), cluster_ctx.clb_nlist.netlist_id().c_str(),
filename_opts.PlaceFile.c_str(), g_vpr_ctx.placement().block_locs());
g_vpr_ctx.mutable_placement().placement_id = placement_id;
}
}

Expand Down Expand Up @@ -389,7 +390,7 @@ int binary_search_place_and_route(const Netlist<>& placement_net_list,
&warnings,
is_flat);

init_draw_coords(final);
init_draw_coords(final, g_vpr_ctx.placement().place_loc_vars());

/* Allocate and load additional rr_graph information needed only by the router. */
alloc_and_load_rr_node_route_structs();
Expand Down Expand Up @@ -556,26 +557,28 @@ static float comp_width(t_chan* chan, float x, float separation) {
break;
}

return (val);
return val;
}

/**
* @brief After placement, logical pins for blocks, and nets must be updated to correspond with physical pins of type.
*
* This is required by blocks with capacity > 1 (e.g. typically IOs with multiple instaces in each placement
* gride location). Since they may be swapped around during placement, we need to update which pins the various
* This is required by blocks with capacity > 1 (e.g. typically IOs with multiple instances in each placement
* grid location). Since they may be swapped around during placement, we need to update which pins the various
* nets use.
*
* This updates both the external inter-block net connecitivity (i.e. the clustered netlist), and the intra-block
* This updates both the external inter-block net connectivity (i.e. the clustered netlist), and the intra-block
* connectivity (since the internal pins used also change).
*
* This function should only be called once
*/
void post_place_sync() {
/* Go through each block */
auto& cluster_ctx = g_vpr_ctx.clustering();
const auto& cluster_ctx = g_vpr_ctx.clustering();
auto& place_loc_vars = g_vpr_ctx.mutable_placement().mutable_place_loc_vars();

// Cluster-based netlist is used for placement
for (auto block_id : cluster_ctx.clb_nlist.blocks()) {
place_sync_external_block_connections(block_id);
for (const ClusterBlockId block_id : cluster_ctx.clb_nlist.blocks()) {
place_sync_external_block_connections(block_id, place_loc_vars);
}
}
8 changes: 4 additions & 4 deletions vpr/src/base/read_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,9 @@ struct ParseFixPins {
ConvertedValue<e_pad_loc_type> from_str(const std::string& str) {
ConvertedValue<e_pad_loc_type> conv_value;
if (str == "free")
conv_value.set_value(FREE);
conv_value.set_value(e_pad_loc_type::FREE);
else if (str == "random")
conv_value.set_value(RANDOM);
conv_value.set_value(e_pad_loc_type::RANDOM);
else {
std::stringstream msg;
msg << "Invalid conversion from '" << str << "' to e_router_algorithm (expected one of: " << argparse::join(default_choices(), ", ") << ")";
Expand All @@ -545,10 +545,10 @@ struct ParseFixPins {

ConvertedValue<std::string> to_str(e_pad_loc_type val) {
ConvertedValue<std::string> conv_value;
if (val == FREE)
if (val == e_pad_loc_type::FREE)
conv_value.set_value("free");
else {
VTR_ASSERT(val == RANDOM);
VTR_ASSERT(val == e_pad_loc_type::RANDOM);
conv_value.set_value("random");
}
return conv_value;
Expand Down
Loading
Loading