The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.38k stars 485 forks source link

add report on cells count #4588

Closed arthurjolo closed 3 months ago

arthurjolo commented 4 months ago

Fixes #1695

This PR adds a command (report_cell_count) to report the cells count before CTS, after CTS and after Filler cells have been added.

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

QuantamHD commented 4 months ago

Should we be writing this in TCL? It seems like we should add something to the C++ api that returns a map<cell_type, int> or something like that instead.

maliberty commented 4 months ago

I agree this shouldn't be in TCL as it won't be available in python. You could separate the stats summary, as Ethan suggests, from the actual reporting.

I don't think the "Cells before/after" messages are needed.

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

arthurjolo commented 4 months ago

A small change now that the code is in C++, the command to report the cells count is now: sta::report_cell_count

QuantamHD commented 4 months ago

A small change now that the code is in C++, the command to report the cells count is now: sta::report_cell_count

Nice! I like the change a lot.

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 4 months ago

This looks very close to gui's DbInstDescriptor::getInstanceType & DbInstDescriptor::getInstanceTypeText. I'd like to avoid having two pieces of code that do nearly, but not quite, the same thing.

Could we move the guts of those functions to your code and call them from GUI? The count function would iterate and sum by calling this new method per inst.

arthurjolo commented 4 months ago

This looks very close to gui's DbInstDescriptor::getInstanceType & DbInstDescriptor::getInstanceTypeText. I'd like to avoid having two pieces of code that do nearly, but not quite, the same thing.

Could we move the guts of those functions to your code and call them from GUI? The count function would iterate and sum by calling this new method per inst.

@maliberty I belive this last commit addresses this. The DbInstDescriptor::getInstanceTypeText I maintained as it was in DbInstDescriptor but using the new dbSta::getInstanceType().

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 4 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 3 months ago

Please show a sample output

arthurjolo commented 3 months ago

Please show a sample output

An output example would be:

Reporting Cells count:
  Macro: 6
  Fill: 523662
  Tapcell: 58169
  Antenna: 412
  Buffer/inverter: 242
  Clock buffer/inverter: 81
  Buffer/inverter from timing repair: 288
  Sequential: 525
  Combinational: 1928
precisionmoon commented 3 months ago

Please show a sample output

An output example would be:

Reporting Cells count:
  Macro: 6
  Fill: 523662
  Tapcell: 58169
  Antenna: 412
  Buffer/inverter: 242
  Clock buffer/inverter: 81
  Buffer/inverter from timing repair: 288
  Sequential: 525
  Combinational: 1928

Does "Combinational" include buffers, clock buffers, buffers from timing repair? The report can be made clearer with something like the following:

Cell usage report: Macros: 6 Fill cells: 523662 Tap cells: 58169 Antenna cells: 412 Data buffers/inverters: 242 Clock buffers/inverters: 81 Timing repair data buffers/inverters: 288 Sequential cells: 525 Complex combinational cells: 1928

Alternate Tcl command name like report_cell_usage may be good also.

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

arthurjolo commented 3 months ago

Complex

Combinational do not include buffers/inverters. I changed the report. That wxample now is:

Cell usage report:
  Macros: 6
  Fill cells: 523662
  Tap cells: 58169
  Antenna cells: 412
  Buffer/inverters: 242
  Clock buffer/inverters: 81
  Timming Repair Buffer/inverters: 288
  Sequential cells: 525
  Complex combinational cells: 1928
github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

precisionmoon commented 3 months ago

Complex

Combinational do not include buffers/inverters. I changed the report. That wxample now is:

Cell usage report:
  Macros: 6
  Fill cells: 523662
  Tap cells: 58169
  Antenna cells: 412
  Buffer/inverters: 242
  Clock buffer/inverters: 81
  Timming Repair Buffer/inverters: 288
  Sequential cells: 525
  Complex combinational cells: 1928

Just a small typo: "Timming" should read "Timing"

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

maliberty commented 3 months ago

Please add a unit test and command documentation.

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"

arthurjolo commented 3 months ago

@maliberty I am having some difficulties on finding where to add the documentation for the command. The dbSta directory doesn’t have a README.md file, is there another directory I can add the documentation?

maliberty commented 3 months ago

Src read me is ok as is somewhat global

github-actions[bot] commented 3 months ago

clang-tidy review says "All clean, LGTM! :+1:"