Cts fix sink count recursion

[openroad-ci]

Summary

Fix a loop in the recursive function to compute the number of sinks post DB write.
The top net of a macro drive drives both the macro tree top buffer and the register tree top buffer, the recursion to count the sinks of the macro tree was leaking into the register tree. Since the sinks of the register tree are not a part of the sinks in the macro tree the recursion never stopped and it ended up leaking into the data nets, that contain loops.

To avoid this issue, this PR prevents the macro tree sink count from going into the register tree. Also throw an error when the count leaks into the data nets.

Type of Change

• Bug fix

Impact

Prevent crash from leaking into data nets.

Verification

• I have verified that the local build succeeds (./etc/Build.sh).
• I have run the relevant tests and they pass.
• My code follows the repository's formatting guidelines.
• I have signed my commits (DCO).

Related Issues

[Link issues here]

[gemini-code-assist]

Code Review

This pull request implements cycle detection and recursion control in the countSinksPostDbWrite function to prevent infinite loops and unintended traversal into register trees. The review feedback identifies a potential bug where an empty top buffer name could lead to incorrect sink counts and points out redundant return statements after terminal error logs.

[gemini-code-assist]

This logic assumes that builder->getTopBufferName() is always populated. However, in some cases (e.g., single-buffer trees created via createSingleBufferClockNet), topBufferName_ may be an empty string. If it is empty, the condition builder->getTopBufferName() != inst->getName() will evaluate to true for the legitimate top buffer, causing the recursion to terminate prematurely and resulting in an incorrect sink count of zero.

Consider checking if the top buffer name is non-empty before applying this termination logic. Additionally, it would be best to ensure topBufferName_ is correctly set in HTreeBuilder::createSingleBufferClockNet to fully resolve the leak issue for all tree types.

      if (builder->getTreeType() != TreeType::RegisterTree) {
        const std::string& topBufferName = builder->getTopBufferName();
        if (!depth && !topBufferName.empty() && topBufferName != inst->getName()) {
          terminate = true;
          trueSink = false;
        }
      }

[gemini-code-assist]

The return; statement after logger_->error is redundant. According to the general rules of this repository, logger_->error is expected to terminate the program, so additional control flow or error handling is unnecessary.

    logger_->error(CTS,
                   369,
                   "Count sinks recursion leaked into data net {}",
                   net->getName());

References

1. Calls to logger_->error are expected to terminate the program, so additional error handling like returning an error code or throwing an exception is not necessary.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[precisionmoon]

@arthurjolo, can you add a regression test also?

[arthurjolo]

@arthurjolo, can you add a regression test also?

Any regression test with a macro and register tree works for it. The crash only happens if the sinks from the clock tree are not sequential elements, as the recursive function always returns when it finds a sequential element.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[precisionmoon]

@arthurjolo, can you add a regression test also?

Any regression test with a macro and register tree works for it. The crash only happens if the sinks from the clock tree are not sequential elements, as the recursive function always returns when it finds a sequential element.

Can you add a test which will fail without your fix? This way, we avoid running into the same issue later.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[precisionmoon]

Arthur J, thanks.