Skip to content

[FASM] Fixed Bug With Wire Creation #3067

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

Merged
merged 2 commits into from
May 21, 2025
Merged

Conversation

AlexandreSinger
Copy link
Contributor

Found a bug within FASM's wire generation where it uses the index of the output pin to create the wire instead of the index of the input pin.

This stemmed from some confusing code which both verified that the wire was being used and returning the first valid pin. It just so happens that it checked the outputs first and returned the output pin instead.

Cleaned up the code and added more error checking to prevent issues like this in the future.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels May 20, 2025
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz , when upgrading to the most recent version of VTR, our testing suite at ZeroASIC found this issue. We simulated the FASM generated by genfasm and found that the lookup table for the wire implementation was incorrect. Based on this issue, we believe that the first inputs to wires were always being used; however, due to recent changes to the packer, we may have allowed different inputs of "wire" mode LUTs to be used which exposed this issue.

This PR fixes the issue and adds some self-checking code to try and prevent headache in the future. Please review when you have a moment.

@AlexandreSinger AlexandreSinger requested a review from amin1377 May 21, 2025 12:51
@AlexandreSinger AlexandreSinger force-pushed the feature-fasm-wire-fix branch from 858e45f to 852cd67 Compare May 21, 2025 13:02
Copy link
Contributor

@amin1377 amin1377 left a comment

Choose a reason for hiding this comment

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

Thanks, Alex! I know you didn’t write the code, but I thought you might have some insight into my questions. For both get_wire_input_pin and is_wire_used, based on the input parameters, I’m not quite sure what the term "wire" refers to. If you could help clarify that, I’d really appreciate it.

@AlexandreSinger AlexandreSinger force-pushed the feature-fasm-wire-fix branch from 852cd67 to e513dbb Compare May 21, 2025 13:20
Found a bug within FASM's wire generation where it uses the index of the
output pin to create the wire instead of the index of the input pin.

This stemmed from some confusing code which both verified that the wire
was being used and returning the first valid pin. It just so happens
that it checked the outputs first and returned the output pin instead.

Cleaned up the code and added more error checking to prevent issues like
this in the future.
@AlexandreSinger AlexandreSinger force-pushed the feature-fasm-wire-fix branch from 6d6e038 to 472db87 Compare May 21, 2025 14:15
@AlexandreSinger AlexandreSinger merged commit 261abb6 into master May 21, 2025
36 checks passed
@AlexandreSinger AlexandreSinger deleted the feature-fasm-wire-fix branch May 21, 2025 16:13
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