gpl: min congestion and inflation defaults modifications #9192
Conversation
this parameter is a threshold for how much congestion in a tile is required to trigger inflation on its cells Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
reducing this value we extend the reach of cells to be inflated Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes the minimum congestion for inflation configurable, adjusts some default values for routability, and adds debug statistics. The changes are well-motivated and seem to improve placement results.
My review has identified a few areas for improvement:
- There's a documentation mismatch for the new parameter's default value.
- The inflation calculation logic is inconsistent between the RUDY and non-RUDY paths.
- There is duplicated code for calculating statistics, which also contains a minor bug in the median calculation.
Addressing these points will improve the correctness and maintainability of the code. Overall, this is a good set of changes.
| if (log_->debugCheck(GPL, "rudy", 1)) { | ||
| std::vector<float> inflation_ratios; | ||
| inflation_ratios.reserve(tg_->tiles().size()); | ||
| for (auto& tile : tg_->tiles()) { | ||
| inflation_ratios.push_back(tile->inflationRatio()); | ||
| } | ||
|
|
||
| if (!inflation_ratios.empty()) { | ||
| std::sort(inflation_ratios.begin(), inflation_ratios.end()); | ||
| float sum = 0.0f; | ||
| for (float val : inflation_ratios) { | ||
| sum += val; | ||
| } | ||
| float mean = sum / inflation_ratios.size(); | ||
| float median = inflation_ratios[inflation_ratios.size() / 2]; | ||
| float variance = 0.0f; | ||
| for (float val : inflation_ratios) { | ||
| variance += (val - mean) * (val - mean); | ||
| } | ||
| float stddev = std::sqrt(variance / inflation_ratios.size()); | ||
| log_->report( | ||
| "RUDY Inflation ratio statistics - Mean: {:.4f}, Median: {:.4f}, Std " | ||
| "Dev: " | ||
| "{:.4f}", | ||
| mean, | ||
| median, | ||
| stddev); | ||
|
|
||
| // Histogram (10 buckets) | ||
| float min_val = inflation_ratios.front(); | ||
| float max_val = inflation_ratios.back(); | ||
|
|
||
| if (max_val - min_val < 1e-6) { | ||
| log_->report( | ||
| "RUDY Inflation ratio statistics - All tiles have inflation ratio: " | ||
| "{:.4f}", | ||
| min_val); | ||
| } else { | ||
| const int num_buckets = 10; | ||
| float step = (max_val - min_val) / num_buckets; | ||
| std::vector<int> bucket_counts(num_buckets, 0); | ||
|
|
||
| for (float val : inflation_ratios) { | ||
| int bucket = static_cast<int>((val - min_val) / step); | ||
| if (bucket >= num_buckets) { | ||
| bucket = num_buckets - 1; | ||
| } | ||
| bucket_counts[bucket]++; | ||
| } | ||
|
|
||
| log_->report("RUDY Inflation ratio distribution:"); | ||
| for (int i = 0; i < num_buckets; i++) { | ||
| float range_start = min_val + i * step; | ||
| float range_end = min_val + (i + 1) * step; | ||
| float percentage = static_cast<float>(bucket_counts[i]) | ||
| / inflation_ratios.size() * 100.0f; | ||
| log_->report("[{:.2f}, {:.2f}): {} ({:.2f}%)", | ||
| range_start, | ||
| range_end, | ||
| bucket_counts[i], | ||
| percentage); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This block of code for calculating and printing statistics is very similar to the printGCellInflation function. To improve maintainability and avoid code duplication, consider extracting this logic into a reusable helper function that takes a vector of floats and a logger as arguments.
| sum += val; | ||
| } | ||
| float mean = sum / cell_inflation_ratios.size(); | ||
| float median = cell_inflation_ratios[cell_inflation_ratios.size() / 2]; |
There was a problem hiding this comment.
The median calculation cell_inflation_ratios[cell_inflation_ratios.size() / 2] is only correct for vectors with an odd number of elements. For an even number of elements, the median should be the average of the two middle elements. This issue also exists in the duplicated statistics calculation logic in calculateRudyTiles. This should be fixed in the suggested helper function.
Additionally, for calculating sum and variance, you could use std::accumulate from the <numeric> header for more concise code.
float median;
const size_t size = cell_inflation_ratios.size();
if (size % 2 == 0) {
median = (cell_inflation_ratios[size / 2 - 1] + cell_inflation_ratios[size / 2]) / 2.0f;
} else {
median = cell_inflation_ratios[size / 2];
}![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: Augusto Berndt <[email protected]>
incorrect README value for min inflation, and maintain consistency between GRT and RUDY versions Signed-off-by: Augusto Berndt <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 participant
This PR makes the existing variable
minInflationRatioconfigurable through a TCL parameter and renames it tominCongestionForInflation. This variable defines a threshold for applying inflation to cells within a tile (RUDY) based on the tile’s congestion value. The default value was changed from 1.01 to 0.8, effectively extending the range in which inflation is applied. In other words, if a tile has a congestion value of 0.8 or higher, its cells will be inflated, otherwise, no inflation is applied to the cells in that tile.This PR also adjusts the “speed” of inflation by reducing
inflationRatioCoeffrom 2.0 to 0.5.Additionally, it includes debug prints for statistics related to congestion tiles and cell inflation, currently guarded by
debugCheck.Experimental results show less detouring due to reduced congestion on later stages.