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

drt: Better Dynamic Programming (DP) Nodes and Elimination of getNestedIdx #5901

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bnmfw
Copy link
Contributor

@bnmfw bnmfw commented Oct 8, 2024

This PR is branched from #5872 and is waiting for it to merge.

std::vector<FlexDPNode>& nodes is a convoluted data structure, it has a non intuitive way of indexing nodes and holds a lot of empty data. This is the first PR in a sequences that will refactor this structure to be std::vector<std::vector<FlexDPNode>>& nodes, which is a more intuitive data structure for it and will make the code more readable.

Here are the changes and how they contribute to this objective:

FlexDPNode Decoupling from old nodes structure

The FlexDPNode object holds a the private prev_node_idx_ which is the index of the previous node in the nodes structure. This was changed so now it holds a pointer to the node itself, if one exists.

Other changes were made to the object adding auxiliary methods to make the code more readable, here are some examples:

  • (curr_node->hasPrevNode()) reads better than (curr_node->getPrevNodeIdx() != -1)
  • (prev_node->isSource() || curr_node->isDrain()) reads better than if (prev_idx_1 == -1 || curr_idx_1 == (int) insts.size())

The use of idx_ as a std::pair<int,int> allows for getting both values without the use of getNestedIdx() which leads to the second change

Elimination of getNestedIdx()

As code was refactored, getting the composed idx directly from node was more convenient and direct than using getNestedIdx(), so it was completely eliminated to only get idx from nodes.

This required both getEdgeCost() functions to get FlexDPNode* as inputs, instead of node indexes.

Copy link
Contributor

github-actions bot commented Oct 8, 2024

clang-tidy review says "All clean, LGTM! 👍"

@maliberty
Copy link
Member

Is std::vector<std::vector<FlexDPNode>>& nodes a matrix? If so there are more efficient mechanisms like https://www.boost.org/doc/libs/1_86_0/libs/multi_array/doc/user.html

@bnmfw
Copy link
Contributor Author

bnmfw commented Oct 8, 2024

Is std::vector<std::vector<FlexDPNode>>& nodes a matrix? If so there are more efficient mechanisms like https://www.boost.org/doc/libs/1_86_0/libs/multi_array/doc/user.html

No it is a vector of vectors indeed. Actually, the current implementation represents a matrix (with a lot of empty data).

This is what I want to talk about on today's meeting actually, but I gave myself a headstart and already started changing things

Signed-off-by: bernardo <[email protected]>
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@bnmfw bnmfw marked this pull request as ready for review November 21, 2024 17:02
Signed-off-by: bernardo <[email protected]>
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@bnmfw bnmfw marked this pull request as draft November 22, 2024 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
drt Detailed Routing In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants