Skip to content

Conversation

amin1377
Copy link
Contributor

@amin1377 amin1377 commented Sep 9, 2025

Adding a check to the RR Graph node validation to raise an error if the pin sides stored in the RR Graph don’t correspond to the architecture file.

@amin1377 amin1377 requested a review from vaughnbetz September 9, 2025 14:45
@github-actions github-actions bot added the lang-cpp C/C++ code label Sep 9, 2025
@github-actions github-actions bot added the libarchfpga Library for handling FPGA Architecture descriptions label Sep 9, 2025
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.

What is the node_sides vector used for? It seems like a heavyweight data structure -- a vector of sides for every node in the rr_graph? Is it alive throughout the rr-graph lifetime or just during building?
It could be stored more compactly as a bit vector (pack in all the sides that are used). I'm also not sure it really deserves to be around at all, as most code doesn't need to check the side of a node. Could be flyweighted too.

rr_graph_sides = rr_graph.node_sides(rr_node);
std::tie(std::ignore, std::ignore, arch_side_vec) = get_pin_coordinates(type, ptc_num, std::vector<e_side>(TOTAL_2D_SIDES.begin(), TOTAL_2D_SIDES.end()));
// Number of sides in arch_side_vec may be higher than rr_graph sides since there may be duplicates (a pin may have different x/y offset on the same side)
// Because of that, we itearte over arch_side_vec and check if the side is in rr_graph_sides
Copy link
Contributor

Choose a reason for hiding this comment

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

itearate -> iterate

@amin1377
Copy link
Contributor Author

amin1377 commented Sep 9, 2025

@vaughnbetz: The side for each node is already stored in a bit vector format in t_rr_node_data. The function I just added creates a vector on demand, which is only used during checking.

@github-actions github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Sep 9, 2025
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexandreSinger AlexandreSinger merged commit 4e04b79 into master Sep 10, 2025
30 checks passed
@AlexandreSinger AlexandreSinger deleted the check_rr_graph_arch branch September 10, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants