Skip to content

[DO NOT REVIEW] synth: add retime#3337

Closed
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
Pinata-Consulting:synth-retime
Closed

[DO NOT REVIEW] synth: add retime#3337
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
Pinata-Consulting:synth-retime

Conversation

@oharboe
Copy link
Copy Markdown
Collaborator

@oharboe oharboe commented Jul 21, 2025

Do a run to see if it has any effect good or bad

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe marked this pull request as draft July 21, 2025 16:01
@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

@maliberty Is there an easy way to tell if this is an outdated rules file or if it is an improvement?

$ make DESIGN_CONFIG=designs/asap7/mock-array/config.mk update_ok
Metric Old New Type
synth__design__instance__area__stdcell 35273.33 6243.29 Tighten
finish__timing__setup__ws -457.76 -434.65 Tighten
finish__timing__drv__setup_violation_count 605 536 Tighten
finish__timing__wns_percent_delay -111.1 -110.88 Tighten

@maliberty
Copy link
Copy Markdown
Member

Even as an improvement it looks rather small. You could run master and compare the results.

When the PR finishes running you could use the dashboard (https://dashboard.precisioninno.com/compare?sourceAType=Branch&sourceBType=Branch)

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

Even as an improvement it looks rather small. You could run master and compare the results.

I didn't expect a material difference for mock-array, I was just curious if it was different.

When the PR finishes running you could use the dashboard (https://dashboard.precisioninno.com/compare?sourceAType=Branch&sourceBType=Branch)

I know MegaBoom requires a retiming for the floating point paths, but is there a design in ORFS that is made with the assumption of retiming?

@maliberty
Copy link
Copy Markdown
Member

Not to my knowledge (though that is limited)

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

With this change, I get:

make DESIGN_CONFIG=designs/asap7/mac/config.mk cts
image

Same design without changes to abc scripts:

image

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

@povik Any idea if there's a way to get retiming going on abc? My changes appears to have no effect(good or bad)

I've made a test design, multiply and accumulate, with 3 pipeline stages, which should be a good test design.

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

@maliberty @tspyrou Are we missing an OpenSTA update?

The intent is to express a max_delay from the output pin of the register to the output pin of this design:

group_path -name reg2out -from [all_registers] -to [all_outputs]

The way things are now, there's a chicken and egg problem: you can't express the max_delay without knowing the propagation delay up to valid_pipe[3]$_DFF_P_/QN.

That said, the max_delay is there to overconstrain optimization

>>> report_checks -path_group reg2out
Startpoint: valid_pipe[3]$_DFF_P_
            (rising edge-triggered flip-flop clocked by clock)
Endpoint: valid_out (output port)
Path Group: reg2out
Path Type: max

  Delay    Time   Description
---------------------------------------------------------
 132.67  132.67   clock network delay (propagated)
   0.00  132.67 ^ valid_pipe[3]$_DFF_P_/CLK (DFFHQNx3_ASAP7_75t_R)
  65.06  197.73 v valid_pipe[3]$_DFF_P_/QN (DFFHQNx3_ASAP7_75t_R)
  10.69  208.42 ^ _2200_/Y (CKINVDCx16_ASAP7_75t_R)
  17.26  225.68 ^ output66/Y (BUFx12f_ASAP7_75t_R)
  16.69  242.37 ^ valid_out (out)
         242.37   data arrival time

  80.00   80.00   max_delay
   0.00   80.00   output external delay
          80.00   data required time
---------------------------------------------------------
          80.00   data required time
        -242.37   data arrival time
---------------------------------------------------------
        -162.37   slack (VIOLATED)

@povik
Copy link
Copy Markdown
Contributor

povik commented Jul 21, 2025

@oharboe Hi Øyvind. The ABC integration in Yosys, as used by our synthesis script, only transfers the combinational part of the network to ABC. Under those circumstances passes like &retime can't do much.

I know there's a Yosys option to pass flops to ABC (abc -dff) but to make use of it requires special sequencing of commands different from our script. To me it's an untested option and I don't know if it can be made to work well. It needs a special script to be written. If you want to pursue this further I'd take it up with the Yosys community or YosysHQ.

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 21, 2025

@povik Thanks for the clarification!

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Aug 2, 2025

@tspyrou FYI

@povik povik requested a review from maliberty August 4, 2025 16:55
@openroad-ci openroad-ci mentioned this pull request Aug 13, 2025
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