Skip to content

rr_graph: avoide div-by-zero issues while getting the delay norm fac #1601

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 1 commit into from
Nov 25, 2020

Conversation

acomodi
Copy link
Collaborator

@acomodi acomodi commented Nov 24, 2020

Signed-off-by: Alessandro Comodi [email protected]

Description

This PR solves a coverty issue on the delay normalization factor calculation function for which, in corner cases, there might be a division by zero.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@acomodi acomodi requested a review from vaughnbetz November 24, 2020 10:14
@github-actions github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Nov 24, 2020
@vaughnbetz
Copy link
Contributor

Would returning 0 result in 0 base costs? If so, I think it's better to return 1 or 1e-9 (1 ns) if this happens. Since we're issuing a warning and continuing we should make sure the returned value will let routing complete.

@acomodi
Copy link
Collaborator Author

acomodi commented Nov 24, 2020

I think it's better to return 1 or 1e-9 (1 ns) if this happens

@vaughnbetz Sure, makes sense. Fixed

@vaughnbetz
Copy link
Contributor

LGTM. Whenever CI goes green this can be merged.

@vaughnbetz
Copy link
Contributor

Not sure what is going on in Travis (looks like one test hung) but this is clearly safe and passed the more extensive tests. Merging.

@acomodi
Copy link
Collaborator Author

acomodi commented Nov 25, 2020

@vaughnbetz Travis is currently having some troubles for what I understood, and this is happening pretty much everywhere. I think we might need to consider to move towards using GH actions instead.

@vaughnbetz
Copy link
Contributor

Hmmm ... won't let me merge unless I change permissions. @acomodi if you can't merge either can you relaunch the hung travis CI build?

@vaughnbetz
Copy link
Contributor

That would be fine with me. Good topic for Thursday.

@vaughnbetz vaughnbetz merged commit 93af9f1 into verilog-to-routing:master Nov 25, 2020
@vaughnbetz
Copy link
Contributor

OK, changed permissions temporarily to let me merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants