Skip to content

[ENG-2040] set_path_margin implementation in OpenSTA#57

Open
stanminlee wants to merge 6 commits into
mainfrom
path-margin
Open

[ENG-2040] set_path_margin implementation in OpenSTA#57
stanminlee wants to merge 6 commits into
mainfrom
path-margin

Conversation

@stanminlee
Copy link
Copy Markdown

@stanminlee stanminlee commented May 22, 2026

Support for set_path_margin in OpenSTA

Overview of command

  • set_path_margin artificially changes the required time for specific paths
  • a negative margin will 'loosen' the timing requirement (useful skew)
  • a positive margin will 'tighten' the timing requirement (harmful skew)
  • we can specify -to, -from, and -through just as any other command similar to this will do

Main implementation details - full workflow

  1. define a "set_path_margin" proc in Sdc.tcl as the command entry. The proc will validate arguments and call the swig wrapper.
  2. we created a swig wrapper in Sdc.i to call the C++ used to construct the timing exception.
  3. define a makePathMargin command in Sdc.cc to create the timing exception
  4. Create full class entry for pathMargin in Exception.cc, mirroring implementations of other clock timing exceptions. Most important detail is that a path margin on a pin is defined by the 'last set_path_margin command set on that pin' which means the last path margin defined in TCL file for the pin will be used.
  5. In visitPathsEnds.cc, we add checks to account for the path margin in addition to other timing exceptions.
  6. When we visit the endpoint, we will apply the margin if it is non-zero (PathEnd.cc)
  7. Dump the path margin in ReportPath.cc

Various updates were made in some header files for class and function definitions.

Test case

  • Added one test case to test that setup/hold margins are applied, do affect the required time/slack, and show up in the STA report.

@linear
Copy link
Copy Markdown

linear Bot commented May 22, 2026

ENG-2040

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR implements set_path_margin, a new SDC command that artificially adjusts the required time for specific timing paths (positive margin tightens, negative loosens). The feature is wired end-to-end: Tcl proc → SWIG → C++ PathMargin exception class → targetClkPathMargin lookup in PathEndClkConstrained → reporting in reportClkUncertainty.

  • PathMargin is modeled as an ExceptionPath subclass with priority 500, lower than PathDelay (3000) and MultiCyclePath (2000) but higher than GroupPath (0); the sign convention negates positive margins for setup checks and leaves them unchanged for hold, matching the semantics described in the PR.
  • All four VisitPathEnds exception-filter conditions are updated so paths with only a set_path_margin exception are still visited and produce timing endpoints.
  • WriteSdc correctly serializes the new exception type; the test covers -to, -from, -through, combined constraints, clock-scoped startpoints, and both setup/hold polarities.

Confidence Score: 4/5

Safe to merge with awareness that set_path_delay + set_path_margin interactions are untested and the extra-args warning text is misleading.

The core path-margin lookup correctly uses the path-aware exceptionTo overload (propagating -from/-through tag states), sign conventions are validated by the test, and the exception priority/override structure mirrors existing PathDelay patterns. The two open points — the PathEndPathDelay + PathMargin simultaneous application and the confusing warning message — are minor and don't affect the primary use case covered by the test.

search/PathEnd.cc: the PathEndPathDelay path, which now also applies targetClkPathMargin on top of path_delay adjustments, deserves a test or a comment clarifying the intended interaction.

Important Files Changed

Filename Overview
sdc/ExceptionPath.cc Implements PathMargin constructor, clone, typePriority, tighterThan (always false), typeString, mergeable (always false), and overrides. Follows PathDelay pattern; tighterThan and overrides semantics intentional per author, but both lack explanatory comments (flagged in prior review threads).
search/PathEnd.cc Adds targetClkPathMargin to PathEnd (returns 0.0) and PathEndClkConstrained (does path-aware exceptionTo lookup with correct sign flip for setup). All three PathEnd clock-constrained subclasses (check, output delay, path delay) now include path margin in targetClkArrivalNoCrpr. The PathEndPathDelay+PathMargin interaction is untested.
sdc/Sdc.tcl Adds set_path_margin Tcl proc. Argument handling is correct but the extra-args warning text is misleading (margin is applied despite saying ignored). -rise/-fall flags are silently no-ops, consistent with set_max_delay behavior.
search/VisitPathEnds.cc Adds isPathMargin() to the four exception-filter conditions that allow path end creation to proceed. Consistent pattern applied to visitCheckEnd, visitOutputDelayEnd1, visitGatedClkEnd, and visitDataCheckEnd1.
search/ReportPath.cc Adds margin display in reportClkUncertainty. Correctly accumulates clk_arrival and omits the line when margin is zero. Placement (after uncertainty, before CRPR) matches test output.
sdc/WriteSdc.cc Adds set_path_margin to writeExceptionCmd (keyword + setup/hold flag) and writeExceptionValue (margin time). From/thru/to serialization reuses shared exception writers correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    TCL["set_path_margin Tcl proc\n(Sdc.tcl)"] --> SWIG["make_path_margin SWIG wrapper\n(Sdc.i)"]
    SWIG --> STA["Sta::makePathMargin\n(Sta.cc)\narrivalsInvalid()"]
    STA --> SDC["Sdc::makePathMargin\n(Sdc.cc)\naddException(PathMargin)"]
    SDC --> EX["PathMargin exception\n(ExceptionPath.cc)\npriority = 500 + fromThruToPriority"]
    VPE["VisitPathEnds\n(VisitPathEnds.cc)"] -->|"isPathMargin() → allow endpoint"| PE["PathEndClkConstrained\nPathEndOutputDelay\nPathEndPathDelay"]
    PE --> TCA["targetClkArrivalNoCrpr()\n+ targetClkPathMargin()"]
    TCA --> LOOKUP["Search::exceptionTo\n(path_margin type)\npath-aware lookup"]
    LOOKUP --> EX
    LOOKUP -->|"found"| SIGN{"checkRole == setup?"}
    SIGN -->|"yes"| NEG["return -margin\n(tightens required time)"]
    SIGN -->|"no"| POS["return +margin\n(tightens required time)"]
    NEG --> RPT["reportClkUncertainty\n(ReportPath.cc)\nshows 'path margin' line"]
    POS --> RPT
Loading

Reviews (3): Last reviewed commit: "better testing" | Re-trigger Greptile

Comment thread search/PathEnd.cc
Comment thread sdc/ExceptionPath.cc
Comment thread search/PathEnd.cc
@stanminlee stanminlee marked this pull request as draft May 22, 2026 00:37
@stanminlee stanminlee marked this pull request as ready for review May 22, 2026 05:51
@stanminlee
Copy link
Copy Markdown
Author

@greptile please review again

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.

1 participant