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

[Packer] Created GreedyClusterer Class #2816

Conversation

AlexandreSinger
Copy link
Contributor

Began encapsulating the different parts of the packer into classes. This will help organize the packer better, which will make it easier to modify in the future.

My plan is to clean up the different parts of the packer so I can add flat placement information into the gain calculation so it can be integrated into the AP flow.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Nov 20, 2024
Began encapsulating the different parts of the packer into classes. This
will help organize the packer better, which will make it easier to
modify in the future.

My plan is to clean up the different parts of the packer so I can add
flat placement information into the gain calculation so it can be
integrated into the AP flow.
@AlexandreSinger AlexandreSinger force-pushed the feature-greedy-clusterer-class branch from e77d55d to 96322ff Compare November 20, 2024 03:22
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz After looking into the greedy clusterer more, I realized that creating my own AP clusterer would cause me to reinvent the wheel a lot. It will be much easier (and probably more useful for the VTR ecosystem) if I clean up the Packer to make it easier to add the ability to optionally pass a flat placement into the clusterer. This PR starts the process of cleaning things up by creating a GreedyClusterer class. I plan to move some of the functions from cluster_utils.cpp into this class and continue to clean up dead / confusing code. I wanted to merge this in pieces to make it easier to review. Let me know what you think!

@vaughnbetz
Copy link
Contributor

That seems reasonable to me. I think the current clusterer has 3 limitations / trrade-offs:

  • doesn't have any physical info. You are solving that by creating a flat placement that you can use to guide it.
  • Creates one cluster at a time (greedy). This is a separate problem, but probably requires some significant refactoring (or a new clusterer that shares some code) to overcome by making some kind of LSC/simultaneous clustering approach
  • Full legality checking is slow. You may need an even faster heuristic legality checker. This is mostly a separate problem though that ideally wouldn't have to be tackled in your work.

@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz Sounds good! I have some ideas on how we can upgrade the packer with flat placement information. We can discuss in our next one on one. Do you have any comments on this PR? I have another PR almost ready which abstracts the seed selection logic into a class.

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; some commenting suggestions.

vpr/src/pack/cluster_util.cpp Outdated Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.cpp Outdated Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Outdated Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Outdated Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Outdated Show resolved Hide resolved
vpr/src/pack/greedy_clusterer.h Show resolved Hide resolved
Updated comments based on Vaughn's feedback.
@vaughnbetz vaughnbetz merged commit 33c131a into verilog-to-routing:master Nov 22, 2024
37 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-greedy-clusterer-class branch November 26, 2024 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants