Skip to content

[ENG-1975] Fix clock rise/fall edge on 0 for non_seq timing types#53

Merged
akashlevy merged 3 commits intomainfrom
ckmux-timing
May 6, 2026
Merged

[ENG-1975] Fix clock rise/fall edge on 0 for non_seq timing types#53
akashlevy merged 3 commits intomainfrom
ckmux-timing

Conversation

@stanminlee
Copy link
Copy Markdown

non_seq_setup_rise
non_seq_setup_fall
non_seq_hold_rise
non_seq_hold_fall

not handled properly when populating data structure storing the clock edges to use when reporting timing checks.

leads to the release and capture clock being on the same edge, which is incorrect.

the change allows us to properly handle these timing types

@linear
Copy link
Copy Markdown

linear Bot commented May 5, 2026

ENG-1975

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR fixes incorrect cycle accounting for nonSeqSetup and nonSeqHold timing roles by including them in setSetupAccting and setHoldAccting, preventing release and capture clocks from being placed on the same edge. A new regression test (non_seq_timing) validates the corrected behavior on a CKMUX cell with non_seq_setup_rising / non_seq_hold_rising arcs.

  • sdc/CycleAccting.cc: Adds one setAccting(TimingRole::nonSeqSetup(), …) call to setSetupAccting and one setAccting(TimingRole::nonSeqHold(), …) call to setHoldAccting; all other setup/hold roles already present serve as the direct precedent for this pattern.
  • New test files: non_seq_timing.{lib,v,tcl,ok} provide end-to-end coverage of a clock-mux non-sequential setup check that was previously reported with wrong timing (same-edge capture), confirming a 9.89 ns slack (MET) after the fix.
  • test/regression_vars.tcl: Registers the new test in the public suite, though the insertion point is slightly out of alphabetical order.

Confidence Score: 5/5

Safe to merge — the one-line additions are narrowly scoped, follow the exact same pattern as the surrounding setup/hold roles, and are backed by a new regression test that exercises the corrected code path.

The change touches only two helper functions in CycleAccting.cc, each receiving a single additional setAccting call that mirrors what every adjacent timing role already does. The new regression validates the fix end-to-end and the expected output is arithmetically consistent with the Liberty constraints. No existing behavior is altered.

The new Liberty test library (test/non_seq_timing.lib) covers only rising-edge non-seq arcs; falling-edge variants mentioned in the PR description are not exercised by the test.

Important Files Changed

Filename Overview
sdc/CycleAccting.cc Adds nonSeqSetup() and nonSeqHold() to setSetupAccting and setHoldAccting so non-sequential timing roles receive proper cycle-accounting entries instead of defaulting to same-edge capture.
test/non_seq_timing.lib New Liberty library defining a CKMUX cell with non_seq_setup_rising and non_seq_hold_rising arcs on CLKSEL used to drive the regression test.
test/non_seq_timing.tcl New regression TCL script that reads the new lib, links the design, applies a single clock, and runs report_checks.
test/non_seq_timing.ok Expected output for the new test, showing a 9.89 ns slack (MET) path with a -0.05 library non-sequential setup constraint; validates the fix produces correct required-time computation.
test/non_seq_timing.v New Verilog netlist instantiating DFFHQx4_ASAP7_75t_R and CKMUX to create a path with a non-sequential timing check on CLKSEL.
test/regression_vars.tcl Registers non_seq_timing in the public test list, but the insertion point breaks the surrounding alphabetical order (inserted between two liberty_* / lib_* entries instead of after them).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CycleAccting::findSrcCycles / findDefaultArrivalSrcDelays"] --> B["setDefaultSetupAccting(src, tgt, delay, req)"]
    A --> C["setDefaultHoldAccting(src, tgt, delay, req)"]

    B --> D["setSetupAccting(src, tgt, delay, req)"]
    B --> E["setAccting(latchSetup)"]
    B --> F["setAccting(dataCheckSetup)"]

    D --> G["setAccting(setup)"]
    D --> H["setAccting(outputSetup)"]
    D --> I["setAccting(gatedClockSetup)"]
    D --> J["setAccting(recovery)"]
    D --> K["setAccting(nonSeqSetup) NEW"]

    C --> L["setHoldAccting(src, tgt, delay, req)"]
    C --> M["setAccting(dataCheckHold)"]

    L --> N["setAccting(hold)"]
    L --> O["setAccting(outputHold)"]
    L --> P["setAccting(removal)"]
    L --> Q["setAccting(latchHold)"]
    L --> R["setAccting(nonSeqHold) NEW"]
Loading

Reviews (2): Last reviewed commit: "remove redundant" | Re-trigger Greptile

Comment thread sdc/CycleAccting.cc Outdated
Comment thread sdc/CycleAccting.cc Outdated
@stanminlee stanminlee changed the title [CUS-511] Fix clock rise/fall edge on 0 for non_seq timing types [ENG-1975] Fix clock rise/fall edge on 0 for non_seq timing types May 5, 2026
@akashlevy
Copy link
Copy Markdown

@greptile re-review

@akashlevy akashlevy merged commit 1a6aeaa into main May 6, 2026
6 checks passed
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.

2 participants