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

Congestion placement #2384

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions vpr/src/base/SetupVPR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ static void SetupPlacerOpts(const t_options& Options, t_placer_opts* PlacerOpts)
PlacerOpts->recompute_crit_iter = Options.RecomputeCritIter;

PlacerOpts->timing_tradeoff = Options.PlaceTimingTradeoff;
PlacerOpts->congestion_tradeoff = Options.CongestionTradeoff;

/* Depends on PlacerOpts->place_algorithm */
PlacerOpts->delay_offset = Options.place_delay_offset;
Expand Down
26 changes: 21 additions & 5 deletions vpr/src/base/read_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ struct ParsePlaceAlgorithm {
conv_value.set_value(CRITICALITY_TIMING_PLACE);
} else if (str == "slack_timing") {
conv_value.set_value(SLACK_TIMING_PLACE);
} else if (str == "congestion_aware"){
conv_value.set_value(CONGESTION_AWARE_PLACE);
} else {
std::stringstream msg;
msg << "Invalid conversion from '" << str << "' to e_place_algorithm (expected one of: " << argparse::join(default_choices(), ", ") << ")";
Expand All @@ -418,6 +420,8 @@ struct ParsePlaceAlgorithm {
conv_value.set_value("bounding_box");
} else if (val == CRITICALITY_TIMING_PLACE) {
conv_value.set_value("criticality_timing");
} else if (val == CONGESTION_AWARE_PLACE) {
conv_value.set_value("congestion_aware");
} else {
VTR_ASSERT(val == SLACK_TIMING_PLACE);
conv_value.set_value("slack_timing");
Expand All @@ -426,7 +430,7 @@ struct ParsePlaceAlgorithm {
}

std::vector<std::string> default_choices() {
return {"bounding_box", "criticality_timing", "slack_timing"};
return {"bounding_box", "criticality_timing", "slack_timing", "congestion_aware"};
}
};

Expand Down Expand Up @@ -1344,6 +1348,8 @@ argparse::ArgumentParser create_arg_parser(std::string prog_name, t_options& arg
" Sets the routing congestion drawing state\n"
" * exit <int>\n"
" Exits VPR with specified exit code\n"
" * set_congestion <int>\n"
" Sets the routing congestion drawing state\n"
"\n"
" Example:\n"
" 'save_graphics place.png; \\\n"
Expand Down Expand Up @@ -1928,9 +1934,10 @@ argparse::ArgumentParser create_arg_parser(std::string prog_name, t_options& arg
"Controls which placement algorithm is used. Valid options:\n"
" * bounding_box: Focuses purely on minimizing the bounding box wirelength of the circuit. Turns off timing analysis if specified.\n"
" * criticality_timing: Focuses on minimizing both the wirelength and the connection timing costs (criticality * delay).\n"
" * slack_timing: Focuses on improving the circuit slack values to reduce critical path delay.\n")
" * slack_timing: Focuses on improving the circuit slack values to reduce critical path delay.\n"
" * congestion_aware: Focuses on improving routability.\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to explain how being congestion aware would result in better routability.

.default_value("criticality_timing")
.choices({"bounding_box", "criticality_timing", "slack_timing"})
.choices({"bounding_box", "criticality_timing", "slack_timing", "congestion_aware"})
.show_in(argparse::ShowIn::HELP_ONLY);

place_grp.add_argument<e_place_algorithm, ParsePlaceAlgorithm>(args.PlaceQuenchAlgorithm, "--place_quench_algorithm")
Expand All @@ -1940,9 +1947,10 @@ argparse::ArgumentParser create_arg_parser(std::string prog_name, t_options& arg
"Valid options:\n"
" * bounding_box: Focuses purely on minimizing the bounding box wirelength of the circuit. Turns off timing analysis if specified.\n"
" * criticality_timing: Focuses on minimizing both the wirelength and the connection timing costs (criticality * delay).\n"
" * slack_timing: Focuses on improving the circuit slack values to reduce critical path delay.\n")
" * slack_timing: Focuses on improving the circuit slack values to reduce critical path delay.\n"
" * congestion_aware: Focuses on improving routability.\n")
.default_value("criticality_timing")
.choices({"bounding_box", "criticality_timing", "slack_timing"})
.choices({"bounding_box", "criticality_timing", "slack_timing", "congestion_aware"})
.show_in(argparse::ShowIn::HELP_ONLY);

place_grp.add_argument(args.PlaceChanWidth, "--place_chan_width")
Expand Down Expand Up @@ -2130,6 +2138,14 @@ argparse::ArgumentParser create_arg_parser(std::string prog_name, t_options& arg
" 0.0 focuses completely on wirelength, 1.0 completely on timing")
.default_value("0.5")
.show_in(argparse::ShowIn::HELP_ONLY);

place_timing_grp.add_argument(args.CongestionTradeoff, "--congest_tradeoff")
.help(
"Trade-off control the bouding value for the contestion matrix.\n"
" a value near routing channel width can be a good value.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

If congest_tradeoff was supposed to be something similar to timing_tradeoff, it should take a value betwen 0 and 1. If not, congest_tradeoff is not a good name for the new option. We already have a place_chan_width command line option that takes the routing channel width for placement purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Misspelled word : bouding -> bounding
While comment says that a value near routing channel width can be good, the default value set to 1. Is there a specific reason for the default value? If not, I think Soheil is right, just use place_chan_width.

" a high value let the VPR to ignore the congestion aware placement and continue its own course of action.\n")
.default_value("1.0")
.show_in(argparse::ShowIn::HELP_ONLY);

place_timing_grp.add_argument(args.RecomputeCritIter, "--recompute_crit_iter")
.help("Controls how many temperature updates occur between timing analysis during placement")
Expand Down
1 change: 1 addition & 0 deletions vpr/src/base/read_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ struct t_options {

/* Timing-driven placement options only */
argparse::ArgValue<float> PlaceTimingTradeoff;
argparse::ArgValue<float> CongestionTradeoff;
argparse::ArgValue<int> RecomputeCritIter;
argparse::ArgValue<int> inner_loop_recompute_divider;
argparse::ArgValue<int> quench_recompute_divider;
Expand Down
6 changes: 4 additions & 2 deletions vpr/src/base/vpr_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,8 @@ struct t_annealing_sched {
enum e_place_algorithm {
BOUNDING_BOX_PLACE,
CRITICALITY_TIMING_PLACE,
SLACK_TIMING_PLACE
SLACK_TIMING_PLACE,
CONGESTION_AWARE_PLACE
};

/**
Expand Down Expand Up @@ -1015,7 +1016,7 @@ class t_place_algorithm {

///@brief Check if the algorithm belongs to the timing driven category.
inline bool is_timing_driven() const {
return algo == CRITICALITY_TIMING_PLACE || algo == SLACK_TIMING_PLACE;
return algo == CRITICALITY_TIMING_PLACE || algo == SLACK_TIMING_PLACE || algo== CONGESTION_AWARE_PLACE;
}

///@brief Accessor: returns the underlying e_place_algorithm enum value.
Expand Down Expand Up @@ -1147,6 +1148,7 @@ struct t_placer_opts {
t_place_algorithm place_algorithm;
t_place_algorithm place_quench_algorithm;
float timing_tradeoff;
float congestion_tradeoff;
float place_cost_exp;
int place_chan_width;
enum e_pad_loc_type pad_loc_type;
Expand Down
2 changes: 2 additions & 0 deletions vpr/src/place/move_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ struct MoveOutcomeStats {
float delta_cost_norm = std::numeric_limits<float>::quiet_NaN();
float delta_bb_cost_norm = std::numeric_limits<float>::quiet_NaN();
float delta_timing_cost_norm = std::numeric_limits<float>::quiet_NaN();
float delta_cong_cost_norm = std::numeric_limits<float>::quiet_NaN();

float delta_bb_cost_abs = std::numeric_limits<float>::quiet_NaN();
float delta_timing_cost_abs = std::numeric_limits<float>::quiet_NaN();
float delta_cong_cost_abs = std::numeric_limits<float>::quiet_NaN();

e_move_result outcome = ABORTED;
float elapsed_time = std::numeric_limits<float>::quiet_NaN();
Expand Down
Loading