verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1k stars 388 forks source link

add detailed routing stage parameter to reclustering utils #2584

Closed KA7E closed 3 months ago

KA7E commented 3 months ago

Added detailed routing stage parameter to pack_mol_in_existing_cluster and start_new_cluster_for_mol; both functions pass this parameter to try_pack_molecule.

Description

Related Issue

Motivation and Context

Without this option, the legalizer runtime is untenable.

How Has This Been Tested?

Types of changes

Checklist:

KA7E commented 3 months ago

@vaughnbetz could you please review? Subsequent legalizer PRs depend on this.

KA7E commented 3 months ago

Re: QoR. This option is key to speeding up the legalizer, since it allows it to use pin counting legality checking instead of runtime intensive intra-cluster routing. I do not think it has any impact on the rest of VPR, since it only impacts re-clustering API functions (which are not currently called anywhere else); for those functions, it only adds an optional const int& parameter.

vaughnbetz commented 3 months ago

It looks like the prior PR I merged on (another one of your PRs) resulted in a conflict with this one. Some line of code got changed twice so git can't figure out which one to use; they are likely the same though as both PRs are closely related. @KA7E : you'll have to pull and resolve the conflict (it should be an easy one) before this can be merged.

It's probably best to have one PR for a function (or a full file) that you're changing at a time so you don't get self conflicts like this. Smaller PRs are easier to merge, but not if they overlap on the affected code, and PRs with up to a couple of hundred lines of code would still be considered pretty small and aren't very hard to review. Some PRs will also wind up being bigger than that since they're larger changes that are coupled together.