Skip to content

Enabled read_vcd/read_saif as part of metrics extraction - #4763

Closed
jeffng-or wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:enable-vcd-saif-power
Closed

jeffng-or wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:enable-vcd-saif-power

Conversation

@jeffng-or

Copy link
Copy Markdown
Contributor

piped through read_vcd/read_saif with three new flow variables:

  • VCD_FILE
  • SAIF_FILE
  • POWER_FILE_SCOPE

@jeffng-or jeffng-or self-assigned this Oct 28, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for power calculation using SAIF or VCD files by adding three new configuration variables: POWER_FILE_SCOPE, SAIF_FILE, and VCD_FILE. These variables are documented and integrated into the reporting metrics script. The review feedback correctly identifies two issues in flow/scripts/report_metrics.tcl: a regex bug in get_power_file_scope_args where escaped parentheses prevent correct capturing, and a line of dead code appending to an unused additional_args variable.

Comment thread flow/scripts/report_metrics.tcl Outdated
set scope_val [string trim $scope_val "\"'"]

# 2. Normalize: remove any backslashes immediately preceding [, ], or .
regsub -all {\\(\[|\]|\.)} $scope_val {\1} scope_val

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The regular expression \\(\[|\]|\.) escapes the parentheses ( and ), which treats them as literal characters instead of a capturing group. This causes the pattern to match literal parentheses and incorrectly match any standalone ] character, replacing it with an empty string or causing unexpected behavior. It should be written without escaping the parentheses to correctly capture the character for the \1 replacement.

    regsub -all {\\([\[\].])} $scope_val {\1} scope_val

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jeffng-or an issue?

Comment thread flow/scripts/report_metrics.tcl Resolved
@openroad-ci

openroad-ci commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit d5a739f · Jenkins build #2 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 1 without a comparable baseline.

@maliberty

Copy link
Copy Markdown
Member

Need yaml formatting

adjusted RE per claude + yaml formatting
tcl fmt

Signed-off-by: Jeff Ng <jeffng@precisioninno.com>
Signed-off-by: Jeff Ng <jeffng@precisioninno.com>
@openroad-ci
openroad-ci force-pushed the enable-vcd-saif-power branch from 4d93d5d to f95b31b Compare October 29, 2026 15:31
@jeffng-or

Copy link
Copy Markdown
Contributor Author

Need yaml formatting

Fixed

@jeffng-or jeffng-or closed this Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants