global_route: catch failures to generate deterministic artifacts - #4416
Conversation
![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 a helper procedure run_global_route_and_catch_failures in flow/scripts/global_route.tcl to centralize error handling and artifact generation during global routing. The reviewer identified two issues with this helper: potential errors if the GENERATE_ARTIFACTS_ON_FAILURE environment variable is undefined, and compatibility issues with the single-process flow where standard artifact-writing functions might be bypassed. A code suggestion was provided to use env_var_equals and direct log_cmd calls to ensure robustness.
1d1c421 to
3bd1f73
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a helper procedure run_global_route_and_catch_failures in flow/scripts/global_route.tcl to centralize failure handling and artifact generation during global routing. Feedback points out that using log_cmd write_db and log_cmd write_sdc directly bypasses the WRITE_ODB_AND_SDC_EACH_STAGE check, which can cause failures in the single-process flow. It is recommended to use orfs_write_db and orfs_write_sdc instead.
Wraps all incremental global_route invocations (e.g. post repair_design or recover_power iterations) in proper catch blocks. If global_route fails or becomes infeasible mid-flow, it writes out a deterministic 5_1_grt-failed.odb rather than crashing without diagnostic artifacts. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
3bd1f73 to
581b8fa
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a helper procedure, run_global_route_and_catch_failures, in flow/scripts/global_route.tcl to wrap global routing executions and gracefully handle failures. The feedback suggests several improvements to this helper: using env_var_equals to safely check the GENERATE_ARTIFACTS_ON_FAILURE environment variable without throwing errors if it is undefined, preserving the original stack trace when rethrowing caught errors by passing $::errorInfo to error, and directly calling log_cmd write_sdc and log_cmd write_db to ensure failure artifacts are written regardless of other configuration settings.
| orfs_write_sdc $::env(RESULTS_DIR)/5_1_grt.sdc | ||
| orfs_write_db $::env(RESULTS_DIR)/5_1_grt.odb | ||
| if { ![do_global_route $res_aware $use_cugr] } { | ||
| return |
There was a problem hiding this comment.
Strange... why is global_route run after we have written the output, should the global route results below be thrown away?
| } | ||
|
|
||
| puts "Estimate parasitics..." | ||
| log_cmd estimate_parasitics -global_routing |
There was a problem hiding this comment.
I would have expected .sdc and .odb to be written out here... why isn't it?
🔍 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 — 0 with regression(s), 0 without a comparable baseline.
|
|
@maliberty Please read my comments: why isn't write_odb/sdc happening at the end in global_route.tcl like all other scripts? |
3 participants
Consistent handling with the original GENERATE_ARTIFACTS_ON_FAILURE_POLICY by using a wrapper function.
Wraps all incremental global_route invocations (e.g. post repair_design or recover_power iterations) in proper catch blocks. If global_route fails or becomes infeasible mid-flow, it writes out a deterministic 5_1_grt-failed.odb rather than crashing without diagnostic artifacts.