grt/rsz: write spef for estimated parasitics #5656
Conversation
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
src/rsz/include/rsz/Resizer.hh
Outdated
| void estimateParasitics(ParasiticsSrc src, const char* file_path = ""); | ||
| void estimateWireParasitics(const char* path = ""); |
There was a problem hiding this comment.
std::string_view is more flexible than char*
There was a problem hiding this comment.
There is a request to take
std::map<Corner*, std::ostream*>& spef_files
instead of a file_path so that the client can control the output (to a ofstream or a ststream). The .i could handle setting these up for file based output.
There was a problem hiding this comment.
Note that corners could be a subset of all available corners
There was a problem hiding this comment.
You could keep the write to file version an an overload that uses the other API as its implementation.
| file_path.append(suffix); | ||
| } | ||
|
|
||
| spef_file[corner].open(file_path, std::ofstream::out); |
There was a problem hiding this comment.
You should check that open succeeded
|
|
||
| void Resizer::writeSpefPorts(Corner* corner, dbNetwork* network) | ||
| { | ||
| auto pin_iter = network->pinIterator(network->topInstance()); |
There was a problem hiding this comment.
sta is old fashioned and this pointer must be freed by the caller. The easiest is to store the result value in a unique_ptr.
| network->staToDb(pin, iterm, bterm, moditerm, modbterm); | ||
|
|
||
| spef_file[corner] << bterm->getName() << " "; | ||
| if (bterm->getIoType() == odb::dbIoType::INPUT) { |
There was a problem hiding this comment.
What if this isn't a bterm? An iterm is possible and a mod* should be an error.
|
Not sure I'm fond of -spef as a flag to |
|
sta reduces the parasitics so you can't write them afterwards except in reduced form which isn't that helpful. |
|
But could you, e.g., just add a Tcl hook or a SWIG hook which determines which function to call? Or are you saying that to write the SPEF without reduced parasitics then it has to be written out immediately? |
|
Yes they have to be written immediately as the unreduced form is not stored anywhere. Storing it would be quite expensive on the off change someone wants to write them out. |
|
That's fair then. |
|
@joaomai there is some feedback that still needs addressing before this can be merged. |
…ofstream Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Other than the two comments, it looks good to me. I'll approve, but will wait for @maliberty final review.
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@maliberty Any more comments? I believe we need your approval to merge this PR. |
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
| @@ -281,29 +281,26 @@ void Resizer::estimateParasitics(ParasiticsSrc src) | |||
| void Resizer::estimateParasitics(ParasiticsSrc src, | |||
| std::map<Corner*, std::ostream*>& spef_streams) | |||
| { | |||
| SpefWriter* spef_writer = nullptr; | |||
There was a problem hiding this comment.
Its better to use std::unique_ptr and avoid manual memory management (eg exception safety).
src/rsz/src/SpefWriter.cc
Outdated
| auto it = spef_streams_.find(corner); | ||
| if (it == spef_streams_.end()) { | ||
| return; | ||
| spef_streams_.clear(); |
There was a problem hiding this comment.
Its going be destructed so there is no need to clear first. The dtor doesn't need to be defined.
src/rsz/src/SpefWriter.cc
Outdated
| } | ||
|
|
||
| void SpefWriter::writeSpefPorts(Corner* corner) | ||
| std::string getIoDirectionText(const odb::dbIoType& ioType) |
There was a problem hiding this comment.
char would do for a return type.
…agement Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
src/rsz/include/rsz/SpefWriter.hh
Outdated
| @@ -64,7 +64,7 @@ class SpefWriter | |||
| SpefWriter(Logger* logger, | |||
| dbSta* sta, | |||
| std::map<Corner*, std::ostream*>& spef_streams); | |||
| ~SpefWriter(); | |||
| ~SpefWriter() = default; | |||
There was a problem hiding this comment.
Not needed as the compiler will generate this automatically.
…g the default Signed-off-by: João Mai <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Great work! |
Fixes #4441.
Added as an option to the estimate_parasitics command, using -spef_file to pass the file path as an argument, writing one or more files based on how many corners there are.
The information is written to the file during estimation, writing the net data before RC reduction is performed.
I'm unsure about the units in the header, using the same ones from the rcx header currently.