YosysHQ / nextpnr

nextpnr portable FPGA place and route tool
ISC License
1.24k stars 237 forks source link

heap: improved net weighting #1246

Closed Ravenslofty closed 6 months ago

Ravenslofty commented 7 months ago

This might be a little controversial.

While doing research, I found Agashiwala, 2015, which describes a quadratic analytical placer. While the paper itself wasn't too interesting, I found Formula 3.8 on timing-driven net weighting, which intrigued me because the formula in nextpnr scales the weight down with users, and this one scales the weight up with users.

So, I compared it to the old formula on 100 runs each of dragonmux's HeadphoneAmp, and got this:

firefox_IzQan83c3B

and my SPRT calculator is happy to accept this as an improvement for this design.

My question is: given how heap is the default placer everywhere in nextpnr, what's the criteria for "seems sufficiently unlikely to regress people's designs"?

Ravenslofty commented 7 months ago

Okay, this is quite weird; I keep forgetting that criticality is "backwards" [lower = more critical] in nextpnr, which means this is...increasing the weight of low-criticality, high-fanout nets and this is improving the maximum frequency of the design by a statistically significant amount. eh?

gatecat commented 7 months ago

lower = more critical

This definitely shouldn't be true

Ravenslofty commented 7 months ago

Am I misunderstanding how criticality in nextpnr works? Because I swear it is from things like router2's crit_weight being 1 - crit (ish).

(it's true I haven't slept yet, but I don't think I'm that off the mark)

gatecat commented 7 months ago

router2's crit_weight being 1 - crit

because we want to lower the congestion penalty for more critical nets, favouring a direct route for these.

Ravenslofty commented 7 months ago

In the hope that the dramatic wire length increase could be soothed with a wire length term, I added that to the formula.

Instead of easing wire length, this happened: image

which means this now has a 1.88 MHz increase in median Fmax, compared to 1.29 MHz without the wire length term.