JSON power reports

[donn]

• power/Power.tcl:
  • add -format flag to report_power, two options: "text", "json"
  • modified report_power_design, report_power_insts to accept a report format and modified accordingly
  • modified report_power_inst, report_power_row to accept both a report format and sentinel variables (to add commas for valid json output)
• tcl/Util.tcl: added new convenience function upvar_opt
• added test for power_json

---

Important

Adds JSON output support for power reports and includes a test case to verify the functionality.

• Behavior:
  • Adds -format flag to report_power in Power.tcl with options "text" and "json".
  • Updates report_power_design, report_power_insts, report_power_inst, and report_power_row to handle JSON format.
• Utilities:
  • Adds upvar_opt function in Util.tcl for optional variable linking.
• Testing:
  • Adds power_json.tcl test to verify JSON output for power reports.
  • Registers power_json test in regression_vars.tcl.

^{This description was created by for e32a6f0. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to e32a6f0 in 2 minutes and 12 seconds. Click for details.

• Reviewed 247 lines of code in 4 files
• Skipped 1 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. power/Power.tcl:114

• Draft comment:
  Replace '==' with 'eq' in conditional checks (e.g. report_format comparison) for clarity and correctness when comparing strings.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% In TCL, 'eq' is indeed preferred for string comparisons as it's more explicit and can avoid issues with numeric conversions. However, in this case the strings being compared are simple literals that can't be confused with numbers. The code works correctly as is, and this is a minor style preference rather than a correctness issue. The suggestion is technically correct - 'eq' is generally considered better practice in TCL. The change would make the code slightly more idiomatic. While technically correct, this is a minor style improvement that doesn't affect correctness or maintainability in any significant way given the simple string literals being compared. Delete the comment. While using 'eq' would be slightly better style, the current code works correctly and this is too minor of an improvement to warrant a comment.

2. power/Power.tcl:188

• Draft comment:
  Similarly, change '==' to 'eq' in report_power_row and report_power_inst when comparing the report_format.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% In Tcl, 'eq' is indeed the preferred operator for string comparison as it's more explicit and can be more efficient than '=='. However, '==' works correctly for string comparison in Tcl and is widely used. This is a style preference rather than a correctness issue. The code works correctly as is. The suggestion is technically correct as a style improvement, but the existing code is not wrong or problematic. The change would be very minor and mostly cosmetic. While using 'eq' would follow best practices, the current code is functional and clear. The benefit of changing it is minimal. While technically correct, this suggestion is a minor style preference that doesn't materially improve the code quality or fix any actual issues.

3. tcl/Util.tcl:191

• Draft comment:
  The new upvar_opt procedure is clear; consider adding brief parameter documentation to clarify its intended usage.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. power/Power.tcl:257

• Draft comment:
  Typo: In the call to report_power_inst, the last argument is passed as last_inst_var but should likely be $last_inst_var so that the variable's value is used.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% In TCL, when passing a variable name to be used with upvar, you pass the name itself, not the value. The code is using upvar_opt which needs the variable name. The comment author seems to have misunderstood this TCL pattern. The code is actually correct as-is. Could there be some edge case where using $ would be correct? Could I be misunderstanding how upvar works in TCL? No, I'm confident about this. The pattern of passing a variable name (not value) to upvar is a standard TCL idiom. We can see it's working correctly because the code successfully sets the variable in the caller's scope on line 293. The comment is incorrect - the code is using the TCL upvar pattern correctly by passing the variable name. The comment should be deleted.

Workflow ID: wflow_cXGJ7mPeoqNWz4lR

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[greptile-apps]

Greptile Summary

This PR adds JSON output support to the report_power command in OpenSTA, extending the existing power reporting functionality to support machine-readable output formats. The change introduces a -format flag to the report_power command with two options: "text" (default) and "json".

The implementation modifies several core functions in power/Power.tcl: report_power_design, report_power_insts, report_power_inst, and report_power_row to handle JSON formatting. The JSON output structure mirrors the text format but provides structured data with nested objects containing power categories (internal, switching, leakage, total, and percent values). For instance reports, each instance is represented as a JSON object with the instance name and power breakdown.

To handle comma placement in JSON arrays and objects, the implementation uses "sentinel variables" that track the last output element. A new utility function upvar_opt was added to tcl/Util.tcl to cleanly handle optional variable linking - it creates an upvar reference only when the variable name is non-empty, otherwise sets the target variable to empty string. This pattern avoids explicit conditionals in the reporting code and provides proper JSON comma separation.

The change includes comprehensive testing with a new power_json.tcl test file that exercises both design-level and instance-level JSON power reporting. The test is properly registered in the regression test suite via test/regression_vars.tcl. The implementation maintains full backward compatibility by defaulting to text format when no -format flag is specified.

Confidence score: 4/5

• This PR is safe to merge with low risk of production issues
• Score reflects well-structured JSON implementation with proper testing, but uses string comparison operators that could be improved
• The main power/Power.tcl file should be reviewed for the string comparison pattern noted in previous review

_{4 files reviewed, 2 comments}

_{Edit Code Review Bot Settings | Greptile}