Skip to content

Conversation

davidbiancolin
Copy link
Contributor

See firesim/firesim#602. Resolves #27.

@davidbiancolin davidbiancolin requested a review from jerryz123 July 14, 2020 17:47
@davidbiancolin davidbiancolin merged commit f22f7ca into dev Jul 14, 2020
@davidbiancolin davidbiancolin deleted the promote-critical-wanrings branch July 14, 2020 17:55
impl_step route_phys_opt_design $TOP $post_phys_options $post_phys_directive $post_phys_preHookTcl $post_phys_postHookTcl
}
# Check if slack has improved after physopt.
set SLACK [get_property SLACK [get_timing_paths]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Vivado get_timing_paths return them in WNS sorted order? Other tools I've used, return them in in clock_group order or something else.

Looking at this code I'm wondering if you need to do something like

set SLACK [tcl::mathfunc::min [get_property SLACK [get_timing_paths]]]

Assuming:

  1. Vivado's TCL interpreter is new enough that you can use tcl::mathfunc::min
  2. get_property will return a list of SLACK when given the paths returned by get_timing_paths

Another thing I thought of, is hold timing. I hope that Vivado is smart enough to consider hold timing violations as fatal errors. Have you ever encountered min-delay timing violations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was originally written by AWS but you raise a good point. I don't see why they'd be sorted.
Page 880 of the Vivado TCL reference guide describes a built-in min, i'll use that.

This command will not check for hold-time violations, without some modification. I've never encountered them though. I'll open another PR with both of these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants