OpenROAD icon indicating copy to clipboard operation
OpenROAD copied to clipboard

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

Open oharboe opened this issue 1 year ago • 25 comments

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 avatar Apr 26 '24 05:04 oharboe

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

oharboe avatar Apr 26 '24 20:04 oharboe

How about an option in the settings?

maliberty avatar Apr 28 '24 02:04 maliberty

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.

maliberty avatar Apr 28 '24 02:04 maliberty

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 avatar Apr 28 '24 05:04 oharboe

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?

oharboe avatar Apr 28 '24 05:04 oharboe

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.

maliberty avatar Apr 28 '24 06:04 maliberty

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 avatar Apr 28 '24 08:04 oharboe

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

oharboe avatar Apr 28 '24 08:04 oharboe

@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 avatar Apr 28 '24 14:04 maliberty

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.

maliberty avatar Apr 28 '24 16:04 maliberty

@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 avatar Apr 28 '24 16:04 oharboe

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

oharboe avatar Apr 28 '24 16:04 oharboe

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.

maliberty avatar Apr 29 '24 16:04 maliberty

@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.

b62833 avatar May 02 '24 01:05 b62833

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

maliberty avatar May 02 '24 17:05 maliberty

@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.

AcKoucher avatar May 03 '24 15:05 AcKoucher

@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

oharboe avatar May 03 '24 15:05 oharboe

@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.

AcKoucher avatar May 15 '24 23:05 AcKoucher

@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!

oharboe avatar May 16 '24 01:05 oharboe

IO to IO

@AcKoucher typically these are called "feedthrough" paths

rovinski avatar May 18 '24 02:05 rovinski

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

maliberty avatar May 18 '24 04:05 maliberty

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.

oharboe avatar May 18 '24 06:05 oharboe

To me, feedthrough paths are any paths which are purely combinational from IO to IO. A metal-only path is just a short, which would be a special type of feedthrough. "IO to IO" is fine to keep uniformity.

rovinski avatar May 19 '24 00:05 rovinski

@rovinski I disagree and find it inconsistent with LEF.

maliberty avatar May 19 '24 00:05 maliberty

I didn't know there was something in LEF, but from what I see it is related to shapes or pins. Multiple major vendors use the term "feedthrough path" for combinational paths, e.g. AMD and Cadence. I am fine with not using the term here, but I just want to alert that it has been used in this context.

rovinski avatar May 19 '24 09:05 rovinski