Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cts enable obstruction aware #5900

Open
wants to merge 39 commits into
base: master
Choose a base branch
from

Conversation

arthurjolo
Copy link
Contributor

This PR removes the CTS option -obstruction_aware and enables it by default, and adds improvements to prevent metrics degradation:

  • Remove CTS option -obstruction_aware
  • Insert delay to balance clock gater tree
  • Improve clock gater handling to correctly inset delay.

precisionmoon and others added 30 commits March 15, 2024 03:28
1) treat clock gaters like macro cell instances
2) need to change arrival computation

Signed-off-by: Cho Moon <[email protected]>
1) arrival computation is now recursive
2) latency adjustments are made from bottom up

Signed-off-by: Cho Moon <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthurjolo <[email protected]>
Signed-off-by: arthurjolo <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Signed-off-by: arthur <[email protected]>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -247,6 +247,12 @@ class TreeBuilder
void setTopBufferDelay(float delay) { topBufferDelay_ = delay; }
odb::dbInst* getTopBuffer() const { return topBuffer_; }
void setTopBuffer(odb::dbInst* inst) { topBuffer_ = inst; }
std::string getTopBufferName() const { return topBufferName_; }
void setTopBufferName(std::string name) { topBufferName_ = name; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'name' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]

src/cts/src/TreeBuilder.h:43:

- #include <vector>
+ #include <utility>
+ #include <vector>
Suggested change
void setTopBufferName(std::string name) { topBufferName_ = name; }
void setTopBufferName(std::string name) { topBufferName_ = std::move(name); }

pathStartNet = port->getNet();
}
if (pathStartNet == topNet) {
clkPathArrival = path->arrival(openSta_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Value stored to 'pathsAccepted' is never read [clang-analyzer-deadcode.DeadStores]

        pathsAccepted += 1;
        ^
Additional context

src/cts/src/TritonCTS.cpp:2117: Value stored to 'pathsAccepted' is never read

        pathsAccepted += 1;
        ^

Comment on lines 2200 to 2217
return;
} else {
// not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not use 'else' after 'return' [readability-else-after-return]

Suggested change
return;
} else {
// not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}
} // not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}

Comment on lines +2220 to +2221
}
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: redundant return statement at the end of a function with a void return type [readability-redundant-control-flow]

Suggested change
}
return;
}

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

}
if (pathStartNet == topNet) {
clkPathArrival = path->arrival(openSta_);
pathsAccepted += 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Value stored to 'pathsAccepted' is never read [clang-analyzer-deadcode.DeadStores]

        pathsAccepted += 1;
        ^
Additional context

src/cts/src/TritonCTS.cpp:2118: Value stored to 'pathsAccepted' is never read

        pathsAccepted += 1;
        ^

Comment on lines +2201 to +2218
} else {
// not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not use 'else' after 'return' [readability-else-after-return]

Suggested change
} else {
// not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}
}
} // not a sink, but a clock gater
odb::dbITerm* outTerm = inst->getFirstOutput();
if (outTerm) {
odb::dbNet* outNet = outTerm->getNet();
if (outNet) {
odb::dbSet<odb::dbITerm> iterms = outNet->getITerms();
odb::dbSet<odb::dbITerm>::iterator iter;
for (iter = iterms.begin(); iter != iterms.end(); ++iter) {
odb::dbITerm* inTerm = *iter;
if (inTerm->getIoType() == odb::dbIoType::INPUT) {
computeSinkArrivalRecur(
topClokcNet, inTerm, sumArrivals, numSinks, graph);
}
}
}
}

Comment on lines +2221 to +2222
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: redundant return statement at the end of a function with a void return type [readability-redundant-control-flow]

Suggested change
return;
}
}

@arthurjolo arthurjolo marked this pull request as ready for review October 11, 2024 14:42
@arthurjolo
Copy link
Contributor Author

Secure-CI is has few metrics failures and a PR has been created to update them, a7/mock_array and ng45/bp_fe have degradation on timing metrics due to a bad placement decision from obstruction aware, I am already working on fixing this issue.

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