-
Notifications
You must be signed in to change notification settings - Fork 593
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
rmp: Adds DelayOptimization pass to remapper #6477
Conversation
Adds new pass based infrastructure to add new Abc optimization passes. The first delay based optimization is added here. General intention is to build lots of interfaces that can be extended by both the public and private clients. Signed-off-by: Ethan Mahintorabi <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Ethan Mahintorabi <[email protected]>
Signed-off-by: Ethan Mahintorabi <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Ethan Mahintorabi <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
How is this accessed? it looks like the only clients that could use this would be via CPP. There is no access via tcl or python |
@@ -0,0 +1,8 @@ | |||
# For debugging optimizations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be committed? Likewise the sdc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would find it helpful if they were committed, but I'll remove them if you feel strongly
#include "utl/deleter.h" | ||
|
||
namespace abc { | ||
extern Abc_Ntk_t* Abc_NtkMap(Abc_Ntk_t* pNtk, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not defined in an abc header than you can include?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's not, I can send a PR to ABC to fix that, but would rather not block this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't block it but it would be nice to fix this and the const issue in abc.
utl::UniquePtrWithDeleter<abc::Abc_Ntk_t> current_network( | ||
abc::Abc_NtkToLogic(const_cast<abc::Abc_Ntk_t*>(ntk)), | ||
&abc::Abc_NtkDelete); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Abc_NtkToLogic return a new Abc_Ntk_t? If so it shouldn't require a non-const input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Abc_NtkToLogic and most ABC passes are const-able, but ABC never declares them as const. I set const on the Optimize input to at least let OpenROAD devs that the input network isn't modified. I can try to send a PR to make it const as well, but once again I don't think this should block the PR.
Signed-off-by: Ethan Mahintorabi <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
Please fix the DCO |
Is this a progress step in part of larger plan? I shared Peter's confusion. |
Signed-off-by: Ethan Mahintorabi <[email protected]>
clang-tidy review says "All clean, LGTM! 👍" |
Short answer this isn't done yet. I'm breaking up the implementation into two different strategy classes
For the first implementation I'm planning on implementing a class called the The ZeroSlackSynthesisStrategy is the thing that can be exposed via tcl or python. Ideally though it just ends up inside resizer and called implicitly in GPL Separately it seems like the DelayOptimizationClass can provide a lot of delay benefits. See below the worst endpoint in AES Nangate45 running at 1.5Ghz before and after optimization. BeforeAfter |
@andyfox-rushc in case you have any comments from your experience in this area. |
Adds new pass based infrastructure to add new Abc optimization passes. The first delay based optimization is added here.
General intention is to build lots of interfaces that can be extended by both the public and private clients.