Skip to content
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 Time + Delay column in Timing Report consistent with report_checks when clocks are not expanded #4916

Open
oharboe opened this issue Apr 6, 2024 · 7 comments

Comments

@oharboe
Copy link
Collaborator

oharboe commented Apr 6, 2024

Description

For all rows in the Data Path Details, the Time column is the previous Time + the current Delay.

Example of what is expected: 484.724+41.142=525.866

However, if I do this for the ces_0_0/clock (Element) line, that is not the case. The 42.713 is not added to the previous line 331.162.

This is from make DESIGN_CONFIG=designs/asap7/mock-array/config.mk route

To reproduce, untar gui.tar.gz

. vars-mock-array-asap7-base.sh
ODB_FILE=results/asap7/mock-array/base/5_route.odb ./run-me-mock-array-asap7-base.sh

image

Suggested Solution

Make the Delay column consistent with report_checks when clocks are not expanded:

>>> report_checks -from {ces_0_0/io_lsbOuts_7} -to {ces_0_4/io_lsbIns_4}
Startpoint: ces_0_0 (rising edge-triggered flip-flop clocked by clock)
Endpoint: ces_0_4 (rising edge-triggered flip-flop clocked by clock)
Path Group: clock
Path Type: max

  Delay    Time   Description
---------------------------------------------------------
   0.00    0.00   clock clock (rise edge)
 331.16  331.16   clock network delay (propagated)
   0.00  331.16 ^ ces_0_0/clock (Element)
 153.56  484.72 ^ ces_0_0/io_lsbOuts_7 (Element)
  41.14  525.87 ^ ces_0_1/io_lsbOuts_6 (Element)
  40.00  565.86 ^ ces_0_2/io_lsbOuts_5 (Element)
  36.94  602.80 ^ ces_0_3/io_lsbOuts_4 (Element)
   0.00  602.81 ^ ces_0_4/io_lsbIns_4 (Element)
         602.81   data arrival time

 250.00  250.00   clock clock (rise edge)
 307.80  557.80   clock network delay (propagated)
 -10.00  547.80   clock uncertainty
   8.79  556.59   clock reconvergence pessimism
         556.59 ^ ces_0_4/clock (Element)
  44.57  601.16   library setup time
         601.16   data required time
---------------------------------------------------------
         601.16   data required time
        -602.81   data arrival time
---------------------------------------------------------
          -1.64   slack (VIOLATED)

Additional Context

No response

@maliberty
Copy link
Member

You have ideal clocks enabled so the clock delay will be zero. It is showing you the computed delay but that value isn't summed in ideal clocks. We could zero it out though that info might be useful.

@maliberty maliberty added the gui GUI label Apr 8, 2024
@maliberty
Copy link
Member

I take that back - you do have clock propagation on. I don't understand the question though. In 484.724+41.142=525.866 where id 41.142 come from? Did you mean 42.713?

@maliberty
Copy link
Member

In the text report I see:

>>> report_checks -fields {slew cap input nets fanout}  -format full -through ces_0_0/io_lsbOuts_7
Startpoint: ces_0_0 (rising edge-triggered flip-flop clocked by clock)
Endpoint: ces_0_4 (rising edge-triggered flip-flop clocked by clock)
Path Group: clock
Path Type: max

Fanout     Cap    Slew   Delay    Time   Description
-----------------------------------------------------------------------------
                          0.00    0.00   clock clock (rise edge)
                        331.16  331.16   clock network delay (propagated)
                134.32    0.00  331.16 ^ ces_0_0/clock (Element)
     1    1.60    5.52  153.56  484.72 ^ ces_0_0/io_lsbOuts_7 (Element)

where the delay is 153.56 rather than 42.713.

@oharboe
Copy link
Collaborator Author

oharboe commented Apr 8, 2024

@maliberty Did you just answer your own question or is there a question for me?

@maliberty
Copy link
Member

If you expand the clock tree it is clearer:
image

The 42.713 is the last delay in producing the 331.162. You need to ad the delay of the next line to get the time.

@maliberty maliberty removed the gui GUI label Apr 8, 2024
@maliberty
Copy link
Member

I don't see a bug here, perhaps a misunderstanding?

@oharboe
Copy link
Collaborator Author

oharboe commented Apr 9, 2024

I don't see a bug here, perhaps a misunderstanding?

Aha! I get it.

Yes: I believe that this is a bug(or unintentional effect) in the representation in the GUI when lines are hidden.

When clock lines are not hidden, I believe that we see the intended representation of the delay column.

When clock lines are hidden, and one one wants the representation of the Delay column to be added to the previous Time to get current time, a choice has to be made for "clock network delay". It must either subtract the Delay on the next line or the Delay on the following line should be zero. I think it would be less surprising if the "ces_0_0/clock (Element)" line did not change, in which case, the "clock network delay" delay column have the delay of "ces_0_0/clock (Element)" subtracted.

Also, the GUI should match report_checks -from {ces_0_0/io_lsbOuts_7} -to {ces_0_4/io_lsbIns_4}.

Update GUI to match report_checks?

>>> report_checks -from {ces_0_0/io_lsbOuts_7} -to {ces_0_4/io_lsbIns_4}
Startpoint: ces_0_0 (rising edge-triggered flip-flop clocked by clock)
Endpoint: ces_0_4 (rising edge-triggered flip-flop clocked by clock)
Path Group: clock
Path Type: max

  Delay    Time   Description
---------------------------------------------------------
   0.00    0.00   clock clock (rise edge)
 331.16  331.16   clock network delay (propagated)
   0.00  331.16 ^ ces_0_0/clock (Element)
 153.56  484.72 ^ ces_0_0/io_lsbOuts_7 (Element)
  41.14  525.87 ^ ces_0_1/io_lsbOuts_6 (Element)
  40.00  565.86 ^ ces_0_2/io_lsbOuts_5 (Element)
  36.94  602.80 ^ ces_0_3/io_lsbOuts_4 (Element)
   0.00  602.81 ^ ces_0_4/io_lsbIns_4 (Element)
         602.81   data arrival time

 250.00  250.00   clock clock (rise edge)
 307.80  557.80   clock network delay (propagated)
 -10.00  547.80   clock uncertainty
   8.79  556.59   clock reconvergence pessimism
         556.59 ^ ces_0_4/clock (Element)
  44.57  601.16   library setup time
         601.16   data required time
---------------------------------------------------------
         601.16   data required time
        -602.81   data arrival time
---------------------------------------------------------
          -1.64   slack (VIOLATED)

@oharboe oharboe changed the title Make Time + Delay column in Timing Report consistent Make Time + Delay column in Timing Report consistent with report_checks Apr 9, 2024
@oharboe oharboe changed the title Make Time + Delay column in Timing Report consistent with report_checks Make Time + Delay column in Timing Report consistent with report_checks when clocks are not expanded Apr 9, 2024
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

No branches or pull requests

2 participants