Merge upstream OpenSTA 08/14 - #403
Conversation
Signed-off-by: James Cherry <cherry@CerezoBook.local>
…` (#471) * Implement set_max_dynamic_power and set_max_leakage_power. Store max dynamic/leakage power like max area, expose getters through Sta, write them in write_sdc, and document that they are ignored during timing. Co-authored-by: Cursor <cursoragent@cursor.com> * Drop get_max_* Tcl wrappers; use SWIG accessors in tests. James pointed out the get_max_* commands duplicate existing SWIG accessors; remove them and call sta::max_* from the regression. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Use get_port_pin instead of find_pin on the port leaf name so escaped hierarchical port names like level1/level2/level3 resolve correctly. Also fix arglilst typo in get_port_pins_error. Co-authored-by: Cursor <cursoragent@cursor.com>
* Implement set_path_margin command Adds a new set_path_margin SDC command that applies a signed slack adjustment to the capture-clock side of timing paths. A positive margin tightens the path (harder to meet) and a negative margin loosens it. Supports -setup/-hold/-from/-through/-to scoping, priority/override semantics matching other exceptions, text and JSON report output, and write_sdc serialisation. Adapted from Silimate PR The-OpenROAD-Project#57; uses upstream/master Mode/Scene and string_view APIs. Test uses unset_path_exceptions in place of the Silimate-only reset_path alias. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix nested delaySum call indentation to match project style. Co-authored-by: Cursor <cursoragent@cursor.com> * Address review: store PathMargin on PathEnd, split tests Keep the path margin exception on PathEndClkConstrained, add PathEnd::hasPathMargin for report gating, and split the monolithic regression into set_path_margin1–6. Rebased onto upstream/master and document the command in ChangeLog. Co-authored-by: Cursor <cursoragent@cursor.com> * Document set_path_margin in OpenSTA.fodt/pdf Add the command reference, index entry, and note that unset_path_exceptions also clears path margin exceptions. Co-authored-by: Cursor <cursoragent@cursor.com> * Drop [[nodiscard]] from PathEnd::hasPathMargin. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
…ix columns (#486) * support for filter in get_scene/mode Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * fix the openroad timeout issue Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> --------- Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: Minju Kim <mkim@precisioninno.com>
Upstream e9c075a (BIGFEATURE: Add `set_path_margin` command, parallaxsw#469) added a `PathMargin *path_margin` parameter to the PathEndCheck, PathEndOutputDelay and PathEndGatedClock constructors, between `mcp` and the argument that followed it. Our per-module gtests are not in the upstream tree, so the upstream change did not touch them and the merge left them calling the old arity. Pass nullptr for the new parameter; these cases only exercise type()/typeName()/copy(), so no margin is needed. No behavioral change - test-only API adaptation. Signed-off-by: Minju Kim <mkim@precisioninno.com>
|
James Cherry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![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 the set_path_margin SDC command to apply signed slack adjustments to timing paths, alongside implementing the set_max_dynamic_power and set_max_leakage_power constraints. It also replaces direct pin lookups with get_port_pin to properly handle escaped port names containing slashes. A critical issue was identified in search/PathEnd.cc where checkRole(sta) can return nullptr, potentially causing a segmentation fault when calling genericRole(). A null check should be added before dereferencing this pointer.
| float | ||
| PathEndClkConstrained::targetClkPathMargin(const StaState *sta) const | ||
| { | ||
| if (!path_margin_) | ||
| return 0.0; | ||
| float margin = path_margin_->margin(); | ||
| if (checkRole(sta)->genericRole() == TimingRole::setup()) | ||
| margin = -margin; | ||
| return margin; | ||
| } |
There was a problem hiding this comment.
The function checkRole(sta) can return nullptr (for example, the default implementation in PathEnd::checkRole returns nullptr, and some subclasses of PathEndClkConstrained like PathEndPathDelay do not override it). Calling genericRole() on a nullptr will result in a segmentation fault. A null check should be added before dereferencing the pointer returned by checkRole(sta).
float
PathEndClkConstrained::targetClkPathMargin(const StaState *sta) const
{
if (!path_margin_)
return 0.0;
float margin = path_margin_->margin();
const TimingRole *role = checkRole(sta);
if (role && role->genericRole() == TimingRole::setup())
margin = -margin;
return margin;
}
|
Thanks Minju for this! Can you please add the links of the secure ci from the ORFS too? I assume this doesn't require any metric update, right? |
|
recheck |
4 participants
Summary
Merge upstream OpenSTA
c3e092033e39c153e8260ddbe99f674f22fb165cintomaster.masterb548398c6cef8c91542685aa66646f243f9f1fc0Upstream commits pulled in
Local changes on top of the merge
Anything listed here is a fix the merge required. Decide whether it
belongs upstream before merging.
Verification
100% tests passed, 0 tests failed out of 6097- run20260814-c3e09203Generated by
etc/sta_sync/sta_sync.sh, run id20260814-c3e09203.