-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make: gui_ prerendering #2610
base: master
Are you sure you want to change the base?
make: gui_ prerendering #2610
Conversation
gui::select_clockviewer_clock $clock_name | ||
save_image -resolution $resolution $::env(REPORTS_DIR)/cts_${clock_name}_layout.webp | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gadfort "How do I unselect the clock tree and return the GUI to the default view?" |
@maliberty Should grt::have_routes return false silently if the instances are not placed?
|
@maliberty Tried on a larger design, works nicely. Nits need to be fixed, broken out as issues. |
How about The-OpenROAD-Project/OpenROAD#6307 ? |
d918d86
to
0e63d2b
Compare
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
less click and wait when the GUI opens for large designs clock tree prerendering is still missing, because there is no way yet to disable the clock tree rendering from .tcl after the clock tree has been rendered. Some progress added so that the user can tell what it is that is taking time. Also added a final gui::unminimize log item so the user knows when the GUI should have unminimized(which it doesn't on Ubuntu 24.04 with Wayland yet, known issue filed). make gui_route [deleted] estimate_parasitics -global_routing Populating timing paths...OK Prerendering Placement heatmap... Prerendering Routing heatmap... Prerendering RUDY heatmap... Prerendering Power heatmap... gui::select_chart "Endpoint Slack" gui::update_timing_report gui::unminimize The-OpenROAD-Project/OpenROAD#6074 Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
0e63d2b
to
64d013d
Compare
@maliberty Ready for review. Preprendering clock tree is still missing, but that's a simple change once this is through and that missing clock tree feature is available in OpenROAD. |
flow/scripts/open.tcl
Outdated
# FIXME reenable when there is a way to disable the rendered clock tree | ||
# | ||
# foreach clock [get_clocks *] { | ||
# if { [llength [get_property $clock sources]] > 0 } { | ||
# set clock_name [get_name $clock] | ||
# save_clocktree_image -clock $clock_name \ | ||
# -width 100 -height 100 \ | ||
# $::env(OBJECTS_DIR)/dummy.png | ||
# break | ||
# } | ||
# } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open should not write files as a side effect. This should be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but how else to precalculate this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it actually save time? Most of the effort is to get sta warmed up. Drawing the tree should be cheap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, last I checked.
However, I can delete the commented code and if this PR is otherwise good to go, we can square away what we already have a solution for and I can then create a new PR with a narrower concern and some numbers.
A way to precalculate this that does not modify GUI state and also does not write files to disk is needed, which isnt available yet, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if there really is a need. I would be curious to see the time saved.
delete clock tree commented out code for now, revisit later. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@maliberty Fixed, should be ready for merge now. This is how far we've come with prerendering for now. Will revisit more later in followup PRs. |
flow/scripts/open.tcl
Outdated
} | ||
} | ||
|
||
set have_routes [expr {$placed && [grt::have_routes]}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need to compute/check $placed? I thought this was fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes: there is no ppl::have_placed method. I can simplify the code a bit here though for the have_routes case.
flow/scripts/open.tcl
Outdated
} | ||
puts "Prerendering $heatmap heatmap..." | ||
gui::set_heatmap $heatmap rebuild 1 | ||
gui::dump_heatmap $heatmap $::env(REPORTS_DIR)/dummy.png |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid the file side effect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes: fixed
simplify now that grt::have_routes checks if placement is done first. no need to save dummy.png to prerender. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@maliberty Fixed, should be good to go. |
less click and wait when the GUI opens
Clock tree prerendering is still missing, because there is no way yet to disable the clock tree rendering from .tcl after the clock tree has been rendered.
Some progress added so that the user can tell what it is that is taking time. Also added a final gui::unminimize log item so the user knows when the GUI should have unminimized(which it doesn't on Ubuntu 24.04 with Wayland yet, known issue filed).