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.35k stars 482 forks source link

Add filtering in Timing Report and End point slack histogram for register to register paths only #5000

Open oharboe opened 3 weeks ago

oharboe commented 3 weeks ago

Description

Timing Reports and Endpoint slack histogram does not distinguish between timing close for register to register paths and timing paths that are there simply to give an optimization target. For more detail on this see https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/1969.

To reproduce:

make DESIGN_CONFIG=designs/asap7/mock-array/Element/config.mk

Then launch GUI:

make DESIGN_CONFIG=designs/asap7/mock-array/Element/config.mk gui_final

image

Suggested Solution

Add a filtering option to Timing Report and Endpoint Slack histogram to only consider register to register paths.

I think it would be nice to be able to articulate a report_checks command type filtering in Timing Report, rather than have lots of buttons.

Endpoint slack histogram could be representing what is in the Timing Report (except that the endpoint slack histogram should not have a limit on number of paths).

I would like to see an endpoint slack histogram for this query:

report_checks -from [all_registers] -to [all_registers] -path_delay max

Additional Context

No response

oharboe commented 3 weeks ago

Prototype https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/pull/1978

I wrote a bit of .tcl that launched a Python script to make matplotlib histogram for reg-reg endpoints

image

maliberty commented 2 weeks ago

How about an option in the settings?

maliberty commented 2 weeks ago

Note that there isn't necessarily a clean split. An endpoint could have reg-to-reg paths and io-to-reg paths. The endpoint doesn't really know which is the source so implementing this is not obivous.

oharboe commented 2 weeks ago

Note that there isn't necessarily a clean split. An endpoint could have reg-to-reg paths and io-to-reg paths. The endpoint doesn't really know which is the source so implementing this is not obivous.

Ah. What I want is a register to register slack histogram. That is unambigious, no?

io to reg, and reg to io paths in mock-array/Element are part of reg to reg paths in mock-array and therefore should not appear in reg to reg slack histograms.

oharboe commented 2 weeks ago

How about an option in the settings?

I think that would be adequate. I dont know how much more useful a more flexible system would be and it would certainly be a higher threshold for the user. Perhaps start with an option and refine if we learn more about other useful usecases(if they exist) that are common enough to warrant GUI support?

maliberty commented 2 weeks ago

Ah. What I want is a register to register slack histogram. That is unambigious, no?

It is a histogram of end-point slack, not start-end pairs. You would have to recompute the whole timing graph in order to exclude the IOs (and reset it back when done). It doesn't seem very practical.

oharboe commented 2 weeks ago

I dont think that is what is needed.

I believe that what I am looking for is the endpoint slack for reg-to-reg paths, i.e. a categorization.of existing endpoint slack histogram.

Perhaps the endpoint slack histogram could be grouped and colored and displayed as stacked bars?

That way there are no new userinterface buttons or recomputations.

oharboe commented 2 weeks ago

Example....

I forgot about io to io paths here, but not all designs will have them. I think excluding unused categories cleans up the graph.

It should be possible to click on a color within a bucket and get only those paths filtered and displayed in Timing Report.

For the slack range 0-10: 10 'reg to reg', 5 'io to reg', and 15 'reg to io'. For the slack range 10-20: 8 'reg to reg', 12 'io to reg', and 10 'reg to io'. For the slack range 20-30: 6 'reg to reg', 9 'io to reg', and 5 'reg to io'.

image

maliberty commented 2 weeks ago

@tspyrou does sta have tags to represent this info? I don't see any efficient way to implement this request short of modifying the constraints and rebuilding the whole graph.

maliberty commented 2 weeks ago

The difficulty is that slack at an endpoint is a single number not a set of numbers based on these categories afaik. Its isn't a matter of presentation.

oharboe commented 2 weeks ago

@maliberty So the feature request hangs together, there just isnt a reasonable way to do it, other than run one query per category. Would the time required to run the queries be number of categories * time to run a query today?

oharboe commented 2 weeks ago

If filtering is fast and delay calculation is slow, then running the query multiple times would be fine.

maliberty commented 2 weeks ago

I talked with Tom and his suggestion is to try generating paths reg2reg paths using

report_checks -from [all_registers] -to [all_registers] -endpoint_count [llength [all_registers]] -unique_paths_to_endpoint

which should give a path per endpoint which can be used for the endpoint slack. Likewise with -from [all_inputs] and -to [all_outputs] for the in2reg and reg2out paths.

Note that an endpoint may appear in in2reg and reg2reg so the histogram total could exceed the endpoint total.

@AcKoucher would you test this out in tcl first to see if that gives the desired info. If so we can figure out the c++ equivalent.

b62833 commented 2 weeks ago

@maliberty, @oharboe I think you have this built in already. Do something like

group_path -name r2r -from [all_registers] -to [all_registers]
report_checks -path_group r2r

I haven't tried it though. Most of the production flows I've used do something like this.

To toss out another idea, the production flows I've run in use virtual clocks to constrain I/Os. The virtual clock latency is zero initially and then gets updated after CTS (or anything else that changes real clock latency) to match the real clock latency. Then your I/O don't clutter up the timing reports. And not everyone registers inputs and outputs so you do need to time them accurately some times.

The definition of "match" can vary. There's either a native tool command to do this automatically or a script as part of the normal flow.

maliberty commented 2 weeks ago

The slack histogram is not the same as report_checks. It is a value per endpoint not a set of paths.

AcKoucher commented 2 weeks ago

@oharboe The idea behind using red/green was to better illustrate the endpoints whose slack is negative. Don't you think that the stacked bars idea would make it harder to interpret the data?

@maliberty So if I understand correctly, there's no way to know the exact path to which a certain endpoint we see in the histogram belongs? I. e. we can't answer the question: in what path does that endpoint possesses the slack we see in histogram? I mean, if we do, it looks like it would be just a matter of checking the first and last pins of the path to see if they belong to registers.

I'll test the .tcl version.

oharboe commented 2 weeks ago

@AcKoucher I think I see what you mean. We primarily want to know worst endpoint slack for reg to reg paths. It is nice to have the other categories, but for our flow they are optimization target violations, not timing closure violations, so not "real violations" in a sense

AcKoucher commented 2 days ago

@oharboe So how about another dropdown menu to select the type of paths? Something like:

"Start/End Filter"

Then for each one we could keep the green and red colors to maintain the violated/non-violated slack histogram visibility logic.

oharboe commented 2 days ago

@oharboe So how about another dropdown menu to select the type of paths? Something like:

"Start/End Filter"

  • No Filter shows everything like the current implementation
  • Register to register
  • IO to Register
  • Register to IO
  • IO to IO

Then for each one we could keep the green and red colors to maintain the violated/non-violated slack histogram visibility logic.

Perfect!

rovinski commented 15 hours ago

IO to IO

@AcKoucher typically these are called "feedthrough" paths

maliberty commented 13 hours ago

I would prefer IO to IO as feedthru generally means a metal only connection between pins.

oharboe commented 11 hours ago

Also, think the context here are four choices of reg & io. regreg, ioio, ioreg, regio. I think in this context introducing feedthrough in lieu of ioio is a stumbling block.