cts: fix existing clock tree identification#10291
cts: fix existing clock tree identification#10291openroad-ci wants to merge 7 commits intoThe-OpenROAD-Project:masterfrom
Conversation
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
![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 updates the separateMacroRegSinks function in TritonCTS.cpp to properly handle nets that already contain timing buffers by skipping their sinks during separation and issuing a warning (CTS 110) if the net has fewer than two sinks. A new test case, buffer_ports, is added to verify this behavior. Feedback indicates a copy-paste error in the new test script where output and reference filenames do not match the test name.
| set vout_file [make_result_file gated_clock4.v] | ||
| write_verilog $vout_file | ||
| diff_files gated_clock4.vok $vout_file |
There was a problem hiding this comment.
The test script appears to use result and comparison filenames (gated_clock4.v and gated_clock4.vok) that do not match the test name (buffer_ports). This suggests a copy-paste error from another test (likely gated_clock4.tcl). Please update these to use names consistent with this test and ensure the corresponding reference file is correctly named and included.
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: arthurjolo <arthurjl@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
Summary
To avoid creating a clock tree on a clock net that already has a tree, CTS looks for a buffer with the source type TIMING. However other commands before CTS can insert buffer in the clock net that have the source type TIMING and CTS doesn't build the tree when it should.
This PR improves the identification of an existing clock tree to look if the TIMING buffer drives the clock net. Also, if there is a TIMING buffer as a load of the net, skip the buffer and build the clock tree only for the other sinks.
Type of Change
Impact
This fix corrects how separateMacroRegSinks identifies nets that already belong to an existing clock tree:
Output pin of a timing buffer (iterm->isOutputSignal() && isTimingBuffer): The buffer is driving this net, so the net genuinely already has a clock buffer — return false with CTS-0105. This is the same early-exit as before, but now correctly scoped to output pins only.
Input pin of a timing buffer (iterm->isInputSignal() && isTimingBuffer): The buffer is a sink that will be traversed by initOneClockTree, so it should be skipped rather than added to register/macro sink lists. The buffer instance is saved in skippedTimingBuf.
Post-loop guard (line 1503): If a timing buffer input was skipped but fewer than 2 real sinks remain, the net is effectively already covered by an existing tree — return false with the new CTS-0110 warning.
Verification
./etc/Build.sh).Related Issues
Fixes #10177