[Prepack] Pack Molecule Data Structure Cleanup #2884
Conversation
Moved the t_pack_molecule data structure to the prepacker. Updated the t_pack_molecule data structure from a linked list to a vector, where each molecule is identified by a unique strong ID. Updated the part of the t_pack_molecule which stores chain information from a shared pointer to a unique ID. Moved clustering related data out of this and into the cluster legalizer. This made the prepacked molecules 100% immutable after they are constructed which makes them much safer to work with. Removed the old "num_blocks" member from t_pack_molecule since it is already available from the atom_blk_ids General code quality cleanups while I was working on this. One header file removed from another header file sent me down a rabbit hole of header files to update. Eventually we need to clean up how we handle header files...
|
VTR Chain Results vs Master (Baseline):
QoR is exactly the same. Runtime is a wash (differences likely due to load). Memory is a very slight increase; I ripped the following data regarding the pack memory:
Each of these increases are under 1% and are likely just due to adding new data structures to maintain information on the molecules (to separate out the clustering information from the molecules). I have launched the nightly tests manually. Currently NightlyTest1 completed with the same QoR results as Amir's run yesterday (1 expected QoR failure). |
|
@vaughnbetz This is ready for review. It looks big, but most of this PR are one-line changes to convert The only files that had big changes were prepack.h / .cpp and cluster_legalizer.h / .cpp where I canged some of the API interfaces. |
|
All nightly tests finished successfully. It had the same exepcted 18 QoR failures (and the values in these failed testcases are the same): |
| * Pre-pack atoms in netlist to molecules | ||
| * 1. Single atoms are by definition a molecule. | ||
| * 2. Forced pack molecules are groupings of atoms that matches a t_pack_pattern definition. | ||
| * 3. Chained molecules are molecules that follow a carry-chain style pattern, |
There was a problem hiding this comment.
If you know when such chained molecules are split into smaller molecules at logic block boundaries (I assume they are split during packing?) then that would be good to document here.
There was a problem hiding this comment.
Thanks for merging this! Raised the PR here: PR #2891
The molecules are not split during clustering. The molecules are never modified after the prepacking stage. The prepacker will create segment chains into molecules which each fit inside of a cluster. It will also maintain IDs so molecules know if they belong to the same chain. The reason I think we do this is because the cluster legalizer only works on molecules and I think it just makes more sense to put the WHOLE molecule into a cluster during legalization than potentially leave a part of it unclustered.
Since many people are starting to look into the Packer for different reasons (refactoring / paralellism / etc.) I wanted to clean up the prepacker. The packed molecules being a linked list has been a painful thing to work with; so changing it to a vector and giving each molecule a unique ID really makes it easier to work with the molecules. I also resolved a data dependency in the packer regarding these molecules. The packer was modifying data in the molecules for carry chains. I moved this data out, into the cluster legalizer, which means that the prepacked molecules are never modified after they are created. This should make parallelizing the packer a bit easier.
Moved the t_pack_molecule data structure to the prepacker.
Updated the t_pack_molecule data structure from a linked list to a vector, where each molecule is identified by a unique strong ID.
Updated the part of the t_pack_molecule which stores chain information from a shared pointer to a unique ID. Moved clustering related data out of this and into the cluster legalizer. This made the prepacked molecules 100% immutable after they are constructed which makes them much safer to work with.
Removed the old "num_blocks" member from t_pack_molecule since it is already available from the atom_blk_ids
General code quality cleanups while I was working on this. One header file removed from another header file sent me down a rabbit hole of header files to update. Eventually we need to clean up how we handle header files...
The resolves some of the parts of Issue #2791