Sta latest 0609#370
Conversation
Signed-off-by: James Cherry <cherry@CerezoBook.local>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
* Use DynamicLatch in DispatchQueue to avoid main thread spinning Replace the busy-yielding pending_task_count_ loop in DispatchQueue::finishTasks with a blocking DynamicLatch. This avoids having the main thread consume CPU cycles while waiting for dispatched tasks to complete. The DynamicLatch implementation uses C++20 std::atomic::wait/notify_all for efficient blocking and wakeup, with proper release-acquire semantics to ensure task results are visible to the waiting thread. * Reformat DynamicLatch to match DispatchQueue style * Update attribution headers in DispatchQueue files to note modifications
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
This reverts commit 452f0bf.
|
James Cherry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![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 introduces support for Ubuntu 24.04, updates macOS Homebrew TCL path detection, replaces spinning in DispatchQueue with a C++20 DynamicLatch, optimizes path group lookups, and adds custom precision formatting to reportArrivals. Feedback on these changes includes adding g++ to the new Dockerfile to prevent build failures, passing this instead of network_edit_ in SdcNetwork::makePort to avoid potential null pointer dereferences, sorting #include headers lexicographically in DispatchQueue.hh, and removing a redundant double semicolon in PathGroup.cc.
| git \ | ||
| wget \ | ||
| cmake \ | ||
| gcc \ |
There was a problem hiding this comment.
The package list includes gcc but is missing g++ (or build-essential). Since OpenSTA is a C++ project, the build will fail during the CMake compilation phase if a C++ compiler is not installed. Please add g++ to the package list.
gcc \\
g++ \\
| SdcNetwork::makePort(Cell *cell, | ||
| std::string_view name) | ||
| { | ||
| std::string escaped_name = escapeDividerBrackets(name, network_edit_); |
There was a problem hiding this comment.
Passing network_edit_ to escapeDividerBrackets is risky because network_edit_ can be nullptr if the adapted network is not editable. Since escapeDividerBrackets only needs the Network interface (to query pathDivider() and pathEscape()), you should pass this instead of network_edit_. This is also consistent with how escapeDividerBrackets is called in makeInstance and makeNet.
std::string escaped_name = escapeDividerBrackets(name, this);| #include <cstddef> | ||
| #include <condition_variable> |
There was a problem hiding this comment.
According to the repository style guide, #include lines must be sorted lexicographically by the path inside <> or "". Alphabetically, <condition_variable> should come before <cstddef>.
| #include <cstddef> | |
| #include <condition_variable> | |
| #include <condition_variable>\n#include <cstddef> |
References
- Sort #include lines lexicographically by the path inside <> or "" (case-sensitive). (link)
| ? path_delay_group_name_ | ||
| : group_path->name(); | ||
| if (path_group_name == group_name) | ||
| return true;; |
There was a problem hiding this comment.
There is a redundant double semicolon ;; at the end of the return true statement. Please remove the extra semicolon.
return true;
|
Not addressing any of Gemini's concerns as these came from upstream and don't really seem like breaking anything. |
|
Corresponding OpenROAD PR The-OpenROAD-Project/OpenROAD#10626 |
* Address a few typos in docs Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> * update help string for report_checks to reflect variation Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com> --------- Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
Signed-off-by: James Cherry <cherry@CerezoBook.local>
3 participants
Merge from upstream 6/9. Had one commit of mine that is reverted too. Needs some test updation as a few function signatures changed. Same reason OpenROAD compilation fails- will land that in a separate PR