gui: add filter to Charts based on Start/End path type
Resolve #5000
@oharboe @maliberty Would you happen to know a design in which we'd have all the path start/end type for a more consistent test?
clang-tidy review says "All clean, LGTM! :+1:"
@oharboe @maliberty Would you happen to know a design in which we'd have all the path start/end type for a more consistent test?
Try designs/asap7/mock-array/Element
@AcKoucher @maliberty I tried on mock-array/Element:
- Why are IO to REG listed with clock and reg to io listed with
? - IO - IO have no clock. This isn't entirely surprising, it is a max delay path only. Can this path be shown in the endpoint slack histogram? If so, under what clock, if any? Add "
" to endpoint histogram? - register to register paths exist with "clock".
- "No filter" and "Register to register" show the same. The other filters show now end-paths.
No endpoints shown in histogram:
When it first loads I see:
which appears to be equivalent to
Would you set the initial state to "No Filter"
@AcKoucher is looking at the lack of IO related paths
The set of clocks is not filter dependent and just shows what are defined in the design.
If I click on one of the buckets, I get a filtered list in Timing Report. I think the same dropdown filter would be useful in the Timing Report.
I am sure this raises some questions to sort through...
clang-tidy review says "All clean, LGTM! :+1:"
If I choose "Register to register" (or anything but "No filter"), I get a crash:
untar gui-crash.tar.gz
$ ./run-me-mock-array_Element-asap7-base.sh
OpenROAD v2.0-13792-g7457e3a7e
Features included (+) or not (-): +Charts +GPU +GUI +Python
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.
read_spef ./results/asap7/mock-array_Element/base/6_final.spef
Signal 11 received
Stack trace:
0# 0x00006335527B8693 in openroad
1# 0x0000729FC5C42990 in /lib/x86_64-linux-gnu/libc.so.6
2# std::_Rb_tree_increment(std::_Rb_tree_node_base const*) in /lib/x86_64-linux-gnu/libstdc++.so.6
3# gui::ChartsWidget::fetchConstrainedPins(std::set<sta::Pin const*, std::less<sta::Pin const*>, std::allocator<sta::Pin const*> >&, bool) in openroad
4# gui::ChartsWidget::changeStartEndFilter() in openroad
5# 0x0000729FC6F06312 in /lib/x86_64-linux-gnu/libQt5Core.so.5
6# QComboBox::currentIndexChanged(int) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
7# 0x0000729FC7C732E6 in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
8# 0x0000729FC7C76C5B in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
9# 0x0000729FC7C76ECB in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
10# 0x0000729FC6F0602D in /lib/x86_64-linux-gnu/libQt5Core.so.5
11# QComboBoxPrivateContainer::itemSelected(QModelIndex const&) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
12# QComboBoxPrivateContainer::eventFilter(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
13# QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Core.so.5
14# QApplicationPrivate::notify_helper(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
15# QApplication::notify(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
16# QCoreApplication::notifyInternal2(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Core.so.5
17# QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
18# 0x0000729FC7BC886A in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
19# 0x0000729FC7BCB12F in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
20# QApplicationPrivate::notify_helper(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Widgets.so.5
21# QCoreApplication::notifyInternal2(QObject*, QEvent*) in /lib/x86_64-linux-gnu/libQt5Core.so.5
22# QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) in /lib/x86_64-linux-gnu/libQt5Gui.so.5
23# QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) in /lib/x86_64-linux-gnu/libQt5Gui.so.5
24# 0x0000729FC1EF9F7E in /lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
25# 0x0000729FC5F13B2C in /lib/x86_64-linux-gnu/libglib-2.0.so.0
26# 0x0000729FC5F6F46F in /lib/x86_64-linux-gnu/libglib-2.0.so.0
27# g_main_context_iteration in /lib/x86_64-linux-gnu/libglib-2.0.so.0
28# QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) in /lib/x86_64-linux-gnu/libQt5Core.so.5
29# QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) in /lib/x86_64-linux-gnu/libQt5Core.so.5
30# QCoreApplication::exec() in /lib/x86_64-linux-gnu/libQt5Core.so.5
31# gui::startGui(int&, char**, Tcl_Interp*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) in openroad
32# ord::tclAppInit(Tcl_Interp*) in openroad
33# Tcl_MainEx in /lib/x86_64-linux-gnu/libtcl8.6.so
34# main in openroad
35# 0x0000729FC5C28150 in /lib/x86_64-linux-gnu/libc.so.6
36# __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
37# _start in openroad
./run-me-mock-array_Element-asap7-base.sh: line 7: 11889 Segmentation fault (core dumped) openroad -no_init -gui ${SCRIPTS_DIR}/gui.tcl
@oharboe The changes in this draft are neither ready for testing nor review.
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
@oharboe Results for the the element test case of your previous comment:
All endpoints:
Filters:
@AcKoucher Perfect! Ready for testing?
@oharboe Yes. Go ahead :)
@AcKoucher Should the same dropdown filter be available from the Timing Report menu?
Of course I can click on one column in the filtered histogram, which gives me the top paths in for that filter in that slack range, so I have what I need.
I'm unsure if that complication to the GUI is worth the potential benefit.
@oharboe Well, something to have in mind is that we're dealing with an endpoint slack histogram. The report that you get from clicking in a certain range gives you the paths of those endpoints, but that doesn't mean you'll get only the paths within that filter.
An endpoint shown in the reg2reg filter might as well appear in the io2reg filter. That's why the "no filter" option - which presents all the endpoints - is not necessarily a superposition of the filtered ones.
I. e., we don't store the startpoint information in the histogram.
However, being able to see those filter options in the Timing Report does seem to be a nice idea. We can generate the paths in .tcl of course though. As a user do you think it'd be useful?
@maliberty Would you have some opinion?
@AcKoucher My domain knowledge isn't up to the task of saying what exactly would be useful here. The limited case that I am interested in and think I understand is that every macro must have timing closure for all reg2reg paths, because those paths are no longer visible where the macro is used as the .lib file doesn't contain information about those paths anymore.
@AcKoucher But from a usability point of view, it is surprising that clicking on an empty column can show paths in the Timing Report. And purely from a usability point of view, surprises should be minimized...
Hmm.... choosing a filter is much slower than showing the no-filter version of the histogram. Is that expected.
Still running minutes later....
A few suspend in debugger shows no debug info(so release build) and the code is gnawing on the same bone:
@oharboe That is expected, because for all endpoints we just get the worst slack to show in the histogram. For the filters we have to query the paths from sta based on the start/end combination which can be slow if there are a lot of endpoints.
As for the empty column I agree with you and I think it shouldn't be that way either, I'll wait for @maliberty 's input.
@AcKoucher Thanks for the clarification. So how much slower could it be? Looks stuck here. I'm thinking 1 minute for the first one and still running after 20 minutes or so.
Is there nothing to be done about the slowdown?
This is the information I need, so if I have to wait, I have to wait...
@oharboe I need to make some more testing to confirm. Perhaps there's more to it.
The filter did complete in the end, perhaps an hour compared to 1 minute with no filter?
Let's prove the viability of this before we worry about adding it to the timing report.