gpl: Expose PDK-aware GPL parameters through variables.yaml - #4418
gpl: Expose PDK-aware GPL parameters through variables.yaml#4418oharboe wants to merge 1 commit into
Conversation
07ab255 to
921baa7
Compare
![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 introduces PDK-aware GPL parameters (GPL_WIRELENGTH_PENALTY and GPL_TIMING_SPAN_CLOCK_PERCENT) to the global placement flow, along with a hyperparameter autotuning script. Key feedback includes addressing a critical division-by-zero risk in the OpenROAD patch when effective_slack_max <= slack_min, resolving the hardcoded clock_period value, and correcting mismatched default values between global_place.tcl and variables.yaml. Additionally, the autotuner script needs to parse actual metrics instead of returning hardcoded zeros and should avoid buffering build outputs to prevent the script from appearing frozen.
| + + (net_weight_max_ - 1) * (effective_slack_max - net_slack) | ||
| + / (effective_slack_max - slack_min); | ||
| + gNet->setTimingWeight(std::max(1.0f, std::min(net_weight_max_, weight))); |
There was a problem hiding this comment.
If timing_weight_span_clock_percent_ is 0.0f (which is the default passed in global_place.tcl), effective_slack_max becomes exactly equal to slack_min. This causes a division by zero when calculating weight, leading to NaN propagation or crashes. Please add a guard to prevent division by zero when effective_slack_max <= slack_min.
+ if (effective_slack_max <= slack_min) {
+ gNet->setTimingWeight(net_weight_max_);
+ } else {
+ const float weight = 1
+ + (net_weight_max_ - 1) * (effective_slack_max - net_slack)
+ / (effective_slack_max - slack_min);
+ gNet->setTimingWeight(std::max(1.0f, std::min(net_weight_max_, weight)));
+ }
| if result.returncode != 0: | ||
| print(f"Build failed for {pdk} {design} (penalty={penalty}, span={span})") | ||
| return None | ||
| return {"WNS": 0.0, "TNS": 0.0, "Congestion": 0.0} |
There was a problem hiding this comment.
Returning hardcoded 0.0 values for WNS, TNS, and Congestion makes the autotuner completely non-functional, as it cannot compare different runs to find the optimal parameters. Please implement parsing of the actual metrics from the build reports or the generated metrics JSON file.
| - // weight(max_slack) = 1 | ||
| + float effective_slack_max = slack_max; | ||
| + if (timing_weight_span_clock_percent_ >= 0.0f) { | ||
| + float clock_period = 1.0f; // placeholder/normalized |
There was a problem hiding this comment.
Hardcoding clock_period = 1.0f as a placeholder makes the percent-based timing weight span calculation incorrect for any design with a non-1.0 clock period. Please query the actual clock period from the STA database or pass it as a parameter.
| append_env_var global_placement_args GPL_WIRELENGTH_PENALTY -wirelength_penalty 0 | ||
| append_env_var global_placement_args GPL_TIMING_SPAN_CLOCK_PERCENT -timing_weight_span_clock_percent 0 |
There was a problem hiding this comment.
The default values passed to append_env_var (0) do not match the default values defined in variables.yaml (1.0 for GPL_WIRELENGTH_PENALTY and -1.0 for GPL_TIMING_SPAN_CLOCK_PERCENT). This discrepancy can lead to inconsistent behavior if the environment variables are not explicitly set. Please update the defaults to match variables.yaml.
append_env_var global_placement_args GPL_WIRELENGTH_PENALTY -wirelength_penalty 1.0
append_env_var global_placement_args GPL_TIMING_SPAN_CLOCK_PERCENT -timing_weight_span_clock_percent -1.0
| result = subprocess.run(cmd, capture_output=True, text=True) | ||
| if result.returncode != 0: | ||
| print(f"Build failed for {pdk} {design} (penalty={penalty}, span={span})") | ||
| return None |
There was a problem hiding this comment.
Using capture_output=True buffers all stdout and stderr in memory, which makes the script appear completely frozen during long-running Bazel builds. Since the output is not parsed, it is better to run the command without capturing output so that the user can see the build progress in real-time.
| result = subprocess.run(cmd, capture_output=True, text=True) | |
| if result.returncode != 0: | |
| print(f"Build failed for {pdk} {design} (penalty={penalty}, span={span})") | |
| return None | |
| result = subprocess.run(cmd, text=True) | |
| if result.returncode != 0: | |
| print(f"Build failed for {pdk} {design} (penalty={penalty}, span={span})") | |
| return None |
921baa7 to
b20c122
Compare
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Advisory — results are log-only and do not affect build status. The authoritative QoR gate remains the local rules-file check. Commit 61 design(s) checked — 8 with regression(s), 0 without a comparable baseline.
❌ asap7/riscv32i-mock-sram base — 1 failing metric(s)
❌ ihp-sg13g2/ibex base — 1 failing metric(s)
❌ nangate45/mempool_group base — 1 failing metric(s)
❌ nangate45/swerv base — 2 failing metric(s)
❌ nangate45/swerv_wrapper base — 3 failing metric(s)
❌ sky130hd/gcd base — 1 failing metric(s)
❌ sky130hd/microwatt base — 2 failing metric(s)
❌ sky130hs/riscv32i base — 2 failing metric(s)
|
79daa60 to
12e0974
Compare
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
12e0974 to
62f5166
Compare
|
|
||
| if { [info exists ::env(GPL_WIRELENGTH_PENALTY)] } { | ||
| lappend global_placement_args -init_wirelength_coef $::env(GPL_WIRELENGTH_PENALTY) | ||
| } |
There was a problem hiding this comment.
remove iffy code default values are guaranteed so env vars here are always set.
| "//flow/designs/asap7/rocket:RocketTile_route" | ||
| ] | ||
|
|
||
| # Grid of hyper-parameters |
There was a problem hiding this comment.
this is a feeble autotuner at home, we need something like Optuna to drive the two dimensional search across all the designs per PDK, wire it up in bazel
| description: | | ||
| Global placement wirelength penalty (passed as -init_wirelength_coef or similar logic). | ||
| stages: | ||
| - place |
There was a problem hiding this comment.
add default values here from the global placement source code
2 participants
Update
variables.yamlandglobal_place.tclto expose OpenROAD global placement tuning parameters (-init_wirelength_coefand-virtual_cts_max_skew_fraction). Includes thetune_gpl_magic.pyBazel-idiomatic autotuning script to find optimal placement parameters for different designs using--define.Campaign Plan (Large Targets Sweep)
The
tune_gpl_magic.pyscript is set up to automatically sweep parameters for the following large target set to find the optimal global placement heuristics. We will be offloading this to a more powerful machine:sky130hd:
//flow/designs/sky130hd/ibex:ibex_core_route//flow/designs/sky130hd/jpeg:jpeg_encoder_route//flow/designs/sky130hd/microwatt:microwatt_route//flow/designs/sky130hd/riscv32i:riscv_routeasap7:
//flow/designs/asap7/mock-array:mock_array_route//flow/designs/asap7/swerv_wrapper:swerv_wrapper_route//flow/designs/asap7/tinyRocket:RocketTile_routeThe hyper-parameters we will sweep are:
GPL_WIRELENGTH_PENALTY: [0.1, 0.25, 0.5]GPL_TIMING_SPAN_CLOCK_PERCENT: [0.05, 0.10, 0.15]To execute the campaign, run: