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.4k stars 491 forks source link

Add slack histogram to the GUI #2649

Closed maliberty closed 1 year ago

maliberty commented 1 year ago

Description

Add a histogram of endpoint slack to the GUI

Suggested Solution

https://doc.qt.io/qt-5/qtcharts-barchart-example.html looks like a good starting point

Additional Context

No response

QuantamHD commented 1 year ago

@maliberty Do you have an example of the format of such a chart?

QuantamHD commented 1 year ago

I found this example online.

image

maliberty commented 1 year ago

That's what I'm thinking of. Having a generic histogram capability would be useful for many things (fanout, congestion, etc).

Sov-trotter commented 1 year ago

Hey All. First time contributor here! Would love to work on this issue. :)

QuantamHD commented 1 year ago

That's great!

The gui code is located here. https://github.com/The-OpenROAD-Project/OpenROAD/tree/master/src/gui

maliberty commented 1 year ago

Feel free to ask questions. @gadfort and I can help

Sov-trotter commented 1 year ago

Hey. I've been able to get a general layout done. image

Looking at the TimingWidget code I am not able to figure out where to get the data (number of paths and the slack). I assume that it's basic preprocessing before plotting, or do I need something like a TimingModel?

gadfort commented 1 year ago

You will likely need to use: https://github.com/The-OpenROAD-Project/OpenROAD/blob/bb4801b7457d04f2ddf27c202803dfbe5c8f324d/src/gui/src/staGuiInterface.cpp#L724 At the moment it's configured to return more than one path per endpoint. This is something relatively easy (I assume) to modify and I'm happy to do that if you want. Another thing to consider adding are controls for setup and hold slacks and corners (although those would be easy to get in once the widget is functional.

In the TimingPath class (which is what the getTimingPaths returns a list of) you should just be able to call getSlack for the slack on that path. I hope this helps.

Sov-trotter commented 1 year ago

At the moment it's configured to return more than one path per endpoint.

Thanks for adding this.

Another thing to consider adding are controls for setup and hold slacks and corners (although those would be easy to get in once the widget is functional.

Sure. This is my first time with Qt, so learning as I go along. :)

maliberty commented 1 year ago

Just checking in - are you able to make progress? Do you need any further help?

Sov-trotter commented 1 year ago

I am not quite able to wrap my head around the general flow. One specific question I have is Is getTimingPaths supposed to run after populatePaths/ updating?

I am not sure if I am getting the data in the right way to pass into the bar chart set.

maliberty commented 1 year ago

@gadfort would you help with this question.

gadfort commented 1 year ago

@Sov-trotter you should be using the STAGuiInterface class for your widget. All you need to do it call getTimingPaths() and that should give you all the paths. You question about populatePaths makes we think you are attempting to integrate into the timing widget (@maliberty I'm not sure this was the intent, but please clarify).

Also if you have a fork of OpenROAD you can always point us to it so we can better help with particular issues.

Sov-trotter commented 1 year ago

No I am not trying to integrate it into the timing widget. Working on a fork might be a good idea. I'll create one in sometime and let you know.

Sov-trotter commented 1 year ago

Hey. I am not quite able to figure out what pin should be the input parameter to getTimingPaths(). I looked around the code but couldn't find what is the appropriate from, thru or to value to get all the timing paths. Any pointers would be helpful. Thanks

gadfort commented 1 year ago

I don't think you need to supply any pins, you can call the function without any of those parameters and you will get all timing paths.

maliberty commented 1 year ago

I'm getting a bit confused. The slack histogram should show slack at timing end points (ie registers, macros, etc). Why do we need timing paths?

gadfort commented 1 year ago

@maliberty we don't technically need the timing paths (it's just the helper function is there and can be used without generating new helper functions). What we need is a single timing path for each endpoint (which the function can generate) and then we just need to mark the slack on the path (which the TimingPath has collected).

maliberty commented 1 year ago

Sta::vertexSlack is faster than getting a path.

maliberty commented 1 year ago

What does "mark the slack on the path" mean?

gadfort commented 1 year ago

The slack is set in the TimingPath object. Yes, you are right about the verterxSlack being faster, I figured that is something we could add to the STA GUI interface in the future, and modify this to use it. Ultimately it's the same information.

maliberty commented 1 year ago

I'm ok with two steps to get there. I think it will matter on large designs but it can wait for a follow on.

maliberty commented 1 year ago

@Sov-trotter how is it going with this? Do you have any code you can share?

Sov-trotter commented 1 year ago

Sorry for the late reply. I haven't been able to find time to work on this lately. I can't seem to fully wrap my head around the Qt API either.

maliberty commented 1 year ago

If you put your work in a public area perhaps someone can help move it forward. If there are specific questions about Qt I can try to answer.

Sov-trotter commented 1 year ago

Hmm in understand. But I won't be able to do much in the limited time I have these days. Thanks for letting me try :)

QuantamHD commented 1 year ago

@Sov-trotter I think it would be helpful for someone else to build on your code rather than starting over. Would it be possible to push what you have for those purposes?

niv280 commented 1 year ago

Hi @Sov-trotter , I will be happy to continue work on it. Can you please share your progress so far?

Sov-trotter commented 1 year ago

Yes sure. Till now I was pretty much at the Qt charts docs step.

AcKoucher commented 1 year ago

Hi Everyone! I've been working on this issue for more than a month, I'm a new contributor and it's helping me a lot to get to know openroad's structure. I apologize not keeping up with this thread. I have been in contact with @maliberty since I started. So, @niv280 would you mind letting me keep working on this? By the way, my general layouy looks like this: image

niv280 commented 1 year ago

Hi, Sure, I didn't started to work on this yet. The layout looks good, looking forward to see the full feature.

maliberty commented 1 year ago

I don't think we should use getTimingPaths to get end point slacks. I think we should add these apis:

StaPins STAGuiInterface::getEndPoints() const
{
  StaPins pins;
  for (auto end : *sta_->endpoints()) {
    pins.insert(end->pin());
  }
  return pins;
}

float STAGuiInterface::getPinSlack(const sta::Pin* pin) const
{
  return sta_->pinSlack(pin,
                        use_max_ ? sta::MinMax::max() : sta::MinMax::min());
}
AcKoucher commented 1 year ago

@maliberty that makes it clearer. Thank you for the apis' code. I'll test it.